在 2021/12/14 20:34, John Garry 写道:

Please consider som rewrite:

On 17/11/2021 02:45, chenxiang wrote:

"scsi: libsas: Keep sas host active until finished some work" -> 
"scsi: libsas: Keep host active while processing events"

quoted

From: Xiang Chen <redacted>

For those works from event queue, if executing them such as
PORTE_BROADCAST_RCVD when sas host is suspended, it will resume sas host
firstly as SMP IOs are sent in the process. So phyup will occur and it
will call work PORTE_BYTES_DMAED. But the original work (such as
PORTE_BROADCAST_RCVD) and the work PORTE_BYTES_DMAED are in the same
singlethread workqueue, so the complete of original work waits for
the complete of work PORTE_BYTES_DMAED while the complete of work
PORTE_BYTES_DMAED wait for the complete of original and it is blocked.

So call pm_runtime_get_noresume() to keep sas host active until
finished those works.

Processing events such as PORTE_BROADCAST_RCVD may cause dependency 
issues for runtime power management support.

Such a problem would be that handling a PORTE_BROADCAST_RCVD event 
requires that the host is resumed to send SMP commands. However, in 
resuming the host, the phyup events generated from re-enabling the 
phys are processed in the same workqueue as the original 
PORTE_BROADCAST_RCVD event. As such, the host will never finish 
resuming (as it waits for the phyup event processing), and then the 
PORTE_BROADCAST_RCVD event cannot be processed as the SMP commands are 
blocked, and so we have a deadlock.

Solve this problem by ensuring that libsas keeps the host active until 
completely finished processing phy or port events, such as 
PORTE_BYTES_DMAED. As such, we don't have to worry about resuming the 
host for processing individual SMP commands in this example.

ok, i will consider to rewrite them.

quoted

Signed-off-by: Xiang Chen <redacted>
Reviewed-by: John Garry <redacted>
---
  drivers/scsi/libsas/sas_event.c | 24 +++++++++++++++++++++---
  1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libsas/sas_event.c 

b/drivers/scsi/libsas/sas_event.c
index 626ef96b9348..3613b9b315bc 100644

--- a/drivers/scsi/libsas/sas_event.c
+++ b/drivers/scsi/libsas/sas_event.c

@@ -50,8 +50,10 @@ void sas_queue_deferred_work(struct sas_ha_struct 

*ha)
      list_for_each_entry_safe(sw, _sw, &ha->defer_q, drain_node) {
          list_del_init(&sw->drain_node);
          ret = sas_queue_work(ha, sw);
-        if (ret != 1)
+        if (ret != 1) {
+            pm_runtime_put(ha->dev);
sas_free_event(to_asd_sas_event(&sw->work));
+        }
      }
      spin_unlock_irq(&ha->lock);
  }

@@ -126,16 +128,22 @@ void sas_enable_revalidation(struct 

sas_ha_struct *ha)
  static void sas_port_event_worker(struct work_struct *work)
  {
      struct asd_sas_event *ev = to_asd_sas_event(work);
+    struct asd_sas_phy *phy = ev->phy;
+    struct sas_ha_struct *ha = phy->ha;
        sas_port_event_fns[ev->event](work);
+    pm_runtime_put(ha->dev);
      sas_free_event(ev);
  }
    static void sas_phy_event_worker(struct work_struct *work)
  {
      struct asd_sas_event *ev = to_asd_sas_event(work);
+    struct asd_sas_phy *phy = ev->phy;
+    struct sas_ha_struct *ha = phy->ha;
        sas_phy_event_fns[ev->event](work);
+    pm_runtime_put(ha->dev);
      sas_free_event(ev);
  }
  @@ -170,14 +178,19 @@ int sas_notify_port_event(struct asd_sas_phy 
*phy, enum port_event event,
      if (!ev)
          return -ENOMEM;
  +    /* Call pm_runtime_put() with pairs in sas_port_event_worker() */
+    pm_runtime_get_noresume(ha->dev);
+
      INIT_SAS_EVENT(ev, sas_port_event_worker, phy, event);
        if (sas_defer_event(phy, ev))
          return 0;
        ret = sas_queue_event(event, &ev->work, ha);
-    if (ret != 1)
+    if (ret != 1) {
+        pm_runtime_put(ha->dev);
          sas_free_event(ev);
+    }
        return ret;
  }

@@ -196,14 +209,19 @@ int sas_notify_phy_event(struct asd_sas_phy 

*phy, enum phy_event event,
      if (!ev)
          return -ENOMEM;
  +    /* Call pm_runtime_put() with pairs in sas_phy_event_worker() */
+    pm_runtime_get_noresume(ha->dev);
+
      INIT_SAS_EVENT(ev, sas_phy_event_worker, phy, event);
        if (sas_defer_event(phy, ev))
          return 0;
        ret = sas_queue_event(event, &ev->work, ha);
-    if (ret != 1)
+    if (ret != 1) {
+        pm_runtime_put(ha->dev);
          sas_free_event(ev);
+    }
        return ret;
  }

.