Re: [PATCH 08/25] lpfc: cleanup: Remove lock on SCSI io completion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/28/2018 1:16 AM, Hannes Reinecke wrote:
On 12/27/18 12:33 AM, James Smart wrote:
A scsi host lock is taken on every io completion to check whether
someone is waiting on the io completion. The lock doesn't have to be
taken on all ios, only those that have been marked as aborted.

Rework to avoid the lock on non-aborted ios.

Signed-off-by: Dick Kennedy <dick.kennedy@xxxxxxxxxxxx>
Signed-off-by: James Smart <jsmart2021@xxxxxxxxx>
---
  drivers/scsi/lpfc/lpfc_scsi.c | 13 +++++++------
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index efc24393d927..f1545ee3da7c 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -739,7 +739,6 @@ lpfc_get_scsi_buf_s4(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp)
      lpfc_cmd->cur_iocbq.iocb_flag = LPFC_IO_FCP;
      lpfc_cmd->prot_seg_cnt = 0;
      lpfc_cmd->seg_cnt = 0;
-    lpfc_cmd->waitq = NULL;
      lpfc_cmd->timeout = 0;
      lpfc_cmd->flags = 0;
      lpfc_cmd->start_time = jiffies;
@@ -3939,13 +3938,15 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pIocbIn,
      cmd->scsi_done(cmd);
      /*
-     * If there is a thread waiting for command completion
+     * If there is an abort thread waiting for command completion
       * wake up the thread.
       */
-    spin_lock_irqsave(shost->host_lock, flags);
-    if (lpfc_cmd->waitq)
-        wake_up(lpfc_cmd->waitq);
-    spin_unlock_irqrestore(shost->host_lock, flags);
+    if (unlikely(lpfc_cmd->cur_iocbq.iocb_flag & LPFC_DRIVER_ABORTED)) {
+        spin_lock_irqsave(shost->host_lock, flags);
+        if (lpfc_cmd->waitq)
+            wake_up(lpfc_cmd->waitq);
+        spin_unlock_irqrestore(shost->host_lock, flags);
+    }
      lpfc_release_scsi_buf(phba, lpfc_cmd);
  }

You sure about this?
For this to work properly it relies on the LPFC_DRIVER_ABORTED flag to be set, _and_ the new value propagated to all CPUs.
Yet no such mechanism are present here.

What _might_ work is to invert the logic here; namely have a 'LPFC_DRIVER_INFLIGHT' bit in the iocb, which is getting set prior to submit the command, and cleared in the _normal_ iocb completion routine. So when a timeout is hit we can check this flag, and only take the lock if the flag is still set. So if the flag is still set we might have hit the race condition, but then we need to check the lock anyway which will resolve the ambiguity.
And we don't impact the fast path with this.

ok - I'll look at this again.

-- james





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux