Re: [ANNOUNCE] Status of unlocked_qcmds=1 operation for .37

[James Bottomley <James.Bottomley@suse.de>]

On Tue, 2010-10-26 at 15:34 -0700, Nicholas A. Bellinger wrote:
> On Tue, 2010-10-26 at 17:27 -0500, James Bottomley wrote:
> > On Tue, 2010-10-26 at 15:08 -0700, Nicholas A. Bellinger wrote:
> > > [PATCH] scsi: Add SCSI_EH_SOFTIRQ_DONE usage
> > > 
> > > This patch introduces a SCSI_EH_SOFTIRQ_DONE flag that is set in scsi_softirq_done()
> > > from block soft_irq context that is used to signal when scsi_try_to_abort_cmd() should
> > > be calling __scsi_try_to_abort_cmd() for a timed out struct scsi_cmnd instead of
> > > returning SUCCESS via checking only blk_test_rq_complete().  This is done because
> > > blk_rq_timed_out_timer() calls blk_mark_rq_complete() before blk_rq_timed_out() ->
> > > struct request_queue->rq_timed_out_fn().
> > 
> > This is getting pretty far off into the weeds.  I think the first step
> > should be queue lock push down into ->queuecommand.  This would still
> > necessitate locking around the serial number.
> 
> Hmmm, I am not sure I understand what you mean here.  My understanding
> is that the whole point of the series was to remove any locking around
> the serial number in order to make scsi_dispatch_cmd() lockless,
> right..?

The point is that several things have to happen for that to be a
reality.  The easiest and most obvious thing is lock push down in
->queuecommand.

The next is most likely serial number elimination.

But the point is that we don't have to do the whole thing all at once
(and spend months trying to get the series right).

> That is what the current patch series already does, in that it makes the
> use of cmd->serial_number optional and requires LLDs who use this for
> anything beyond an informational purpose to explictly call
> scsi_cmd_get_serial(). 

OK, so this patch is a corner case where the error handler is using the
serial number value to deduce something the block layer already knows
(whether the command completed or not). I don't think introducing a
substitute flag is the right way, the information should just be
extracted properly.

But arguments about this don't have to impede the lock push down.

James

> > The next step might be
> > serial number elimination/reduction which might need to address issues
> > like this (or not ... block already knows the information, there's not
> > really much need for us to track it twice).  There might also be other
> > lock aquisition reduction as part of step 2.
> > 
> 
> Correct, with this series cmd->serial_number still exists, but is
> completly optional and no longer used for any SCSI ML functionality.  It
> remains used only beyond an information purpose by a handful of LLDs in
> their legacy error recovery handling, which have been updated in this
> series to explictly call scsi_cmd_get_serial().
> 
> So other than those handful special cases, all of the LLDs included in
> the series which explict set SHT->unlocked_qcmd=1 to run in
> scsi_dispatch_cmd() lockless mode will no longer need ->serial_number.
> 
> --nab
> 
> --
> 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