Re: [Qemu-devel] [for-2.12 6/7] pci: Eliminate redundant PCIDevice::bus pointer

[David Gibson]

On Wed, Nov 29, 2017 at 09:41:09AM -0200, Eduardo Habkost wrote:
> On Wed, Nov 29, 2017 at 12:54:04PM +0200, Marcel Apfelbaum wrote:
> > On 29/11/2017 10:46, David Gibson wrote:
> > > The bus pointer in PCIDevice is basically redundant with QOM information.
> > > It's always initialized to the qdev_get_parent_bus(), the only difference
> > > is the type.
> > > 
> > > Therefore this patch eliminates the field, instead creating a 
> > > pci_get_bus()
> > > helper to do the type mangling to derive it conveniently from the QOM
> > > Device object underneath.
> > > 
> > 
> > 
> > Hi David,
> > 
> > I personally don't see why the caching of the bus bothers you.
> > It makes the code a little easier to read, but indeed is a duplication
> > of data; let's see what Michael has to say, is a matter of
> > taste I suppose.
> 
> I'm all for removing duplication of data because it makes the
> code less fragile.  We don't even take care of clearing
> pci_dev->bus when the device is removed from the bus, so there's
> a window between unplugging a device and actually freeing the
> object where pci_dev->bus can become a dangling pointer.

Right.  In some other things I'm working on I also got bitten by the
fact that the PCI parent pointer wasn't initialized until realize()
time, when I needed to get to the bus before that.

As a rule I like to keep data normalized, because it's harder for
things to get out of sync confusingly.  The fact that on modern
hardware recalculating things is generally faster than the the extra
cache footprint is just a bonus.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature