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

[Bjorn Helgaas <helgaas@kernel.org>]

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!