[dm-devel] blk-mq request allocation stalls

[Keith Busch]

On Tue, 13 Jan 2015, Jens Axboe wrote:
> On 01/12/2015 11:35 AM, Keith Busch wrote:
>> On Mon, 12 Jan 2015, Keith Busch wrote:
>>> Oh, let's look at "__blk_rq_prep_clone". dm calls that after
>>> blk_get_request() for the blk-mq based multipath types and overrides the
>>> destinations cmd_flags with the source's even though the source was not
>>> allocated from a blk-mq based queue, much less a shared tag.
>> 
>> Untested patch. This will also preserve the failfast cmd_flag dm-mpath
>> set after allocating.
>> 
>> ---
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index 7e78931..6201090 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -2895,7 +2895,10 @@ EXPORT_SYMBOL_GPL(blk_rq_unprep_clone);
>>   static void __blk_rq_prep_clone(struct request *dst, struct request *src)
>>   {
>>       dst->cpu = src->cpu;
>> -    dst->cmd_flags = (src->cmd_flags & REQ_CLONE_MASK) | REQ_NOMERGE;
>> +    if (dst->q->mq_ops)
>> +        dst->cmd_flags |= (src->cmd_flags & REQ_CLONE_MASK) | REQ_NOMERGE;
>> +    else
>> +        dst->cmd_flags = (src->cmd_flags & REQ_CLONE_MASK) | REQ_NOMERGE;
>
> Making the two cases different is a bit... nonsensical. We should do this for 
> both cases, if safe, or move the MQ_INFLIGHT flag and expand the CLONE_MASK.

Expanding the clone mask won't do any good since the src doesn't come
from blk-mq and wouldn't ever have MQ_INFLIGHT set. Blk-mq initializes
the cmd_flags when you get one so I assumed OR'ing was safe. For the
non-blk-mq case, we have no guarantees how the req was initialized and
could have nonsense cmd_flags.