Re: [PATCH 2/2] PCI: pciehp: Use down_read/write_nested(reset_lock) to fix lockdep errors

From: Lukas Wunner <lukas@wunner.de>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Theodore Ts'o <tytso@mit.edu>,
	Andreas Noever <andreas.noever@gmail.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH 2/2] PCI: pciehp: Use down_read/write_nested(reset_lock) to fix lockdep errors
Date: Mon, 29 Nov 2021 19:59:46 +0100	[thread overview]
Message-ID: <20211129185946.GA1475@wunner.de> (raw)
In-Reply-To: <20211129153943.GA4896@wunner.de>

On Mon, Nov 29, 2021 at 04:39:43PM +0100, Lukas Wunner wrote:
> On Mon, Nov 29, 2021 at 01:19:34PM +0100, Hans de Goede wrote:
> > Use down_read_nested() and down_write_nested() when taking the
> > ctrl->reset_lock rw-sem, passing the PCI-device depth in the hierarchy
> > as lock subclass parameter. This fixes the following false-positive lockdep
> > report when unplugging a Lenovo X1C8 from a Lenovo 2nd gen TB3 dock:
> [...]
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> That's exactly what I had in mind, thanks.

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.

It may be necessary to call lockdep_register_key() for each level or
for each hotplug port and assign the lock with lockdep_set_class()
(or ..._and_name() and use the dev_name()).

It's these complications that made me put aside the problem back in the day.
My apologies for not remembering them earlier.

Thanks,

Lukas