Re: [PATCH v2] platform/x86: p2sb: Defer P2SB device scan when P2SB device has func 0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Shinichiro,

On 3/4/24 10:55 AM, Hans de Goede wrote:
> Hi,
> 
> On 3/4/24 4:19 AM, Shinichiro Kawasaki wrote:
>> On Mar 03, 2024 / 20:35, Hans de Goede wrote:

<snip>

>>> Also talking about making things more KISS, how
>>> about completely dropping the fs_initcall and
>>> simply always delay the caching of a devfn until
>>> the first call of p2sb_bar() for that devfn ?
>>>
>>> That way fixing the issue will also actually reduce /
>>> simplify the code :)
>>
>> This will simplify the code more, but it has two drawabacks:
>>
>> 1) It still leaves the rare deadlock scenario. If the drivers which call
>>    p2sb_bar() are not probed during boot, and if they are probed afterwards by
>>    sysfs pci bus rescan, pci_rescan_remove_lock causes the deadlock.
>>
>> 2) It triggers lockdep splat for pci_rescan_remove_lock at sysfs pci bus rescan,
>>    even for devices unrelated to p2sb (This is what I regularly observe during
>>    kernel tests for storage sub-system.)
>>
>> I suggest to limit these drawbacks only on goldmont.
> 
> I was not aware of the lockdep triggering issue. I agree that should be avoided
> when possible. I see you have kept the fs_initcall() for this in v3, good.

I have just taken a second look at my analysis of when p2sb_bar(devfn=0) is called
on Goldmont platforms and my analysis for:

1) edac/pnd2_edac             devfn = 0

was wrong, p2sb_bar(devfn=0) is only used on Denverton (aka "Goldmont D")
which uses the default P2SB_DEVFN_DEFAULT = PCI_DEVFN(31, 1) devfn not
the special Goldmont devfn.

So the p2sb_bar(devfn=0) call actually only happens on Goldmont from:

3) mfd/lpc_ich for pinctrl    devfn = 0

and then only when the ACPI tables fail to properly describe the GPIO
controllers as ACPI devices, if the GPIO controllers are described
in ACPI, which they are on the ASUS VivoBook D540NV-GQ065T then that
call gets skipped.

So on the ASUS VivoBook D540NV-GQ065T p2sb_bar(devfn=0) gets never
called. Which explains why not caching it fixes things. I assume that this
laptop just seems not likes the P2SB is touched at all and by not caching
the BAR for the P2SB it ends up not getting touched at all.

This also means that likely the P2SB devfn itself generally speaking is
often not touched on Goldmont platforms. So we can avoid the lockdep
issue on PCI bus rescan there by caching the SPI controller
PCI_DEVFN(13, 2) devfn from fs_initcall(), since that will be the only
devfn for which p2sb_bar() will get called (except on hw with the
GPIO controller missing from the ACPI tables which should be rare).

I have prepared a follow up patch to your v3 to cache the
SPI controller devfn instead of the P2SB devfn at fs_initcall()
time. I'll post this shortly and I'll also ask the bug reporter
to test the combination of our 2 patches.

Regards,

Hans





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux