Re: [PATCH v2 1/3] PCI: Revert the cfg_access_lock lockdep mechanism

[Leon Romanovsky <leon@kernel.org>]

On Tue, Jun 04, 2024 at 12:11:21PM -0500, Bjorn Helgaas wrote:
> On Tue, Jun 04, 2024 at 11:03:54AM +0300, Kalle Valo wrote:
> > Dan Williams <dan.j.williams@intel.com> writes:
> > 
> > > While the experiment did reveal that there are additional places that
> > > are missing the lock during secondary bus reset, one of the places that
> > > needs to take cfg_access_lock (pci_bus_lock()) is not prepared for
> > > lockdep annotation.
> > >
> > > Specifically, pci_bus_lock() takes pci_dev_lock() recursively and is
> > > currently dependent on the fact that the device_lock() is marked
> > > lockdep_set_novalidate_class(&dev->mutex). Otherwise, without that
> > > annotation, pci_bus_lock() would need to use something like a new
> > > pci_dev_lock_nested() helper, a scheme to track a PCI device's depth in
> > > the topology, and a hope that the depth of a PCI tree never exceeds the
> > > max value for a lockdep subclass.
> > >
> > > The alternative to ripping out the lockdep coverage would be to deploy a
> > > dynamic lock key for every PCI device. Unfortunately, there is evidence
> > > that increasing the number of keys that lockdep needs to track to be
> > > per-PCI-device is prohibitively expensive for something like the
> > > cfg_access_lock.
> > >
> > > The main motivation for adding the annotation in the first place was to
> > > catch unlocked secondary bus resets, not necessarily catch lock ordering
> > > problems between cfg_access_lock and other locks. Solve that narrower
> > > problem with follow-on patches, and just due to targeted revert for now.
> > >
> > > Fixes: 7e89efc6e9e4 ("PCI: Lock upstream bridge for pci_reset_function()")
> > > Reported-by: Imre Deak <imre.deak@intel.com>
> > > Closes: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_134186v1/shard-dg2-1/igt@device_reset@unbind-reset-rebind.html
> > > Cc: Jani Saarinen <jani.saarinen@intel.com>
> > > Cc: Dave Jiang <dave.jiang@intel.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > 
> > In our ath11k test box commit 7e89efc6e9e4 was causing random kernel
> > crashes. I tested patches 1-3 and did not see anymore crashes so:
> > 
> > Tested-by: Kalle Valo <kvalo@kernel.org>
> 
> Added to commit logs, thank you!
> 

Thanks,
Tested-by: Leon Romanovsky <leonro@nvidia.com>