Re: pci_walk_bus race condition

["Zhang, Yanmin" <yanmin_zhang@linux.intel.com>]

On Fri, 2006-06-02 at 13:24, Andrew Morton wrote:
> On Thu, 1 Jun 2006 22:11:41 -0700
> Andrew Morton <akpm@osdl.org> wrote:
> 
> > On Fri, 02 Jun 2006 12:35:43 +0800
> > "Zhang, Yanmin" <yanmin_zhang@linux.intel.com> wrote:
> > 
> > > pci_walk_bus has a race with pci_destroy_dev. When cb is called
> > > in pci_walk_bus, pci_destroy_dev might unlink the dev pointed by next.
> > > Later on in the next loop, pointer next becomes NULL and cause
> > > kernel panic.
> > > 
> > > Below patch against 2.6.17-rc4 fixes it by changing pci_bus_lock (spin_lock)
> > > to pci_bus_sem (rw_semaphore).
> > 
> > How does s/spinlock/rwsem/ fix a race??
> 
> oic.  "and hold the lock across the callback".
Sorry for missing the statement.

> 
> Is the ranking of pci_bus_sem and dev->dev.sem correct+consistent?  It
> looks OK.
Yes, I think so. The write lock of pci_bus_sem and dev->dev.sem are used in
different steps. Here we use read lock of pci_bus_sem + dev->dev.sem.

> 
> It might be worth making a not that the callback function cannot call any
> PCI layer function which takes pci_bus_sem - that'll casue a recursive
> down_read(), which is a nasty source of rare deadlocks.
Currently, only pci error recovery codes call it while cb are the error
callback functions in the driver. They shouldn't try to apply for write lock of
pci_walk_sem. Perhaps we could add more comments in function pci_walk_bus.