[PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Nicholas A. Bellinger]

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

Greetings Hannes, Bo and Co,

This series converts megaraid_sas to run in modern host_lock-less mode
for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
and is intended for .38 mainline code.

The first patch adds a handful of missing barriers around instance->fw_outstanding
usage w/ atomic_add() and atomic_dec().

The second converts instance->issuepend_done to an atomic_t, along with
the necessary assignments in order to run w/ Scsi_Host->host_lock, and
without instance->hba_lock.

The third patch does the actual conversion, and adds a __megasas_get_cmd()
usased by megasas_queue_command() w/ instance->hba_lock, along with
being held for megasas_build_ldio() and megasas_build_dcdb() in order
to locate the proper frame for struct megasas_cmd.  This is really the
one major change in order to get host_lock-less to function with interrupts
disabled around hba_lock.

So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
environment.  This has not been tested on real silicon yet, but I believe
this series should be working there as well.

Comments are welcome, thanks!

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>

Nicholas Bellinger (3):
  megaraid_sas: Add smp_mb__after_atomic_*() for
    instance->fw_outstanding
  megaraid_sas: Convert instance->issuepend_done to atomic_t
  megaraid_sas: Convert SHT->queuecommand() to run host_lock-less

 drivers/scsi/megaraid/megaraid_sas.c |   84 ++++++++++++++++++++++-----------
 drivers/scsi/megaraid/megaraid_sas.h |    2 +-
 2 files changed, 57 insertions(+), 29 deletions(-)

-- 
1.7.2.3

[PATCH 1/3] megaraid_sas: Add smp_mb__after_atomic_*() for instance->fw_outstanding

[Nicholas A. Bellinger]

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

This patch adds four missing smp_mb__after_atomic_[inc,dec] barriers
for atomic_inc() and atomic_dec() usage with instance->fw_outstanding
within the main I/O dispatcher megasas_queue_command_lck(), completion
callback megasas_complete_cmd() and struct megasas_instance HW reset
queue drain in megasas_issue_pending_cmds_again().

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

diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
index 7451bc0..c103554 100644
--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -1398,6 +1398,7 @@ megasas_queue_command_lck(struct scsi_cmnd *scmd, void (*done) (struct scsi_cmnd
 	 * Issue the command to the FW
 	 */
 	atomic_inc(&instance->fw_outstanding);
+	smp_mb__after_atomic_inc();	
 
 	instance->instancet->fire_cmd(instance, cmd->frame_phys_addr,
 				cmd->frame_count-1, instance->reg_set);
@@ -2068,6 +2069,7 @@ megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 		if (exception) {
 
 			atomic_dec(&instance->fw_outstanding);
+			smp_mb__after_atomic_dec();
 
 			scsi_dma_unmap(cmd->scmd);
 			cmd->scmd->scsi_done(cmd->scmd);
@@ -2116,6 +2118,7 @@ megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 		}
 
 		atomic_dec(&instance->fw_outstanding);
+		smp_mb__after_atomic_dec();
 
 		scsi_dma_unmap(cmd->scmd);
 		cmd->scmd->scsi_done(cmd->scmd);
@@ -2219,6 +2222,7 @@ megasas_issue_pending_cmds_again(struct megasas_instance *instance)
 			cmd, cmd->scmd->cmnd[0], cmd->scmd->serial_number);
 
 			atomic_inc(&instance->fw_outstanding);
+			smp_mb__after_atomic_inc();
 			instance->instancet->fire_cmd(instance,
 					cmd->frame_phys_addr,
 					cmd->frame_count-1, instance->reg_set);
-- 
1.7.2.3

[PATCH 2/3] megaraid_sas: Convert instance->issuepend_done to atomic_t

[Nicholas A. Bellinger]

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

This patch converts struct megasas_instance->issuepend_done to
an atomic_t fw_issuepend_done.  This is because ->issuepend_done is
being used as a check to determine if SCSI_MLQUEUE_HOST_BUSY should
be returned during megasas_queue_command_lck().  So in order for a
conversion to lock-less operation to occur, this bit needs to be
handled in an atomic manner.

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/scsi/megaraid/megaraid_sas.c |   13 ++++++-------
 drivers/scsi/megaraid/megaraid_sas.h |    2 +-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
index c103554..fca9713 100644
--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -1344,7 +1344,7 @@ megasas_queue_command_lck(struct scsi_cmnd *scmd, void (*done) (struct scsi_cmnd
 	instance = (struct megasas_instance *)
 	    scmd->device->host->hostdata;
 
-	if (instance->issuepend_done == 0)
+	if (atomic_read(&instance->fw_issuepend_done) == 0)
 		return SCSI_MLQUEUE_HOST_BUSY;
 
 	spin_lock_irqsave(&instance->hba_lock, flags);
@@ -1590,8 +1590,7 @@ void megasas_do_ocr(struct megasas_instance *instance)
 	}
 	instance->instancet->disable_intr(instance->reg_set);
 	instance->adprecovery   = MEGASAS_ADPRESET_SM_INFAULT;
-	instance->issuepend_done = 0;
-
+	atomic_set(&instance->fw_issuepend_done, 0);
 	atomic_set(&instance->fw_outstanding, 0);
 	megasas_internal_reset_defer_cmds(instance);
 	process_fw_state_change_wq(&instance->work_init);
@@ -1941,7 +1940,7 @@ megasas_service_aen(struct megasas_instance *instance, struct megasas_cmd *cmd)
 	megasas_return_cmd(instance, cmd);
 
 	if ((instance->unload == 0) &&
-		((instance->issuepend_done == 1))) {
+		((atomic_read(&instance->fw_issuepend_done) == 1))) {
 		struct megasas_aen_event *ev;
 		ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
 		if (!ev) {
@@ -2357,7 +2356,7 @@ process_fw_state_change_wq(struct work_struct *work)
 		instance->instancet->enable_intr(instance->reg_set);
 
 		megasas_issue_pending_cmds_again(instance);
-		instance->issuepend_done = 1;
+		atomic_set(&instance->fw_issuepend_done, 1);
 	}
 	return ;
 }
@@ -2417,8 +2416,8 @@ megasas_deplete_reply_queue(struct megasas_instance *instance,
 
 			instance->instancet->disable_intr(instance->reg_set);
 			instance->adprecovery	= MEGASAS_ADPRESET_SM_INFAULT;
-			instance->issuepend_done = 0;
 
+			atomic_set(&instance->fw_issuepend_done, 0);
 			atomic_set(&instance->fw_outstanding, 0);
 			megasas_internal_reset_defer_cmds(instance);
 
@@ -3798,7 +3797,7 @@ megasas_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	megasas_poll_wait_aen = 0;
 	instance->flag_ieee = 0;
 	instance->ev = NULL;
-	instance->issuepend_done = 1;
+	atomic_set(&instance->fw_issuepend_done, 1);
 	instance->adprecovery = MEGASAS_HBA_OPERATIONAL;
 	megasas_poll_wait_aen = 0;
 
diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h
index ad16f5e..eaeb873 100644
--- a/drivers/scsi/megaraid/megaraid_sas.h
+++ b/drivers/scsi/megaraid/megaraid_sas.h
@@ -1303,6 +1303,7 @@ struct megasas_instance {
 
 	atomic_t fw_outstanding;
 	atomic_t fw_reset_no_pci_access;
+	atomic_t fw_issuepend_done;
 
 	struct megasas_instance_template *instancet;
 	struct tasklet_struct isr_tasklet;
@@ -1311,7 +1312,6 @@ struct megasas_instance {
 	u8 flag;
 	u8 unload;
 	u8 flag_ieee;
-	u8 issuepend_done;
 	u8 disableOnlineCtrlReset;
 	u8 adprecovery;
 	unsigned long last_time;
-- 
1.7.2.3

[PATCH 3/3] megaraid_sas: Convert SHT->queuecommand() to run host_lock-less

[Nicholas A. Bellinger]

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

This patch converts megasas_queue_command_lck -> to megasas_queue_command
running in modern host_lock-less mode.  This includes running using
struct megasas_instance->hba_lock to disable interrupts around existing
callers in megasas_queue_command().  In the existing code the
megasas_instance->hba_lock is taken to disable interrupts for

*) Setup of megasas_cmd for scsi_cmnd descriptor in megasas_get_cmd()

*) Issuing megasas_cmd to firmware via instance->instancet->fire_cmd()

Along with commit 1ee7ec10436 and 748925014d to make the following
megasas_instance members use proper atomic_t values:

*) fw_issuepend_done: used to signal SCSI_MLQUEUE_HOST_BUSY during
                     ->queuecommand(), and used during shutdown)

*) fw_outstanding:  used to track number outstanding megasas_cmd
                    descriptors

The one part of the code not originaly held, but has now been converted
to hold instance->hba_lock w/ interrupts disabled megasas_build_ldio()
and megasas_build_dcdb().  This part of code was originally protected
by Scsi_Host->host_lock disabling interrupts.

This allows for megaraid_sas to run in host_lock less dispatch
w/ disabling interrupts around internal megasas_instance->hba_lock.
So far this is functioning with QEMU Megasas 8708EM2 HBA emulation
with SG_IO into KVM Guest from TCM_Loop backstores on a .37-rc3 KVM
x86_64 Host.

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
---
 drivers/scsi/megaraid/megaraid_sas.c |   67 +++++++++++++++++++++++-----------
 1 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
index fca9713..2a5345c 100644
--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -122,16 +122,17 @@ megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 		     u8 alt_status);
 
 /**
- * megasas_get_cmd -	Get a command from the free pool
+ * __megasas_get_cmd -	Get a command from the free pool
  * @instance:		Adapter soft state
  *
  * Returns a free command from the pool
+ * Locking: Called with instance->cmd_pool_lock w/ spin_lock_irqsave
  */
-static struct megasas_cmd *megasas_get_cmd(struct megasas_instance
-						  *instance)
+static struct megasas_cmd *
+__megasas_get_cmd(struct megasas_instance *instance)
 {
+	struct megasas_cmd *cmd;
 	unsigned long flags;
-	struct megasas_cmd *cmd = NULL;
 
 	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
 
@@ -143,7 +144,25 @@ static struct megasas_cmd *megasas_get_cmd(struct megasas_instance
 		printk(KERN_ERR "megasas: Command pool empty!\n");
 	}
 
+	return cmd;
+}
+
+/**
+ * megasas_get_cmd -    Get a command from the free pool
+ * @instance:           Adapter soft state
+ *
+ * Returns a free command from the pool
+ */
+static struct megasas_cmd *
+megasas_get_cmd(struct megasas_instance *instance)
+{
+	struct megasas_cmd *cmd;
+	unsigned long flags;
+
+	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
+	cmd = __megasas_get_cmd(instance);
 	spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
+
 	return cmd;
 }
 
@@ -1332,9 +1351,18 @@ megasas_dump_pending_frames(struct megasas_instance *instance)
  * megasas_queue_command -	Queue entry point
  * @scmd:			SCSI command to be queued
  * @done:			Callback entry point
+ *
+ * This is the main Linux/SCSI per I/O cmd dispatcher that is running
+ * in 'host_lock-less' mode w/o the legacy Scsi_Host->host_Lock held,
+ * and with IRQs enabled.
+ *
+ * This means that interaction with FW requires the instance->hba_lock
+ * be held with interrupts disabled, or that other megasas_instance
+ * reference must be done in an atomic fashion in modern host_lock-less
+ * mode.
  */
 static int
-megasas_queue_command_lck(struct scsi_cmnd *scmd, void (*done) (struct scsi_cmnd *))
+megasas_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *scmd)
 {
 	u32 frame_count;
 	struct megasas_cmd *cmd;
@@ -1347,23 +1375,18 @@ megasas_queue_command_lck(struct scsi_cmnd *scmd, void (*done) (struct scsi_cmnd
 	if (atomic_read(&instance->fw_issuepend_done) == 0)
 		return SCSI_MLQUEUE_HOST_BUSY;
 
-	spin_lock_irqsave(&instance->hba_lock, flags);
-	if (instance->adprecovery != MEGASAS_HBA_OPERATIONAL) {
-		spin_unlock_irqrestore(&instance->hba_lock, flags);
-		return SCSI_MLQUEUE_HOST_BUSY;
-	}
-
-	spin_unlock_irqrestore(&instance->hba_lock, flags);
-
-	scmd->scsi_done = done;
-	scmd->result = 0;
-
 	if (MEGASAS_IS_LOGICAL(scmd) &&
 	    (scmd->device->id >= MEGASAS_MAX_LD || scmd->device->lun)) {
 		scmd->result = DID_BAD_TARGET << 16;
 		goto out_done;
 	}
 
+	spin_lock_irqsave(&instance->hba_lock, flags);
+	if (instance->adprecovery != MEGASAS_HBA_OPERATIONAL) {
+		spin_unlock_irqrestore(&instance->hba_lock, flags);
+		return SCSI_MLQUEUE_HOST_BUSY;
+	}
+
 	switch (scmd->cmnd[0]) {
 	case SYNCHRONIZE_CACHE:
 		/*
@@ -1371,14 +1394,17 @@ megasas_queue_command_lck(struct scsi_cmnd *scmd, void (*done) (struct scsi_cmnd
 		 * No need to send it down
 		 */
 		scmd->result = DID_OK << 16;
+		spin_unlock_irqrestore(&instance->hba_lock, flags);
 		goto out_done;
 	default:
 		break;
 	}
 
-	cmd = megasas_get_cmd(instance);
-	if (!cmd)
+	cmd = __megasas_get_cmd(instance);
+	if (!cmd) {
+		spin_unlock_irqrestore(&instance->hba_lock, flags);
 		return SCSI_MLQUEUE_HOST_BUSY;
+	}
 
 	/*
 	 * Logical drive command
@@ -1387,6 +1413,7 @@ megasas_queue_command_lck(struct scsi_cmnd *scmd, void (*done) (struct scsi_cmnd
 		frame_count = megasas_build_ldio(instance, scmd, cmd);
 	else
 		frame_count = megasas_build_dcdb(instance, scmd, cmd);
+	spin_unlock_irqrestore(&instance->hba_lock, flags);
 
 	if (!frame_count)
 		goto out_return_cmd;
@@ -1414,12 +1441,10 @@ megasas_queue_command_lck(struct scsi_cmnd *scmd, void (*done) (struct scsi_cmnd
  out_return_cmd:
 	megasas_return_cmd(instance, cmd);
  out_done:
-	done(scmd);
+	scmd->scsi_done(scmd);
 	return 0;
 }
 
-static DEF_SCSI_QCMD(megasas_queue_command)
-
 static struct megasas_instance *megasas_lookup_instance(u16 host_no)
 {
 	int i;
-- 
1.7.2.3

Re: [PATCH 3/3] megaraid_sas: Convert SHT->queuecommand() to run host_lock-less

[Christoph Hellwig]

On Sat, Nov 27, 2010 at 11:17:36PM -0800, Nicholas A. Bellinger wrote:
>  /**
> - * megasas_get_cmd -	Get a command from the free pool
> + * __megasas_get_cmd -	Get a command from the free pool
>   * @instance:		Adapter soft state
>   *
>   * Returns a free command from the pool
> + * Locking: Called with instance->cmd_pool_lock w/ spin_lock_irqsave
>   */
> -static struct megasas_cmd *megasas_get_cmd(struct megasas_instance
> -						  *instance)
> +static struct megasas_cmd *
> +__megasas_get_cmd(struct megasas_instance *instance)
>  {
> +	struct megasas_cmd *cmd;
>  	unsigned long flags;
> -	struct megasas_cmd *cmd = NULL;
>  
>  	spin_lock_irqsave(&instance->cmd_pool_lock, flags);

--- snip ---

> +static struct megasas_cmd *
> +megasas_get_cmd(struct megasas_instance *instance)
> +{
> +	struct megasas_cmd *cmd;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&instance->cmd_pool_lock, flags);

I can't see how this works.

Re: [PATCH 3/3] megaraid_sas: Convert SHT->queuecommand() to run host_lock-less

[Nicholas A. Bellinger]

On Mon, 2010-11-29 at 17:41 +0100, Christoph Hellwig wrote:
> On Sat, Nov 27, 2010 at 11:17:36PM -0800, Nicholas A. Bellinger wrote:
> >  /**
> > - * megasas_get_cmd -	Get a command from the free pool
> > + * __megasas_get_cmd -	Get a command from the free pool
> >   * @instance:		Adapter soft state
> >   *
> >   * Returns a free command from the pool
> > + * Locking: Called with instance->cmd_pool_lock w/ spin_lock_irqsave
> >   */
> > -static struct megasas_cmd *megasas_get_cmd(struct megasas_instance
> > -						  *instance)
> > +static struct megasas_cmd *
> > +__megasas_get_cmd(struct megasas_instance *instance)
> >  {
> > +	struct megasas_cmd *cmd;
> >  	unsigned long flags;
> > -	struct megasas_cmd *cmd = NULL;
> >  
> >  	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
> 
> --- snip ---
> 
> > +static struct megasas_cmd *
> > +megasas_get_cmd(struct megasas_instance *instance)
> > +{
> > +	struct megasas_cmd *cmd;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
> 
> I can't see how this works.
> 

Ugh, I had to recode patch #3 after accidently git resetting on the
wrong branch..

Thanks alot for spotting this, fixed with:

[lock_less-LLDs-for-38-v2 13a4cd2] megaraid_sas: Remove bogus hba_lock __megasas_get_cmd()
 1 files changed, 0 insertions(+), 3 deletions(-)

Thanks!

--nab

Re: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Nicholas A. Bellinger]

On Sat, 2010-11-27 at 23:17 -0800, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Greetings Hannes, Bo and Co,
> 
> This series converts megaraid_sas to run in modern host_lock-less mode
> for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
> This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
> and is intended for .38 mainline code.
> 
> The first patch adds a handful of missing barriers around instance->fw_outstanding
> usage w/ atomic_add() and atomic_dec().
> 
> The second converts instance->issuepend_done to an atomic_t, along with
> the necessary assignments in order to run w/ Scsi_Host->host_lock, and
> without instance->hba_lock.
> 
> The third patch does the actual conversion, and adds a __megasas_get_cmd()
> usased by megasas_queue_command() w/ instance->hba_lock, along with
> being held for megasas_build_ldio() and megasas_build_dcdb() in order
> to locate the proper frame for struct megasas_cmd.  This is really the
> one major change in order to get host_lock-less to function with interrupts
> disabled around hba_lock.
> 
> So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
> TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
> environment.  This has not been tested on real silicon yet, but I believe
> this series should be working there as well.
> 

And quick screenshot running in KVM Host TCM_Loop host_lock-less mode
into KVM Guest megaraid_sas host_lock-less mode w/ small block LTP
disktest.

http://www.linux-iscsi.org/index.php/File:TCM_loop-megasas-37-rc3.png

Just a FYI, with scsi-generic <-> TCM_Loop everything appears to be
stable for both small and large block size tests.

--nab

Re: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Hannes Reinecke]

On 11/28/2010 08:28 AM, Nicholas A. Bellinger wrote:
> On Sat, 2010-11-27 at 23:17 -0800, Nicholas A. Bellinger wrote:
>> From: Nicholas Bellinger <nab@linux-iscsi.org>
>>
>> Greetings Hannes, Bo and Co,
>>
>> This series converts megaraid_sas to run in modern host_lock-less mode
>> for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
>> This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
>> and is intended for .38 mainline code.
>>
>> The first patch adds a handful of missing barriers around instance->fw_outstanding
>> usage w/ atomic_add() and atomic_dec().
>>
>> The second converts instance->issuepend_done to an atomic_t, along with
>> the necessary assignments in order to run w/ Scsi_Host->host_lock, and
>> without instance->hba_lock.
>>
>> The third patch does the actual conversion, and adds a __megasas_get_cmd()
>> usased by megasas_queue_command() w/ instance->hba_lock, along with
>> being held for megasas_build_ldio() and megasas_build_dcdb() in order
>> to locate the proper frame for struct megasas_cmd.  This is really the
>> one major change in order to get host_lock-less to function with interrupts
>> disabled around hba_lock.
>>
>> So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
>> TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
>> environment.  This has not been tested on real silicon yet, but I believe
>> this series should be working there as well.
>>
> 
> And quick screenshot running in KVM Host TCM_Loop host_lock-less mode
> into KVM Guest megaraid_sas host_lock-less mode w/ small block LTP
> disktest.
> 
> http://www.linux-iscsi.org/index.php/File:TCM_loop-megasas-37-rc3.png
> 
> Just a FYI, with scsi-generic <-> TCM_Loop everything appears to be
> stable for both small and large block size tests.
> 
Oh, good. I did some initial tests with megaraid_sas & host_lock
disabled, but ran into the problems you mentioned.
And have been too lazy to pursue this problem further.
Thanks for your work; I'll give it a spin and see if the throughput
speed has increased.

Thanks for the patches.

Btw, I've found one issue wrt Windows7 booting; now we're not
crashing anymore. But we're not exactly progressing, either;
apparently Win7 fails to notify that we've finished the commands.
Continue debugging.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
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

Re: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Nicholas A. Bellinger]

On Mon, 2010-11-29 at 16:56 +0100, Hannes Reinecke wrote:
> On 11/28/2010 08:28 AM, Nicholas A. Bellinger wrote:
> > On Sat, 2010-11-27 at 23:17 -0800, Nicholas A. Bellinger wrote:
> >> From: Nicholas Bellinger <nab@linux-iscsi.org>
> >>
> >> Greetings Hannes, Bo and Co,
> >>
> >> This series converts megaraid_sas to run in modern host_lock-less mode
> >> for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
> >> This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
> >> and is intended for .38 mainline code.
> >>
> >> The first patch adds a handful of missing barriers around instance->fw_outstanding
> >> usage w/ atomic_add() and atomic_dec().
> >>
> >> The second converts instance->issuepend_done to an atomic_t, along with
> >> the necessary assignments in order to run w/ Scsi_Host->host_lock, and
> >> without instance->hba_lock.
> >>
> >> The third patch does the actual conversion, and adds a __megasas_get_cmd()
> >> usased by megasas_queue_command() w/ instance->hba_lock, along with
> >> being held for megasas_build_ldio() and megasas_build_dcdb() in order
> >> to locate the proper frame for struct megasas_cmd.  This is really the
> >> one major change in order to get host_lock-less to function with interrupts
> >> disabled around hba_lock.
> >>
> >> So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
> >> TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
> >> environment.  This has not been tested on real silicon yet, but I believe
> >> this series should be working there as well.
> >>
> > 
> > And quick screenshot running in KVM Host TCM_Loop host_lock-less mode
> > into KVM Guest megaraid_sas host_lock-less mode w/ small block LTP
> > disktest.
> > 
> > http://www.linux-iscsi.org/index.php/File:TCM_loop-megasas-37-rc3.png
> > 
> > Just a FYI, with scsi-generic <-> TCM_Loop everything appears to be
> > stable for both small and large block size tests.
> > 
> Oh, good. I did some initial tests with megaraid_sas & host_lock
> disabled, but ran into the problems you mentioned.
> And have been too lazy to pursue this problem further.
> Thanks for your work; I'll give it a spin and see if the throughput
> speed has increased.
> 
> Thanks for the patches.
> 

Aside from the extra __megasas_get_cmd() breakage, the conversion was
really quite straight-forward..  ;)

> Btw, I've found one issue wrt Windows7 booting; now we're not
> crashing anymore. But we're not exactly progressing, either;
> apparently Win7 fails to notify that we've finished the commands.
> Continue debugging.
> 

Ok, then we probably will need to verify that our v0.12.5-megasas in the
following qemu-kvm.git branch is still working as expected w/ your Win7
build:

windows7-megasas-working dde5ee2 [scsi-bus]: Add MAINTENANCE_IN and \
	MAINTENANCE_OUT case for SCSIRequest xfer and xfer_mode setup

Just for reference, this is back with the hardcoded defs for fw_sge and
fw_cmds:

/* Static definitions */
#define MEGASAS_MAX_FRAMES 1000
#define MEGASAS_MAX_SGE 8
#define MEGASAS_MAX_LUNS 128

Perhaps the larger default fw_sge in recent megasas emulation code is
causing an issue here..?

--nab

RE: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Yang, Bo]

Nicholas,

Thanks for your patches (patches 1 -3).  We need to build the driver and provide it to our testing team to do the full verification on lock_less kernel.  The patch 2 and patch 3 changed the driver's major routines (I/O, get/return cmds routines and OCR routine).  I will keep update after we did the internal patch re-view for those changes as well as the testing team give us the report.

By the way, can you send me the full link of: lio-core-2.6.git/lock_less-LLDs-for-38-v2?  I am try to understand the changes for patch 3


+	struct megasas_cmd *cmd;
 	unsigned long flags;
-	struct megasas_cmd *cmd = NULL;

And 

+megasas_get_cmd(struct megasas_instance *instance) {
+	struct megasas_cmd *cmd;
+	unsigned long flags;
+
+	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
+	cmd = __megasas_get_cmd(instance);
 	spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
+
 	return cmd;
 }     

Regards,

Bo Yang  

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Nicholas A. Bellinger
Sent: Sunday, November 28, 2010 2:18 AM
To: linux-scsi; Hannes Reinecke; Yang, Bo
Cc: Christoph Hellwig; James Bottomley; Nicholas Bellinger
Subject: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

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

Greetings Hannes, Bo and Co,

This series converts megaraid_sas to run in modern host_lock-less mode
for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
and is intended for .38 mainline code.

The first patch adds a handful of missing barriers around instance->fw_outstanding
usage w/ atomic_add() and atomic_dec().

The second converts instance->issuepend_done to an atomic_t, along with
the necessary assignments in order to run w/ Scsi_Host->host_lock, and
without instance->hba_lock.

The third patch does the actual conversion, and adds a __megasas_get_cmd()
usased by megasas_queue_command() w/ instance->hba_lock, along with
being held for megasas_build_ldio() and megasas_build_dcdb() in order
to locate the proper frame for struct megasas_cmd.  This is really the
one major change in order to get host_lock-less to function with interrupts
disabled around hba_lock.

So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
environment.  This has not been tested on real silicon yet, but I believe
this series should be working there as well.

Comments are welcome, thanks!

Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>

Nicholas Bellinger (3):
  megaraid_sas: Add smp_mb__after_atomic_*() for
    instance->fw_outstanding
  megaraid_sas: Convert instance->issuepend_done to atomic_t
  megaraid_sas: Convert SHT->queuecommand() to run host_lock-less

 drivers/scsi/megaraid/megaraid_sas.c |   84 ++++++++++++++++++++++-----------
 drivers/scsi/megaraid/megaraid_sas.h |    2 +-
 2 files changed, 57 insertions(+), 29 deletions(-)

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

RE: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Nicholas A. Bellinger]

On Mon, 2010-11-29 at 23:26 -0700, Yang, Bo wrote:
> Nicholas,
>
> Thanks for your patches (patches 1 -3).  We need to build the driver and provide it to our testing team to do
> the full verification on lock_less kernel.

> The patch 2 and patch 3 changed the driver's major routines (I/O, get/return cmds routines and OCR routine).
> I will keep update after we did the internal patch re-view for those changes as well as the testing team
> give us the report.
> 

Great, thanks for moving forward on the lock_less conversion..  Please
let me know if you have any other questions.

> By the way, can you send me the full link of: lio-core-2.6.git/lock_less-LLDs-for-38-v2?  I am try to understand the changes for patch 3


The patch #3 in full context is available here:

http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=blob;f=drivers/scsi/megaraid/megaraid_sas.c;h=c42e1e4ef8c9923d9a930fa631f8d65455afadd6;hb=lock_less-LLDs-for-38-v2

Thanks!

--nab


> +	struct megasas_cmd *cmd;
>  	unsigned long flags;
> -	struct megasas_cmd *cmd = NULL;
> 
> And 
> 
> +megasas_get_cmd(struct megasas_instance *instance) {
> +	struct megasas_cmd *cmd;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
> +	cmd = __megasas_get_cmd(instance);
>  	spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
> +
>  	return cmd;
>  }     
> 
> Regards,
> 
> Bo Yang  
> 
> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Nicholas A. Bellinger
> Sent: Sunday, November 28, 2010 2:18 AM
> To: linux-scsi; Hannes Reinecke; Yang, Bo
> Cc: Christoph Hellwig; James Bottomley; Nicholas Bellinger
> Subject: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally
> 
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Greetings Hannes, Bo and Co,
> 
> This series converts megaraid_sas to run in modern host_lock-less mode
> for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
> This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
> and is intended for .38 mainline code.
> 
> The first patch adds a handful of missing barriers around instance->fw_outstanding
> usage w/ atomic_add() and atomic_dec().
> 
> The second converts instance->issuepend_done to an atomic_t, along with
> the necessary assignments in order to run w/ Scsi_Host->host_lock, and
> without instance->hba_lock.
> 
> The third patch does the actual conversion, and adds a __megasas_get_cmd()
> usased by megasas_queue_command() w/ instance->hba_lock, along with
> being held for megasas_build_ldio() and megasas_build_dcdb() in order
> to locate the proper frame for struct megasas_cmd.  This is really the
> one major change in order to get host_lock-less to function with interrupts
> disabled around hba_lock.
> 
> So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
> TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
> environment.  This has not been tested on real silicon yet, but I believe
> this series should be working there as well.
> 
> Comments are welcome, thanks!
> 
> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> 
> Nicholas Bellinger (3):
>   megaraid_sas: Add smp_mb__after_atomic_*() for
>     instance->fw_outstanding
>   megaraid_sas: Convert instance->issuepend_done to atomic_t
>   megaraid_sas: Convert SHT->queuecommand() to run host_lock-less
> 
>  drivers/scsi/megaraid/megaraid_sas.c |   84 ++++++++++++++++++++++-----------
>  drivers/scsi/megaraid/megaraid_sas.h |    2 +-
>  2 files changed, 57 insertions(+), 29 deletions(-)
> 

RE: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Nicholas A. Bellinger]

On Tue, 2010-11-30 at 13:21 -0800, Nicholas A. Bellinger wrote:
> On Mon, 2010-11-29 at 23:26 -0700, Yang, Bo wrote:
> > Nicholas,
> >
> > Thanks for your patches (patches 1 -3).  We need to build the driver and provide it to our testing team to do
> > the full verification on lock_less kernel.
> 
> > The patch 2 and patch 3 changed the driver's major routines (I/O, get/return cmds routines and OCR routine).
> > I will keep update after we did the internal patch re-view for those changes as well as the testing team
> > give us the report.
> > 
> 
> Great, thanks for moving forward on the lock_less conversion..  Please
> let me know if you have any other questions.
> 
> > By the way, can you send me the full link of: lio-core-2.6.git/lock_less-LLDs-for-38-v2?  I am try to understand the changes for patch 3
> 
> 
> The patch #3 in full context is available here:
> 
> http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=blob;f=drivers/scsi/megaraid/megaraid_sas.c;h=c42e1e4ef8c9923d9a930fa631f8d65455afadd6;hb=lock_less-LLDs-for-38-v2
> 
> Thanks!
> 
> --nab
> 
> 

Hi Bo,

I also noticed that __megasas_get_cmd() still needs the initial *cmd =
NULL assignment.  Pushing the following into lock_less-LLDs-for-38-v2
now:

Subject: [PATCH] megaraid_sas: Add NULL assignment in __megasas_get_cmd()

This patch changes the initial struct megasas_cmd *cmd to NULL,
which follows the pre lock_less conversion code.

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

diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
index c42e1e4..be956a5 100644
--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -131,7 +131,7 @@ megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 static struct megasas_cmd *
 __megasas_get_cmd(struct megasas_instance *instance)
 {
-       struct megasas_cmd *cmd;
+       struct megasas_cmd *cmd = NULL;

        if (!list_empty(&instance->cmd_pool)) {
                cmd = list_entry((&instance->cmd_pool)->next,
-- 
1.7.2.3

Thanks!

--nab

> > +	struct megasas_cmd *cmd;
> >  	unsigned long flags;
> > -	struct megasas_cmd *cmd = NULL;
> > 
> > And 
> > 
> > +megasas_get_cmd(struct megasas_instance *instance) {
> > +	struct megasas_cmd *cmd;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
> > +	cmd = __megasas_get_cmd(instance);
> >  	spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
> > +
> >  	return cmd;
> >  }     
> > 
> > Regards,
> > 
> > Bo Yang  
> > 
> > -----Original Message-----
> > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Nicholas A. Bellinger
> > Sent: Sunday, November 28, 2010 2:18 AM
> > To: linux-scsi; Hannes Reinecke; Yang, Bo
> > Cc: Christoph Hellwig; James Bottomley; Nicholas Bellinger
> > Subject: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally
> > 
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > Greetings Hannes, Bo and Co,
> > 
> > This series converts megaraid_sas to run in modern host_lock-less mode
> > for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
> > This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
> > and is intended for .38 mainline code.
> > 
> > The first patch adds a handful of missing barriers around instance->fw_outstanding
> > usage w/ atomic_add() and atomic_dec().
> > 
> > The second converts instance->issuepend_done to an atomic_t, along with
> > the necessary assignments in order to run w/ Scsi_Host->host_lock, and
> > without instance->hba_lock.
> > 
> > The third patch does the actual conversion, and adds a __megasas_get_cmd()
> > usased by megasas_queue_command() w/ instance->hba_lock, along with
> > being held for megasas_build_ldio() and megasas_build_dcdb() in order
> > to locate the proper frame for struct megasas_cmd.  This is really the
> > one major change in order to get host_lock-less to function with interrupts
> > disabled around hba_lock.
> > 
> > So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
> > TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
> > environment.  This has not been tested on real silicon yet, but I believe
> > this series should be working there as well.
> > 
> > Comments are welcome, thanks!
> > 
> > Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> > 
> > Nicholas Bellinger (3):
> >   megaraid_sas: Add smp_mb__after_atomic_*() for
> >     instance->fw_outstanding
> >   megaraid_sas: Convert instance->issuepend_done to atomic_t
> >   megaraid_sas: Convert SHT->queuecommand() to run host_lock-less
> > 
> >  drivers/scsi/megaraid/megaraid_sas.c |   84 ++++++++++++++++++++++-----------
> >  drivers/scsi/megaraid/megaraid_sas.h |    2 +-
> >  2 files changed, 57 insertions(+), 29 deletions(-)
> > 
> 
> --
> 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

RE: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Yang, Bo]

Nicholas,

Thanks for put it back.  This set (to NULL) is in original megasas_get_cmd  src.  

Can you share your testing with us (how much test you did in lock_less kernel)?  We don't do any test with this kernel..

Thanks,

Bo Yang


-----Original Message-----
From: Nicholas A. Bellinger [mailto:nab@linux-iscsi.org] 
Sent: Tuesday, November 30, 2010 5:06 PM
To: Yang, Bo
Cc: linux-scsi; Hannes Reinecke; Christoph Hellwig; James Bottomley; Daftardar, Jayant
Subject: RE: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

On Tue, 2010-11-30 at 13:21 -0800, Nicholas A. Bellinger wrote:
> On Mon, 2010-11-29 at 23:26 -0700, Yang, Bo wrote:
> > Nicholas,
> >
> > Thanks for your patches (patches 1 -3).  We need to build the driver and provide it to our testing team to do
> > the full verification on lock_less kernel.
> 
> > The patch 2 and patch 3 changed the driver's major routines (I/O, get/return cmds routines and OCR routine).
> > I will keep update after we did the internal patch re-view for those changes as well as the testing team
> > give us the report.
> > 
> 
> Great, thanks for moving forward on the lock_less conversion..  Please
> let me know if you have any other questions.
> 
> > By the way, can you send me the full link of: lio-core-2.6.git/lock_less-LLDs-for-38-v2?  I am try to understand the changes for patch 3
> 
> 
> The patch #3 in full context is available here:
> 
> http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=blob;f=drivers/scsi/megaraid/megaraid_sas.c;h=c42e1e4ef8c9923d9a930fa631f8d65455afadd6;hb=lock_less-LLDs-for-38-v2
> 
> Thanks!
> 
> --nab
> 
> 

Hi Bo,

I also noticed that __megasas_get_cmd() still needs the initial *cmd =
NULL assignment.  Pushing the following into lock_less-LLDs-for-38-v2
now:

Subject: [PATCH] megaraid_sas: Add NULL assignment in __megasas_get_cmd()

This patch changes the initial struct megasas_cmd *cmd to NULL,
which follows the pre lock_less conversion code.

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

diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
index c42e1e4..be956a5 100644
--- a/drivers/scsi/megaraid/megaraid_sas.c
+++ b/drivers/scsi/megaraid/megaraid_sas.c
@@ -131,7 +131,7 @@ megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
 static struct megasas_cmd *
 __megasas_get_cmd(struct megasas_instance *instance)
 {
-       struct megasas_cmd *cmd;
+       struct megasas_cmd *cmd = NULL;

        if (!list_empty(&instance->cmd_pool)) {
                cmd = list_entry((&instance->cmd_pool)->next,
-- 
1.7.2.3

Thanks!

--nab

> > +	struct megasas_cmd *cmd;
> >  	unsigned long flags;
> > -	struct megasas_cmd *cmd = NULL;
> > 
> > And 
> > 
> > +megasas_get_cmd(struct megasas_instance *instance) {
> > +	struct megasas_cmd *cmd;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
> > +	cmd = __megasas_get_cmd(instance);
> >  	spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
> > +
> >  	return cmd;
> >  }     
> > 
> > Regards,
> > 
> > Bo Yang  
> > 
> > -----Original Message-----
> > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Nicholas A. Bellinger
> > Sent: Sunday, November 28, 2010 2:18 AM
> > To: linux-scsi; Hannes Reinecke; Yang, Bo
> > Cc: Christoph Hellwig; James Bottomley; Nicholas Bellinger
> > Subject: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally
> > 
> > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > 
> > Greetings Hannes, Bo and Co,
> > 
> > This series converts megaraid_sas to run in modern host_lock-less mode
> > for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
> > This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
> > and is intended for .38 mainline code.
> > 
> > The first patch adds a handful of missing barriers around instance->fw_outstanding
> > usage w/ atomic_add() and atomic_dec().
> > 
> > The second converts instance->issuepend_done to an atomic_t, along with
> > the necessary assignments in order to run w/ Scsi_Host->host_lock, and
> > without instance->hba_lock.
> > 
> > The third patch does the actual conversion, and adds a __megasas_get_cmd()
> > usased by megasas_queue_command() w/ instance->hba_lock, along with
> > being held for megasas_build_ldio() and megasas_build_dcdb() in order
> > to locate the proper frame for struct megasas_cmd.  This is really the
> > one major change in order to get host_lock-less to function with interrupts
> > disabled around hba_lock.
> > 
> > So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
> > TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
> > environment.  This has not been tested on real silicon yet, but I believe
> > this series should be working there as well.
> > 
> > Comments are welcome, thanks!
> > 
> > Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> > 
> > Nicholas Bellinger (3):
> >   megaraid_sas: Add smp_mb__after_atomic_*() for
> >     instance->fw_outstanding
> >   megaraid_sas: Convert instance->issuepend_done to atomic_t
> >   megaraid_sas: Convert SHT->queuecommand() to run host_lock-less
> > 
> >  drivers/scsi/megaraid/megaraid_sas.c |   84 ++++++++++++++++++++++-----------
> >  drivers/scsi/megaraid/megaraid_sas.h |    2 +-
> >  2 files changed, 57 insertions(+), 29 deletions(-)
> > 
> 
> --
> 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

RE: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally

[Nicholas A. Bellinger]

On Tue, 2010-11-30 at 15:30 -0700, Yang, Bo wrote:
> Nicholas,
> 
> Thanks for put it back.  This set (to NULL) is in original megasas_get_cmd  src.  
> 
> Can you share your testing with us (how much test you did in lock_less kernel)?
> We don't do any test with this kernel..
> 

Sure..   So I have been testing initially with RAMDISK_DR (for stressing
testing) and smaller sized FILEIO backstores w/ XFS for consistency.
For the stressing testing using LTP disktest, (4K) and large (128K)
block sizes to both RAMDISK_DR and FILEIO LUNs where used w/ Hannes's
8708EM2 emulation -> SG_IO into a lock-less megaraid_sas .37-rc3 KVM
guest.

These stress tests where running sustained on the order of hours without
any observed issues at 40K IOPs for 4K and ~1800 MB/sec for 128K.
Following the /proc/scsi_target/mib/scsi_lu statistics the test run was
on the order of 100,000,000 individual I/Os and ~900 GB of total SCSI
payload traffic in lock_less mode.

For the FILEIO tests, I created a small XFS filesystem and copied over a
number of binary packages and compared md5sums after an umount and
reboot, etc.  Also, running xfs_repair reported no issues for FS level
consistency.

--nab


> Thanks,
> 
> Bo Yang
> 
> 
> -----Original Message-----
> From: Nicholas A. Bellinger [mailto:nab@linux-iscsi.org] 
> Sent: Tuesday, November 30, 2010 5:06 PM
> To: Yang, Bo
> Cc: linux-scsi; Hannes Reinecke; Christoph Hellwig; James Bottomley; Daftardar, Jayant
> Subject: RE: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally
> 
> On Tue, 2010-11-30 at 13:21 -0800, Nicholas A. Bellinger wrote:
> > On Mon, 2010-11-29 at 23:26 -0700, Yang, Bo wrote:
> > > Nicholas,
> > >
> > > Thanks for your patches (patches 1 -3).  We need to build the driver and provide it to our testing team to do
> > > the full verification on lock_less kernel.
> > 
> > > The patch 2 and patch 3 changed the driver's major routines (I/O, get/return cmds routines and OCR routine).
> > > I will keep update after we did the internal patch re-view for those changes as well as the testing team
> > > give us the report.
> > > 
> > 
> > Great, thanks for moving forward on the lock_less conversion..  Please
> > let me know if you have any other questions.
> > 
> > > By the way, can you send me the full link of: lio-core-2.6.git/lock_less-LLDs-for-38-v2?  I am try to understand the changes for patch 3
> > 
> > 
> > The patch #3 in full context is available here:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/nab/lio-core-2.6.git;a=blob;f=drivers/scsi/megaraid/megaraid_sas.c;h=c42e1e4ef8c9923d9a930fa631f8d65455afadd6;hb=lock_less-LLDs-for-38-v2
> > 
> > Thanks!
> > 
> > --nab
> > 
> > 
> 
> Hi Bo,
> 
> I also noticed that __megasas_get_cmd() still needs the initial *cmd =
> NULL assignment.  Pushing the following into lock_less-LLDs-for-38-v2
> now:
> 
> Subject: [PATCH] megaraid_sas: Add NULL assignment in __megasas_get_cmd()
> 
> This patch changes the initial struct megasas_cmd *cmd to NULL,
> which follows the pre lock_less conversion code.
> 
> Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/scsi/megaraid/megaraid_sas.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas.c b/drivers/scsi/megaraid/megaraid_sas.c
> index c42e1e4..be956a5 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.c
> +++ b/drivers/scsi/megaraid/megaraid_sas.c
> @@ -131,7 +131,7 @@ megasas_complete_cmd(struct megasas_instance *instance, struct megasas_cmd *cmd,
>  static struct megasas_cmd *
>  __megasas_get_cmd(struct megasas_instance *instance)
>  {
> -       struct megasas_cmd *cmd;
> +       struct megasas_cmd *cmd = NULL;
> 
>         if (!list_empty(&instance->cmd_pool)) {
>                 cmd = list_entry((&instance->cmd_pool)->next,
> -- 
> 1.7.2.3
> 
> Thanks!
> 
> --nab
> 
> > > +	struct megasas_cmd *cmd;
> > >  	unsigned long flags;
> > > -	struct megasas_cmd *cmd = NULL;
> > > 
> > > And 
> > > 
> > > +megasas_get_cmd(struct megasas_instance *instance) {
> > > +	struct megasas_cmd *cmd;
> > > +	unsigned long flags;
> > > +
> > > +	spin_lock_irqsave(&instance->cmd_pool_lock, flags);
> > > +	cmd = __megasas_get_cmd(instance);
> > >  	spin_unlock_irqrestore(&instance->cmd_pool_lock, flags);
> > > +
> > >  	return cmd;
> > >  }     
> > > 
> > > Regards,
> > > 
> > > Bo Yang  
> > > 
> > > -----Original Message-----
> > > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Nicholas A. Bellinger
> > > Sent: Sunday, November 28, 2010 2:18 AM
> > > To: linux-scsi; Hannes Reinecke; Yang, Bo
> > > Cc: Christoph Hellwig; James Bottomley; Nicholas Bellinger
> > > Subject: [PATCH 0/3] megaraid_sas: Convert to host_lock less w/ interrupts disabled internally
> > > 
> > > From: Nicholas Bellinger <nab@linux-iscsi.org>
> > > 
> > > Greetings Hannes, Bo and Co,
> > > 
> > > This series converts megaraid_sas to run in modern host_lock-less mode
> > > for >= .37-rc3+ with interrupts disabled internally around megasas_instance->hba_lock.
> > > This series is currently living in lio-core-2.6.git/lock_less-LLDs-for-38-v2,
> > > and is intended for .38 mainline code.
> > > 
> > > The first patch adds a handful of missing barriers around instance->fw_outstanding
> > > usage w/ atomic_add() and atomic_dec().
> > > 
> > > The second converts instance->issuepend_done to an atomic_t, along with
> > > the necessary assignments in order to run w/ Scsi_Host->host_lock, and
> > > without instance->hba_lock.
> > > 
> > > The third patch does the actual conversion, and adds a __megasas_get_cmd()
> > > usased by megasas_queue_command() w/ instance->hba_lock, along with
> > > being held for megasas_build_ldio() and megasas_build_dcdb() in order
> > > to locate the proper frame for struct megasas_cmd.  This is really the
> > > one major change in order to get host_lock-less to function with interrupts
> > > disabled around hba_lock.
> > > 
> > > So far this has been tested with Hannes's QEMU 8708EM2 HBA emulation with
> > > TCM_Loop backends using SG_IO from KVM host in a paired Host/Guest .37-rc3
> > > environment.  This has not been tested on real silicon yet, but I believe
> > > this series should be working there as well.
> > > 
> > > Comments are welcome, thanks!
> > > 
> > > Signed-off-by: Nicholas A. Bellinger <nab@linux-iscsi.org>
> > > 
> > > Nicholas Bellinger (3):
> > >   megaraid_sas: Add smp_mb__after_atomic_*() for
> > >     instance->fw_outstanding
> > >   megaraid_sas: Convert instance->issuepend_done to atomic_t
> > >   megaraid_sas: Convert SHT->queuecommand() to run host_lock-less
> > > 
> > >  drivers/scsi/megaraid/megaraid_sas.c |   84 ++++++++++++++++++++++-----------
> > >  drivers/scsi/megaraid/megaraid_sas.h |    2 +-
> > >  2 files changed, 57 insertions(+), 29 deletions(-)
> > > 
> > 
> > --
> > 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
> 
> --
> 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