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.
