On Fri, Apr 22, 2022 at 5:08 PM Dan Williams <dan.j.willi...@intel.com> wrote: > > On Fri, Apr 22, 2022 at 4:58 PM Ira Weiny <ira.we...@intel.com> wrote: > > > > On Thu, Apr 21, 2022 at 08:33:18AM -0700, Dan Williams wrote: > > > The CXL "root" device, ACPI0017, is an attach point for coordinating > > > platform level CXL resources and is the parent device for a CXL port > > > topology tree. As such it has distinct locking rules relative to other > > > CXL subsystem objects, but because it is an ACPI device the lock class > > > is established well before it is given to the cxl_acpi driver. > > > > This final sentence gave me pause because it implied that the device lock > > class > > was set to something other than no validate. But I don't see that anywhere > > in > > the acpi code. So given that it looks to me like ACPI is just using the > > default no validate class... > > Oh, good observation. *If* ACPI had set a custom lock class then > cxl_acpi would need to be careful to restore that ACPI-specific class > and not reset it to "no validate" on exit, or skip setting its own > custom class. However, I think for generic buses like ACPI that feed > devices into other subsystems it likely has little reason to set its > own class. For safety, since device_lock_set_class() is general > purpose, I'll have it emit a debug message and fail if the class is > not "no validate" on entry. >
So this turned out way uglier than I expected: drivers/cxl/acpi.c | 4 +++- include/linux/device.h | 33 +++++++++++++++++++++++++-------- 2 files changed, 28 insertions(+), 9 deletions(-) ...so I'm going to drop it and just add a comment about the expectations. As Peter said there's already a multitude of ways to cause false positive / negative results with lockdep so this is just one more area where one needs to be careful and understand the lock context they might be overriding.