Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

PM: runtime: Remove link state checks in rpm_get/put_supplier()

To support runtime PM for hisi SAS driver (the driver is in directory
drivers/scsi/hisi_sas), we add device link between scsi_device->sdev_gendev
(consumer device) and hisi_hba->dev(supplier device) with flags
DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE.

After runtime suspended consumers and supplier, unload the dirver which
causes a hung.

We found that it called function device_release_driver_internal() to
release the supplier device (hisi_hba->dev), as the device link was
busy, it set the device link state to DL_STATE_SUPPLIER_UNBIND, and
then it called device_release_driver_internal() to release the consumer
device (scsi_device->sdev_gendev).

Then it would try to call pm_runtime_get_sync() to resume the consumer
device, but because consumer-supplier relation existed, it would try
to resume the supplier first, but as the link state was already
DL_STATE_SUPPLIER_UNBIND, so it skipped resuming the supplier and only
resumed the consumer which hanged (it sends IOs to resume scsi_device
while the SAS controller is suspended).

Simple flow is as follows:

device_release_driver_internal -> (supplier device)
if device_links_busy ->
device_links_unbind_consumers ->
...
WRITE_ONCE(link->status, DL_STATE_SUPPLIER_UNBIND)
device_release_driver_internal (consumer device)
pm_runtime_get_sync -> (consumer device)
...
__rpm_callback ->
rpm_get_suppliers ->
if link->state == DL_STATE_SUPPLIER_UNBIND -> skip the action of resuming the supplier
...
pm_runtime_clean_up_links
...

Correct suspend/resume ordering between a supplier device and its consumer
devices (resume the supplier device before resuming consumer devices, and
suspend consumer devices before suspending the supplier device) should be
guaranteed by runtime PM, but the state checks in rpm_get_supplier() and
rpm_put_supplier() break this rule, so remove them.

Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
[ rjw: Subject and changelog edits ]
Cc: All applicable <stable@vger.kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

authored by

Xiang Chen and committed by
Rafael J. Wysocki
d12544fb ba4f184e

+1 -4
+1 -4
drivers/base/power/runtime.c
··· 291 291 device_links_read_lock_held()) { 292 292 int retval; 293 293 294 - if (!(link->flags & DL_FLAG_PM_RUNTIME) || 295 - READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND) 294 + if (!(link->flags & DL_FLAG_PM_RUNTIME)) 296 295 continue; 297 296 298 297 retval = pm_runtime_get_sync(link->supplier); ··· 311 312 312 313 list_for_each_entry_rcu(link, &dev->links.suppliers, c_node, 313 314 device_links_read_lock_held()) { 314 - if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND) 315 - continue; 316 315 317 316 while (refcount_dec_not_one(&link->rpm_active)) 318 317 pm_runtime_put(link->supplier);