On Fri, Apr 22, 2022 at 9:28 PM Ira Weiny <ira.we...@intel.com> wrote: > > On Thu, Apr 21, 2022 at 08:33:51AM -0700, Dan Williams wrote: > > Lockdep reports the following deadlock scenarios for CXL root device > > power-management, device_prepare(), operations, and device_shutdown() > > operations for 'nd_region' devices: > > > > --- > > Chain exists of: > > &nvdimm_region_key --> &nvdimm_bus->reconfig_mutex --> > > system_transition_mutex > > > > Possible unsafe locking scenario: > > > > CPU0 CPU1 > > ---- ---- > > lock(system_transition_mutex); > > lock(&nvdimm_bus->reconfig_mutex); > > lock(system_transition_mutex); > > lock(&nvdimm_region_key); > > > > -- > > > > Chain exists of: > > &cxl_nvdimm_bridge_key --> acpi_scan_lock --> &cxl_root_key > > > > Possible unsafe locking scenario: > > > > CPU0 CPU1 > > ---- ---- > > lock(&cxl_root_key); > > lock(acpi_scan_lock); > > lock(&cxl_root_key); > > lock(&cxl_nvdimm_bridge_key); > > > > --- > > > > These stem from holding nvdimm_bus_lock() over hibernate_quiet_exec() > > which walks the entire system device topology taking device_lock() along > > the way. The nvdimm_bus_lock() is protecting against unregistration, > > multiple simultaneous ops callers, and preventing activate_show() from > > racing activate_store(). For the first 2, the lock is redundant. > > Unregistration already flushes all ops users, and sysfs already prevents > > multiple threads to be active in an ops handler at the same time. For > > the last userspace should already be waiting for its last > > activate_store() to complete, and does not need activate_show() to flush > > the write side, so this lock usage can be deleted in these attributes. > > > > I'm sorry if this is obvious but why can't the locking be removed from > capability_show() and nvdimm_bus_firmware_visible() as well?
It can, that's a good catch, thanks.
