Re: [PATCH 10/24] scsi-multipath: add scsi_mpath_{start,end}_request()

[Benjamin Marzinski <bmarzins@redhat.com>]

On Wed, Mar 04, 2026 at 11:11:08AM +0000, John Garry wrote:
> On 04/03/2026 06:13, Benjamin Marzinski wrote:
> > > +	scsi_mpath_end_request(req);
> > > +
> > >   	/*
> > >   	 * In the MQ case the command gets freed by __blk_mq_end_request,
> > >   	 * so we have to do all cleanup that depends on it earlier.
> > This looks wrong. We start accounting in scsi_queue_rq(), and we need to
> > end it whenever we complete or requeue the request, otherwise the
> > accounting will get off. But not all requests go through
> > scsi_end_request(). scsi_mpath_failover_req(), for instance, calls
> > blk_mq_end_request() directly, and other functions, like
> > scsi_queue_insert() call blk_mq_requeue_request(). I'm pretty sure that
> > this should go in scsi_complete(), as well in the error path of
> > scsi_queue_rq().
> 
> ok, let me check that further.
> 

I think I was a little hasty here. Looking at sd_mpath_start_command()
and sd_mpath_end_command() in patch 17, I can see that they protect
against repeat calls, so requeueing the request should be o.k. There's
still a problem when scsi_mpath_failover_req() calls blk_mq_end_request()
directly, and when scsi_queue_rq() exits with a failure where the
request won't requeued (all the returns except BLK_STS_OK,
BLK_STS_RESOURCE, and BLK_STS_DEV_RESOURCE).

-Ben

> Thanks for the notice.