Hi,
On 11/30/21 20:39, Lukas Wunner wrote:
> On Tue, Nov 30, 2021 at 11:15:40AM +0100, Hans de Goede wrote:
>> On 11/29/21 19:59, Lukas Wunner wrote:
>>> Hm, I found the notes that I took when I investigated Theodore's report:
>>> Using a subclass may be problematic because it's limited to a value < 8
>>> (MAX_LOCKDEP_SUBCLASSES). If there's a hotplug port at a deeper level
>>> than 8 in the PCI hierarchy (can easily happen, Thunderbolt daisy chain
>>> allows up to 6 devices, each device contains a PCIe switch, so 2 levels per
>>> device), look_up_lock_class() in kernel/locking/lockdep.c will emit a BUG
>>> error message.
>
> Actually, after thinking about this some more it has occurred to me
> that you should be able to make do with 8 subclasses if you change
> the algorithm in patch [1/2] to something like this:
>
> while (bus->parent) {
> - depth++;
> bus = bus->parent;
> + if (bus->self->is_hotplug_bridge)
> + depth++;
> }
>
> (It may be necessary to add a NULL pointer check for bus->self,
> off the top of my hat I'm not sure if that's set for the root bus.)
>
> Because with the patches as they are, the array of 8 subclasses is
> sparsely populated, i.e. if a parent port is not a hotplug port,
> a subclass is wasted. You only care about hotplug ports (more
> specifically those handled by pciehp) because those are the only
> ones with a reset_lock. The above change should result in a
> densely populated array of subclasses.
>
> Naturally, because this makes the function pciehp-specific,
> it should be moved from include/linux/pci.h to pciehp_hpc.c.
>
> With Thunderbolt you can have 6 devices plus the hotplug port in
> the host controller. And there may be an 8th hotplug port at the
> Root Port if the host controller can be "unplugged" when it goes
> to D3cold. (That's handled by acpiphp, not pciehp, and I'm not
> even sure if is_hotplug_bridge is set in that case.)
> So 8 subclasses should be sufficient.
>
> Aside from Thunderbolt, nested hotplug ports exist in data centers
> with hot-removable NVMe slots in hot-removable chassis, but I highly
> doubt those use cases have more than 8 levels of hotplug ports.
>
> So that would probably be a pragmatic and minimalistic approach to this
> problem.
Ack. I've prepared and tested a v2 patch using this approach, this
v2 also addresses all your and Bjorn's other remarks.
I will send out the v2 patch right after this email.
Regards,
Hans
