Whilst working on what was supposed to be a fairly trivial patch, I
stumbled across what looks to be a relatively significant bug in the way
pci bridges are handled. I could easily be wrong, as the pci code is
far from anything I'd be willing to call an area of expertise. That
said, my description of the problem follows:
I am trying to add a file to the pci bus (ie: /sysfs/devices/pciXXXX:XX)
directories in sysfs. This led to the discovery that struct pci_bus
(inlude/linux/pci.h) does not have an *actual* struct dev inside it,
rather a pointer to a struct dev. I found this interesting, as most
device-type-thingies have an honest to goodness struct dev, allowing the
use of container_of(), etc. A quick perusal of the code offered no
reason why struct pci_bus couldn't be changed to have the actual struct
dev inside it, saving us kmalloc'ing and kfree'ing these, and generally
keeping track of them. I was wrong.
In pci_alloc_child_bus (drivers/pci/probe.c), the child bus is allocated
and it's struct dev * is set to point to the struct dev belonging to the
bridge that this bus is 'on', or 'behind'. pci_alloc_child_bus is
called in 3 places: pci_add_new_bus and twice in pci_scan_bridge. The
calls in pci_scan_bridge allocate a new struct pci_bus, but then seem to
throw the references away, *without* freeing them.
It appears that these pseudo-busses are allocated and used as a kind of
hack, to allow devices to be parented to pci bridge devices. The
pci_dev for the bridge is allocated and initialized, then a child bus is
created, and it's *dev is pointed to the struct device belonging to the
pci bridge. There are no refcounts used for this, and it doesn't appear
to be cleaned up in any way. If anyone can offer any insight into this
problem, or show me why I'm wrong, it would be greatly appreciated.
Thanks!
-Matt
On Wed, Sep 24, 2003 at 05:34:03PM -0700, Matthew Dobson wrote:
> In pci_alloc_child_bus (drivers/pci/probe.c), the child bus is allocated
> and it's struct dev * is set to point to the struct dev belonging to the
> bridge that this bus is 'on', or 'behind'. pci_alloc_child_bus is
> called in 3 places: pci_add_new_bus and twice in pci_scan_bridge. The
> calls in pci_scan_bridge allocate a new struct pci_bus, but then seem to
> throw the references away, *without* freeing them.
That is correct - they persist after they have been allocated until the
bridge device is destroyed (if ever) - it's lifetime is directly equivalent
to the lifetime of the bridge.
If you look carefully at pci_alloc_child_bus(), you will notice that
bridge->subordinate is setup to point at the pci_bus, which provides
a method to access the data held in the pci_bus later (eg, while we're
freeing the structures.)
--
Russell King ([email protected]) http://www.arm.linux.org.uk/personal/
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
Russell King wrote:
> On Wed, Sep 24, 2003 at 05:34:03PM -0700, Matthew Dobson wrote:
>
>>In pci_alloc_child_bus (drivers/pci/probe.c), the child bus is allocated
>>and it's struct dev * is set to point to the struct dev belonging to the
>>bridge that this bus is 'on', or 'behind'. pci_alloc_child_bus is
>>called in 3 places: pci_add_new_bus and twice in pci_scan_bridge. The
>>calls in pci_scan_bridge allocate a new struct pci_bus, but then seem to
>>throw the references away, *without* freeing them.
>
>
> That is correct - they persist after they have been allocated until the
> bridge device is destroyed (if ever) - it's lifetime is directly equivalent
> to the lifetime of the bridge.
>
> If you look carefully at pci_alloc_child_bus(), you will notice that
> bridge->subordinate is setup to point at the pci_bus, which provides
> a method to access the data held in the pci_bus later (eg, while we're
> freeing the structures.)
Ok, I see that now. I guess my only remaining question is why do child
busses not get their own struct device, but rather only a pointer to the
bridge's struct device? There's no refcounting done on this, ie: no
pci_dev_get/put calls, but I guess that's kinda ok, since we're pretty
sure that the child bus won't exist for longer than the bridge that owns
it, right? So using the bridge's struct dev allows the pci topology to
look cleaner? As in, there's no actual bus exposed in sysfs/procfs/etc,
just devices that seem to be hanging off the bridge?
Cheers!
-Matt
> Ok, I see that now. I guess my only remaining question is why do child
> busses not get their own struct device, but rather only a pointer to the
> bridge's struct device? There's no refcounting done on this, ie: no
> pci_dev_get/put calls, but I guess that's kinda ok, since we're pretty
> sure that the child bus won't exist for longer than the bridge that owns
> it, right? So using the bridge's struct dev allows the pci topology to
> look cleaner? As in, there's no actual bus exposed in sysfs/procfs/etc,
> just devices that seem to be hanging off the bridge?
Buses are not devices. Bridges are devices and get a struct device. Buses
are physical (or logical) collections of devices at the same topological
level which reside on one side of a bridge. I.e. they are objects of some
sort, but not devices, and hence are not represented in /sys/devices/.
It would be nice to export them in /sys/bus/pci/ somehow, but it's one of
those things that I haven't gotten around to in the last 6 months or so.
:)
Pat