Re: [PATCH] scsi: ufs: Fix task management completion

[Adrian Hunter <adrian.hunter@intel.com>]

On 22/09/21 11:48 pm, Bart Van Assche wrote:
> On 9/22/21 2:10 AM, Adrian Hunter wrote:
>> The UFS driver uses blk_mq_tagset_busy_iter() when identifying task
>> management requests to complete, however blk_mq_tagset_busy_iter()
>> doesn't work.
>>
>> blk_mq_tagset_busy_iter() only iterates requests dispatched by the block
>> layer. That appears as if it might have started since commit 37f4a24c2469a1
>> ("blk-mq: centralise related handling into blk_mq_get_driver_tag") which
>> removed 'data->hctx->tags->rqs[rq->tag] = rq' from blk_mq_rq_ctx_init()
>> which gets called:
>>
>>     blk_get_request
>>         blk_mq_alloc_request
>>             __blk_mq_alloc_request
>>                 blk_mq_rq_ctx_init
>>
>> Since UFS task management requests are not dispatched by the block
>> layer, hctx->tags->rqs[rq->tag] remains NULL,  and since
>> blk_mq_tagset_busy_iter() relies on finding requests using
>> hctx->tags->rqs[rq->tag], UFS task management requests are never found by
>> blk_mq_tagset_busy_iter().
>>
>> By using blk_mq_tagset_busy_iter(), the UFS driver was relying on internal
>> details of the block layer, which was fragile and subsequently got
>> broken. Fix by removing the use of blk_mq_tagset_busy_iter() and having
>> the driver keep track of task management requests.
> 
> Thanks for the detailed analysis. I agree that using blk_mq_tagset_busy_iter()
> no longer works due to recent changes in the block layer. Has it been
> considered to export blk_mq_all_tag_iter() and to use that function instead of
> blk_mq_tagset_busy_iter()?

It seemed better not to be the only driver using the block layer in a
different way.  Having the UFS driver self-contained in this regard
seemed more robust.  If anything, the code without blk_mq_tagset_busy_iter()
is slightly shorter, so using a block layer iterator doesn't help much.