Re: a small problem about pci_stop_and_remove_bus_device() function

[Jiang Liu <liuj97@gmail.com>]

On 09/14/2012 12:43 AM, Bjorn Helgaas wrote:
> I didn't intend this change in behavior, so in that sense, this is a
> regression.  We could restore the previous behavior by changing
> pci_stop_and_remove_bus_device() so the "pci_remove_bus();
> dev->subordinate = NULL;" code is after the pci_stop_dev() call, as
> you suggest.
> 
> But I'm not 100% sure that's the correct fix.  I think it's possible
> to have a bridge device that has no secondary bus.  For example, I
> don't think a bridge configured with "secondary > subordinate", e.g.,
> "[bus ff-00]", will forward any config transactions downstream.  In
> that case, we'll have a struct pci_dev for the bridge device, but
> there won't be a struct pci_bus for the secondary bus, so
> dev->subordinate will be NULL.  There are actually quite a few
> existing tests for this situation in pnv_ioda_setup_bus_PE(),
> eeh_add_device_tree_late(), yenta_probe(), etc.
> 
> When we enumerate bridges, we build the bridge pci_dev before building
> the downstream pci_bus, so symmetry suggests that we should tear down
> the pci_bus before tearing down the pci_dev.
> 
> So I wonder if a better fix is remove the assumption that
> "dev->subordinate != NULL means this is a bridge device."  There are
> many places where we test "dev->subordinate" to iterate through
> downstream devices or something similar; those should be fine.  We'd
> only have to change the places that care about actual type of the
> device, e.g., the config space differences between header types.
> 
> Where did you trip over this?  If you just found this by inspection,
> my congratulations, it's a pretty subtle issue :)
HI Bjorn,
	This issue was disclosed when developing the patch set "[PATCH v2 0/9] 
enhance PCI related drivers to handle hotplug events". BTW, I have sent this patch
set to you just now.
	We are not assume that dev->subordinate is not NULL for a bridge device,
but that dev->subordinate is consistent when PCI bus notification callbacks are
called for BUS_NOTIFY_ADD_DEVICE and BUS_NOTIFY_DEL_DEVICE.
	Thanks!
	Gerry