Re: [PATCH 04/12] IB/srp: Fix connection state tracking

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2787 bytes --]

On Wed, 2015-05-06 at 11:29 +0200, Bart Van Assche wrote:
> Hello Doug,
> 
> On 05/05/15 18:10, Doug Ledford wrote:
> > Be that as it may, that doesn't change what I said about posting a
> > command to a known disconnected QP.  You could just fail immediately.
> > Something like:
> >
> > if (!ch->connected) {
> > 	scmnd->result = DID_NO_CONNECT;
> > 	goto err;
> > }
> >
> > right after getting the channel in queuecommand would work.  That would
> > save a couple spinlocks, several DMA mappings, a call into the low level
> > driver, and a few other things.  (And I only left requeue on the table
> > because I wasn't sure how the blk_mq dealt with just a single channel
> > being down versus all of them being down)
> 
> What you wrote above looks correct to me. However, it is intentional 
> that such a check is not present in srp_queuecommand(). The intention 
> was to optimize the hot path of that driver as much as possible. Hence 
> the choice to post a work request on the QP even after it has been 
> disconnected and to let the HCA generate an error completion.

Given the amount of time it takes to do all of the dma mapping in that
function on the normal hot path, I suspect the above test, especially if
you added an unlikely annotation, would not even make a blip on your
performance numbers.  In any case, it's not something I would require in
the patch.

> > But my point in all of this is that if you have a single qp between
> > yourself and the target, then any error including a qp resource error ==
> > path error since you only have one path.  When you have a multi queue
> > device, that's no longer true.  A transient resource problem on one qp
> > does not mean a path event (at least not necessarily, although your
> > statement below converts a QP event into a path event by virtue
> > disconnecting and reconnecting all of the QPs).  My curiosity is now
> > moot given what you wrote about tearing everything down and reconnecting
> > (unless the error handling is modified to be more subtle in its
> > workings), but the original question in my mind was what happens at the
> > blk_mq level if you did have a single queue drop but not all of them and
> > you weren't using multipath.
> 
> If we want to support this without adding similar code to handle this in 
> every SCSI LLD I think we need to change first how blk-mq and 
> dm-multipath interact. Today dm-multipath is a layer on top of blk-mq. 
> Supporting the above scenario properly is possible e.g. by integrating 
> multipath support in the blk-mq layer. I think Hannes and Christoph have 
> already started to work on this.

Ok.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]