qla2xxx: Run queuecommand outside the host_lock

PATCH In review Matthew Wilcox 3 participants 5 messages Apr 1, 2011 – Apr 8, 2011

Subsystems (2)

SCSI QLogic QLA2XXX FC-SCSI driver

lore

Share & tools

lore

Matthew Wilcox

author

2011-04-01 20:22 UTC #0 lore raw

PATCH 3/3 qla2xxx: Run queuecommand outside the host_lock

The qla2xxx driver still uses the host_lock, but only to protect against some rare events like rport deletion.

Signed-off-by Matthew Wilcox <[email protected]>

drivers/scsi/qla2xxx/qla_os.c +5 ~~-15~~

index f27724d..61340e1 100644
@@ -506,7 +506,7 @@ qla24xx_fw_version_str(struct scsi_qla_host *vha, char *str)
 
 static inline srb_t *
 qla2x00_get_new_sp(scsi_qla_host_t *vha, fc_port_t *fcport,
-    struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *))
+    struct scsi_cmnd *cmd)
 {
 	srb_t *sp;
 	struct qla_hw_data *ha = vha->hw;

@@ -520,16 +520,15 @@ qla2x00_get_new_sp(scsi_qla_host_t *vha, fc_port_t *fcport,
 	sp->cmd = cmd;
 	sp->flags = 0;
 	CMD_SP(cmd) = (void *)sp;
-	cmd->scsi_done = done;
 	sp->ctx = NULL;
 
 	return sp;
 }
 
 static int
-qla2xxx_queuecommand_lck(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *))
+qla2xxx_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
 {
-	scsi_qla_host_t *vha = shost_priv(cmd->device->host);
+	scsi_qla_host_t *vha = shost_priv(shost);
 	fc_port_t *fcport = (struct fc_port *) cmd->device->hostdata;
 	struct fc_rport *rport = starget_to_rport(scsi_target(cmd->device));
 	struct qla_hw_data *ha = vha->hw;

@@ -537,7 +536,6 @@ qla2xxx_queuecommand_lck(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)
 	srb_t *sp;
 	int rval;
 
-	spin_unlock_irq(vha->host->host_lock);
 	if (ha->flags.eeh_busy) {
 		if (ha->flags.pci_channel_io_perm_failure)
 			cmd->result = DID_NO_CONNECT << 16;

@@ -569,7 +567,7 @@ qla2xxx_queuecommand_lck(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)
 		goto qc24_target_busy;
 	}
 
-	sp = qla2x00_get_new_sp(base_vha, fcport, cmd, done);
+	sp = qla2x00_get_new_sp(base_vha, fcport, cmd);
 	if (!sp)
 		goto qc24_host_busy_lock;

@@ -577,8 +575,6 @@ qla2xxx_queuecommand_lck(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)
 	if (rval != QLA_SUCCESS)
 		goto qc24_host_busy_free_sp;
 
-	spin_lock_irq(vha->host->host_lock);
-
 	return 0;
 
 qc24_host_busy_free_sp:

@@ -586,23 +582,17 @@ qc24_host_busy_free_sp:
 	mempool_free(sp, ha->srb_mempool);
 
 qc24_host_busy_lock:
-	spin_lock_irq(vha->host->host_lock);
 	return SCSI_MLQUEUE_HOST_BUSY;
 
 qc24_target_busy:
-	spin_lock_irq(vha->host->host_lock);
 	return SCSI_MLQUEUE_TARGET_BUSY;
 
 qc24_fail_command:
-	spin_lock_irq(vha->host->host_lock);
-	done(cmd);
+	cmd->scsi_done(cmd);
 
 	return 0;
 }
 
-static DEF_SCSI_QCMD(qla2xxx_queuecommand)
-
-
 /*
  * qla2x00_eh_wait_on_command
  *    Waits for the command to be returned by the Firmware for some

Madhu Iyengar

2011-04-01 21:33 UTC #1 lore raw

Mathew,

We submitted the host_lock related changes earlier, and the changes were accepted and went into the 2.6.38-rc6 (or round-about that timeframe) upstream kernel.

Here's the patch in that sequence we submitted earlier (which is now in the upstream/scsi stream):
http://marc.info/?l=linux-scsi&m=129850384318495&w=2

Cheers,
Madhu

Quoted 103 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Matthew Wilcox
Sent: Friday, April 01, 2011 1:22 PM
To: [email protected]
Subject: [PATCH 3/3] qla2xxx: Run queuecommand outside the host_lock

The qla2xxx driver still uses the host_lock, but only to protect against some rare events like rport deletion.

Signed-off-by Matthew Wilcox <[email protected]>

drivers/scsi/qla2xxx/qla_os.c +1 -1

index f27724d..61340e1 100644
@@ -506,7 +506,7 @@ qla24xx_fw_version_str(struct scsi_qla_host *vha, char
 *str)
 
 static inline srb_t *
 qla2x00_get_new_sp(scsi_qla_host_t *vha, fc_port_t *fcport,
-    struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *))
+    struct scsi_cmnd *cmd)
 {
      srb_t *sp;

      struct qla_hw_data *ha = vha->hw;

drivers/scsi/qla2xxx/qla_os.c +2 -3

@@ -520,16 +520,15 @@ qla2x00_get_new_sp(scsi_qla_host_t *vha, fc_port_t
 *fcport,
      sp->cmd = cmd;
      sp->flags = 0;
      CMD_SP(cmd) = (void *)sp;
-     cmd->scsi_done = done;
      sp->ctx = NULL;
 
      return sp;
 }
 
 static int
-qla2xxx_queuecommand_lck(struct scsi_cmnd *cmd, void (*done)(struct
 scsi_cmnd *))
+qla2xxx_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *cmd)
 {
-     scsi_qla_host_t *vha = shost_priv(cmd->device->host);
+     scsi_qla_host_t *vha = shost_priv(shost);
      fc_port_t *fcport = (struct fc_port *) cmd->device->hostdata;

      struct fc_rport *rport = starget_to_rport(scsi_target(cmd->device));
      struct qla_hw_data *ha = vha->hw;

drivers/scsi/qla2xxx/qla_os.c +0 -1

@@ -537,7 +536,6 @@ qla2xxx_queuecommand_lck(struct scsi_cmnd *cmd, void
 (*done)(struct scsi_cmnd *)
      srb_t *sp;
      int rval;
 
-     spin_unlock_irq(vha->host->host_lock);
      if (ha->flags.eeh_busy) {
              if (ha->flags.pci_channel_io_perm_failure)

                      cmd->result = DID_NO_CONNECT << 16;

drivers/scsi/qla2xxx/qla_os.c +1 -3

@@ -569,7 +567,7 @@ qla2xxx_queuecommand_lck(struct scsi_cmnd *cmd, void
 (*done)(struct scsi_cmnd *)
              goto qc24_target_busy;
      }
 
-     sp = qla2x00_get_new_sp(base_vha, fcport, cmd, done);
+     sp = qla2x00_get_new_sp(base_vha, fcport, cmd);
      if (!sp)
              goto qc24_host_busy_lock;

@@ -577,8 +575,6 @@ qla2xxx_queuecommand_lck(struct scsi_cmnd *cmd, void
 (*done)(struct scsi_cmnd *)
      if (rval != QLA_SUCCESS)
              goto qc24_host_busy_free_sp;
 
-     spin_lock_irq(vha->host->host_lock);
-
      return 0;

qc24_host_busy_free_sp:

drivers/scsi/qla2xxx/qla_os.c +1 -8

@@ -586,23 +582,17 @@ qc24_host_busy_free_sp:
      mempool_free(sp, ha->srb_mempool);
 
 qc24_host_busy_lock:
-     spin_lock_irq(vha->host->host_lock);
      return SCSI_MLQUEUE_HOST_BUSY;
 
 qc24_target_busy:
-     spin_lock_irq(vha->host->host_lock);
      return SCSI_MLQUEUE_TARGET_BUSY;
 
 qc24_fail_command:
-     spin_lock_irq(vha->host->host_lock);
-     done(cmd);
+     cmd->scsi_done(cmd);
 
      return 0;
 }
 
-static DEF_SCSI_QCMD(qla2xxx_queuecommand)
-
-
 /*
  * qla2x00_eh_wait_on_command
  *    Waits for the command to be returned by the Firmware for some
--

Quoted 3 lines

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

Matthew Wilcox

2011-04-02 03:31 UTC #2 lore raw

Quoted 6 lines · Madhu Iyengar

Mathew,
We submitted the host_lock related changes earlier, and the changes were accepted and went into the 2.6.38-rc6 (or round-about that timeframe) upstream kernel.
Here's the patch in that sequence we submitted earlier (which is now in the upstream/scsi stream):
http://marc.info/?l=linux-scsi&m=129850384318495&w=2

Ah, cool. You missed a step though:

	scsi_qla_host_t *vha = shost_priv(cmd->device->host);

That can just be:

	scsi_qla_host_t *vha = shost_priv(host);

Signature · 4 lines

Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Madhu Iyengar

2011-04-02 14:22 UTC #3 lore raw

Quoted 20 lines

-----Original Message-----
From: Matthew Wilcox [mailto:[email protected]]
Sent: Friday, April 01, 2011 8:31 PM
To: Madhu Iyengar
Cc: Matthew Wilcox; [email protected]
Subject: Re: [PATCH 3/3] qla2xxx: Run queuecommand outside the host_lock
Quoted 3 lines · Madhu Iyengar
Mathew,
We submitted the host_lock related changes earlier, and the changes were
accepted and went into the 2.6.38-rc6 (or round-about that timeframe) upstream kernel.
Quoted 2 lines
Here's the patch in that sequence we submitted earlier (which is now in
the upstream/scsi stream):
Quoted 1 line
http://marc.info/?l=linux-scsi&m=129850384318495&w=2

Quoted 8 lines

Ah, cool. You missed a step though:

      scsi_qla_host_t *vha = shost_priv(cmd->device->host);

That can just be:

      scsi_qla_host_t *vha = shost_priv(host);

Thanks for pointing it out (:-). We'll correct that when we submit our next patch set.

Quoted 5 lines

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."

Madhu Iyengar

2011-04-08 23:07 UTC #4 lore raw

Quoted 34 lines

-----Original Message-----
From: Madhu Iyengar
Sent: Saturday, April 02, 2011 7:22 AM
To: Matthew Wilcox
Cc: Matthew Wilcox; [email protected]; Madhu Iyengar
Subject: RE: [PATCH 3/3] qla2xxx: Run queuecommand outside the host_lock
Quoted 11 lines
-----Original Message-----
From: Matthew Wilcox [mailto:[email protected]]
Sent: Friday, April 01, 2011 8:31 PM
To: Madhu Iyengar
Cc: Matthew Wilcox; [email protected]
Subject: Re: [PATCH 3/3] qla2xxx: Run queuecommand outside the host_lock
Quoted 3 lines · Madhu Iyengar
Mathew,
We submitted the host_lock related changes earlier, and the changes
were
Quoted 4 lines
accepted and went into the 2.6.38-rc6 (or round-about that timeframe) upstream kernel.
Quoted 2 lines
Here's the patch in that sequence we submitted earlier (which is now
in
Quoted 7 lines
the upstream/scsi stream):
Quoted 1 line
http://marc.info/?l=linux-scsi&m=129850384318495&w=2
Ah, cool. You missed a step though:
    scsi_qla_host_t *vha = shost_priv(cmd->device->host);

Quoted 8 lines

Quoted 4 lines
That can just be:
    scsi_qla_host_t *vha = shost_priv(host);
Thanks for pointing it out (:-). We'll correct that when we submit our next patch set.

Matthew, By the way, forgot to ask. Shall I take the liberty in adding an "Acked-by" or "Reviewed-by" from you when I submit a patch with the above change? Please let me know, which one. Thanks in advance.

Here's the title of the patch, unless you prefer a different one:
qla2xxx: Minor change in queuecommand function.

Quoted 5 lines

Quoted 5 lines
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."