Re: [PATCH] scsi: ufs: mark HPB support as BROKEN

[Jens Axboe <axboe@kernel.dk>]

On 10/27/21 8:12 AM, Keith Busch wrote:
> On Wed, Oct 27, 2021 at 06:16:19AM -0700, Bart Van Assche wrote:
>> On 10/26/21 10:27 PM, Christoph Hellwig wrote:
>>> On Tue, Oct 26, 2021 at 01:10:47PM -0700, Bart Van Assche wrote:
>>>> If blk_insert_cloned_request() is moved into the device mapper then I
>>>> think that blk_mq_request_issue_directly() will need to be exported.
>>>
>>> Which is even worse.
>>>
>>>> How
>>>> about the (totally untested) patch below for removing the
>>>> blk_insert_cloned_request() call from the UFS-HPB code?
>>>
>>> Which again doesn't fix anything.  The problem is that it fans out one
>>> request into two on the same queue, not the specific interface used.
>>
>> That patch fixes the reported issue, namely removing the additional accounting
>> caused by calling blk_insert_cloned_request(). Please explain why it is
>> considered wrong to fan out one request into two. That code could be reworked
>> such that the block layer is not involved as Adrian Hunter explained. However,
>> before someone spends time on making these changes I think that someone should
>> provide more information about why it is considered wrong to fan out one request
>> into two.
> 
> The original request consumes a tag from that queue's tagset. If the
> lifetime of that tag depends on that same queue having another free tag,
> you can deadlock.

And to expand on that, one potential solution would be to require as
many reserved tags as normal tags, and have the cloned/direct insert
grab requests out of the reserved pool. That guarantees the forward
progress that is currently violated by randomly requiring an extra tag.

-- 
Jens Axboe