Re: [PATCH RFC v2] platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe

Andy Shevchenko
2023-12-13T14:06:00Z

On Tue, Dec 12, 2023 at 08:47:46PM +0900, Shin'ichiro Kawasaki wrote:
> p2sb_bar() unhides P2SB device to get resources from the device. It
> guards the operation by locking pci_rescan_remove_lock so that parallel
> rescans do not find the P2SB device. However, this lock causes deadlock
> when PCI bus rescan is triggered by /sys/bus/pci/rescan. The rescan
> locks pci_rescan_remove_lock and probes PCI devices. When PCI devices
> call p2sb_bar() during probe, it locks pci_rescan_remove_lock again.
> Hence the deadlock.
>
> To avoid the deadlock, do not lock pci_rescan_remove_lock in p2sb_bar().
> Instead, do the lock at fs_initcall. Introduce p2sb_cache_resources()
> for fs_initcall which gets and caches the P2SB resources. At p2sb_bar(),
> refer the cache and return to the caller.

...

> +/* Cache BAR0 of P2SB device from function 0 ot 7 */
> +#define NR_P2SB_RES_CACHE 8

Here... (at least some better comment with TODO needs to be added,
and next patches can address that).

> struct resource *bar0 = &pdev->resource[0];

...and here... (okay, not exactly here, but with the same idea,
to use pci_resource_n() macro)

> + if (!PCI_FUNC(devfn_p2sb)) {
> + slot_p2sb = PCI_SLOT(devfn_p2sb);
> + for (fn = 1; fn < 8; fn++)

...and here...

> + p2sb_scan_and_cache(bus, PCI_DEVFN(slot_p2sb, fn));
> + }

...and so on I gave comments. Any reason why they left unaddressed?

--
With Best Regards,
Andy Shevchenko