<Trimming CC list>
On Thu, 2010-11-18 at 12:15 +0200, Boaz Harrosh wrote:
> On 11/18/2010 12:19 AM, Nicholas A. Bellinger wrote:
> > From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
> >
> > This patch converts the mpt2sas driver to run in host_lock less mode
> > with the new IRQ_DISABLE_SCSI_QCMD() that disables interrupts while
> > calling ->queuecommand() dispatch
> >
> > Signed-off-by: Nicholas A. Bellinger <nab@xxxxxxxxxxxxxxx>
> > ---
> > drivers/scsi/mpt2sas/mpt2sas_scsih.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> > index 1a96a00..e564fe7 100644
> > --- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> > +++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> > @@ -3304,7 +3304,7 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 ioc_status)
> > }
> >
> > /**
> > - * _scsih_qcmd - main scsi request entry point
> > + * _scsih_qcmd_irq_disable - main scsi request entry point
> > * @scmd: pointer to scsi command object
> > * @done: function pointer to be invoked on completion
> > *
> > @@ -3315,7 +3315,7 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 ioc_status)
> > * SCSI_MLQUEUE_HOST_BUSY if the entire host queue is full
> > */
> > static int
> > -_scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
> > +_scsih_qcmd_irq_disable(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
> > {
> > struct MPT2SAS_ADAPTER *ioc = shost_priv(scmd->device->host);
> > struct MPT2SAS_DEVICE *sas_device_priv_data;
> > @@ -3441,7 +3441,7 @@ _scsih_qcmd_lck(struct scsi_cmnd *scmd, void (*done)(struct scsi_cmnd *))
> > return SCSI_MLQUEUE_HOST_BUSY;
> > }
> >
> > -static DEF_SCSI_QCMD(_scsih_qcmd)
> > +static IRQ_DISABLE_SCSI_QCMD(_scsih_qcmd)
> >
>
> How can this (and other in the patchset) can be correct? I mean I expect
> that if you remove the lock,xx_qcmd_lck,unlock then inside the xx_qcmd_lck
> there was an unlock,do123,lock and that driver was effectively running lockless
> before. (like in iscsi). But here this is new behaviour. If it is correct
> I would like to see a statement from you that:
> "I have audited this driver, and all shared resources are protected by
> XYZ so ..."
>
> Otherwise how can I know this is correct? I have never audited this driver myself
>
So for this specific mpt2sas case, Vasu Dev had been testing the
lock_less case w/o disabling interrupts for his original
SHT->unlocked_qcmd=1 patch, and from his comments this mode was stable
during his JBOD lock_less small block IOP performance test.
This patch currently includes IRQ_DISABLE_SCSI_QCMD() usage which I am
not even sure is needed (according to Vasu's original findings), but I
decided to err on the conservative side until we can get some feedback
from the the mpt2sas maintainer (Kashyap @ LSI CC'ed).
--nab
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
