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
