Skip to contentSkip to conversation

libsas: Convert to host_lock less w/ interrupts disabled externally

PATCH In review Nicholas A. Bellinger 2 participants 3 messages Nov 17, 2010

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 4/11 libsas: Convert to host_lock less w/ interrupts disabled externally

From: Nicholas Bellinger <[email protected]>

This patch converts the libsas queuecommand to run in host_lock less mode w/ the new IRQ_DISABLE_SCSI_QCMD() that disables interrupts while calling ->queuecommand() dispatch.

---
 drivers/scsi/libsas/sas_scsi_host.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/libsas/sas_scsi_host.c +2 -2
index 29251fa..595ed21 100644
@@ -189,7 +189,7 @@ int sas_queue_up(struct sas_task *task)
  * Note: XXX: Remove the host unlock/lock pair when SCSI Core can
  * call us without holding an IRQ spinlock...
  */
-static int sas_queuecommand_lck(struct scsi_cmnd *cmd,
+static int sas_queuecommand_irq_disable(struct scsi_cmnd *cmd,
 		     void (*scsi_done)(struct scsi_cmnd *))
 	__releases(host->host_lock)
 	__acquires(dev->sata_dev.ap->lock)
@@ -254,7 +254,7 @@ out:
 	return res;
 }
 
-DEF_SCSI_QCMD(sas_queuecommand)
+IRQ_DISABLE_SCSI_QCMD(sas_queuecommand)
 
 static void sas_eh_finish_cmd(struct scsi_cmnd *cmd)
 {
Signature · 1 line
1.7.2.3
#1 lore raw

drivers/scsi/libsas/sas_scsi_host.c int sas_queue_up(struct sas_task *task) · line 190

Quoted 3 lines
drivers/scsi/libsas/sas_scsi_host.c +0 -0
@@ -189,7 +189,7 @@ int sas_queue_up(struct sas_task *task)
  * Note: XXX: Remove the host unlock/lock pair when SCSI Core can
  * call us without holding an IRQ spinlock...

That comment can be removed now.

drivers/scsi/libsas/sas_scsi_host.c int sas_queue_up(struct sas_task *task) · line 192

Quoted 2 lines
drivers/scsi/libsas/sas_scsi_host.c +0 -1
  */
-static int sas_queuecommand_lck(struct scsi_cmnd *cmd,

That's now how the code looks like upstream.

drivers/scsi/libsas/sas_scsi_host.c int sas_queue_up(struct sas_task *task) · line 195

Quoted 4 lines
drivers/scsi/libsas/sas_scsi_host.c +1 -0
+static int sas_queuecommand_irq_disable(struct scsi_cmnd *cmd,
 		     void (*scsi_done)(struct scsi_cmnd *))
 	__releases(host->host_lock)
 	__acquires(dev->sata_dev.ap->lock)

The host_lock annotations are not incorrect, and the sata_dev ones never made sense.

drivers/scsi/libsas/sas_scsi_host.c out: · line 257

Quoted 6 lines
drivers/scsi/libsas/sas_scsi_host.c +1 -1
@@ -254,7 +254,7 @@ out:
 	return res;
 }
 
-DEF_SCSI_QCMD(sas_queuecommand)
+IRQ_DISABLE_SCSI_QCMD(sas_queuecommand)

And in addition this doesn't even seem correct to me. First thing sas_queuecommand upstream does is spin_unlock_irq on the host_lock which it expects to be held, then it does a spin_lock_irqsave for the sata case, or operations that do not appear to need a lock for the sas case.

#2 lore raw
Quoted 23 lines · Christoph Hellwig

Review int sas_queue_up(struct sas_task *task) · line 190

Quoted 3 lines
(quoted diff) +0 -0
@@ -189,7 +189,7 @@ int sas_queue_up(struct sas_task *task)
  * Note: XXX: Remove the host unlock/lock pair when SCSI Core can
  * call us without holding an IRQ spinlock...

That comment can be removed now.

Review int sas_queue_up(struct sas_task *task) · line 192

Quoted 2 lines
(quoted diff) +0 -1
  */
-static int sas_queuecommand_lck(struct scsi_cmnd *cmd,

That's now how the code looks like upstream.

Review int sas_queue_up(struct sas_task *task) · line 195

Quoted 4 lines
(quoted diff) +1 -0
+static int sas_queuecommand_irq_disable(struct scsi_cmnd *cmd,
 		     void (*scsi_done)(struct scsi_cmnd *))
 	__releases(host->host_lock)
 	__acquires(dev->sata_dev.ap->lock)

The host_lock annotations are not incorrect, and the sata_dev ones never made sense.

Quoted 4 lines
(quoted diff) +0 -0
@@ -254,7 +254,7 @@ out:
 	return res;
 }
 
Quoted 8 lines

Review out: · line 257

Quoted 2 lines
(quoted diff) +1 -1
-DEF_SCSI_QCMD(sas_queuecommand)
+IRQ_DISABLE_SCSI_QCMD(sas_queuecommand)

And in addition this doesn't even seem correct to me. First thing sas_queuecommand upstream does is spin_unlock_irq on the host_lock which it expects to be held, then it does a spin_lock_irqsave for the sata case, or operations that do not appear to need a lock for the sas case.

Ugh, this was the one patch that I carried forwad and had to apply by hand, and evidently I missed the some bits. Pushing this commit now..

Thanks,

Quoted 1 line

From 444d87126389286c869d10c947707c1d33855995 Mon Sep 17 00:00:00 2001

From: Nicholas Bellinger <[email protected]>
Date: Wed, 17 Nov 2010 22:38:23 +0000
Subject: [PATCH] libsas: Added dropped bits for host_lock-less operation

This patch fixes drop-lock_less usage, lockdep annotation, and comment changes that where dropped while moving the libsas forward.

---
 drivers/scsi/libsas/sas_scsi_host.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)
(quoted diff) +0 -0

b/drivers/scsi/libsas/sas_scsi_host.c
index 595ed21..011580f 100644

drivers/scsi/libsas/sas_scsi_host.c +0 -7
@@ -185,24 +185,17 @@ int sas_queue_up(struct sas_task *task)
 /**
  * sas_queuecommand -- Enqueue a command for processing
  * @parameters: See SCSI Core documentation
- *
- * Note: XXX: Remove the host unlock/lock pair when SCSI Core can
- * call us without holding an IRQ spinlock...
  */
 static int sas_queuecommand_irq_disable(struct scsi_cmnd *cmd,
                     void (*scsi_done)(struct scsi_cmnd *))
-       __releases(host->host_lock)
        __acquires(dev->sata_dev.ap->lock)
        __releases(dev->sata_dev.ap->lock)
-       __acquires(host->host_lock)
 {
        int res = 0;
        struct domain_device *dev = cmd_to_domain_dev(cmd);
        struct Scsi_Host *host = cmd->device->host;
        struct sas_internal *i = to_sas_internal(host->transportt);
 
-       spin_unlock_irq(host->host_lock);
-
        {
                struct sas_ha_struct *sas_ha = dev->port->ha;
                struct sas_task *task;
@@ -250,7 +243,6 @@ static int sas_queuecommand_irq_disable(struct

scsi_cmnd *cmd,

                }
        }
 out:
-       spin_lock_irq(host->host_lock);
        return res;

}

Signature · 1 line
1.7.2.3
Quoted 5 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