Re: [bug report] lockdep WARN at PCI device rescan

Lukas Wunner
2023-11-29T11:17:46Z

On Tue, Nov 28, 2023 at 07:45:06AM +0000, Shinichiro Kawasaki wrote:
> On Nov 24, 2023 / 17:22, Andy Shevchenko wrote:
> > Another possible solution I was thinking about is to have a local cache,
> > so, make p2sb.c to be called just after PCI enumeration at boot time
> > to cache the P2SB device's bar, and then cache the bar based on the device
> > in question at the first call. Yet it may not remove all theoretical /
> > possible scenarios with dead lock (taking into account hotpluggable
> > devices), but won't fail the i801 driver in the above use case IIUC.
>
> Thanks for the idea. I created an experimental patch below (it does not guard
> list nor free the list elements, so it is incomplete). I confirmed that this
> patch avoids the deadlock. So your idea looks working. I still observe the
> deadlock WARN, but it looks better than the hang by the deadlock.

Your patch uses a list to store a multitude of struct resource.
Is that actually necessary? I thought there can only be a single
P2SB device in the system?

> Having said that, Heiner says in another mail that "A solution has to support
> pci drivers using p2sb_bar() in probe()". This idea does not fulfill it. Hmm.

Basically what you need to do is create two initcalls:

Add one arch_initcall to unhide the P2SB device.

The P2SB subsequently gets enumerated by the PCI core in a subsys_initcall.

Then add an fs_initcall which extracts and stashes the struct resource,
hides the P2SB device and destroys the corresponding pci_dev.

Then you don't need to acquire any locks at runtime, just retrieve the
stashed struct resource.

This approach will result in the P2SB device briefly being enumerated
and a driver could in theory bind to it. Andy, is this a problem?
I'm not seeing any drivers in the tree which bind to 8086/c5c5.

Thanks,

Lukas