[PATCH 04/11] libsas: Convert to host_lock less w/ interrupts disabled externally

[PATCH 04/11] libsas: Convert to host_lock less w/ interrupts disabled externally

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

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.

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/scsi/libsas/sas_scsi_host.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 29251fa..a5ce08f 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -183,26 +183,22 @@ int sas_queue_up(struct sas_task *task)
 }
 
 /**
- * sas_queuecommand -- Enqueue a command for processing
+ * sas_queuecommand_irq_disable -- 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_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)
 	__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,11 +246,10 @@ static int sas_queuecommand_lck(struct scsi_cmnd *cmd,
 		}
 	}
 out:
-	spin_lock_irq(host->host_lock);
 	return res;
 }
 
-DEF_SCSI_QCMD(sas_queuecommand)
+IRQ_DISABLE_SCSI_QCMD(sas_queuecommand)
 
 static void sas_eh_finish_cmd(struct scsi_cmnd *cmd)
 {
-- 
1.7.2.3

[PATCH 04/11] libsas: Convert to host_lock less w/ interrupts disabled externally

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

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.

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/scsi/libsas/sas_scsi_host.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 29251fa..595ed21 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -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)
 {
-- 
1.7.2.3

Re: [PATCH 04/11] libsas: Convert to host_lock less w/ interrupts disabled externally

[Christoph Hellwig]

> @@ -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.

>   */
> -static int sas_queuecommand_lck(struct scsi_cmnd *cmd,

That's now how the code looks like upstream.

> +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.

> @@ -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.

Re: [PATCH 04/11] libsas: Convert to host_lock less w/ interrupts disabled externally

[Nicholas A. Bellinger]

On Wed, 2010-11-17 at 17:30 -0500, Christoph Hellwig wrote:
> > @@ -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.
> 
> >   */
> > -static int sas_queuecommand_lck(struct scsi_cmnd *cmd,
> 
> That's now how the code looks like upstream.
> 
> > +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.
> 
> > @@ -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.

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,

>From 444d87126389286c869d10c947707c1d33855995 Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger <nab@linux-iscsi.org>
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.

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
Reported-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/libsas/sas_scsi_host.c |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libsas/sas_scsi_host.c
b/drivers/scsi/libsas/sas_scsi_host.c
index 595ed21..011580f 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -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;
 }

-- 
1.7.2.3


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