Re: [PATCH V3 3/3] blk-mq: fix dispatch from sw queue

[Jens Axboe <axboe@kernel.dk>]

On 12/17/18 6:29 AM, Jens Axboe wrote:
> On 12/17/18 5:55 AM, Jens Axboe wrote:
>> On 12/17/18 5:08 AM, Ming Lei wrote:
>>> When requst is added to rq list of sw queue(ctx), the rq may be from
>>> different type of hctx, especially after multi queue mapping is introduced.
>>>
>>> So when dispach request from sw queue via blk_mq_flush_busy_ctxs() or
>>> blk_mq_dequeue_from_ctx(), one request belonging to other queue type of
>>> hctx can be dispatch to current hctx in case that read queue or poll queue
>>> is enabled.
>>>
>>> This patch fixes this issue by introducing per-queue-type list.
>>
>> Looks good, just one comment:
>>
>>> diff --git a/block/blk-mq.h b/block/blk-mq.h
>>> index d1ed096723fb..0973a91eb1dd 100644
>>> --- a/block/blk-mq.h
>>> +++ b/block/blk-mq.h
>>> @@ -12,14 +12,16 @@ struct blk_mq_ctxs {
>>>  	struct blk_mq_ctx __percpu	*queue_ctx;
>>>  };
>>>  
>>> +struct blk_mq_ctx_list {
>>> +	spinlock_t		lock;
>>> +	struct list_head	rq_list;
>>> +}  ____cacheline_aligned_in_smp;
>>> +
>>>  /**
>>>   * struct blk_mq_ctx - State for a software queue facing the submitting CPUs
>>>   */
>>>  struct blk_mq_ctx {
>>> -	struct {
>>> -		spinlock_t		lock;
>>> -		struct list_head	rq_list;
>>> -	}  ____cacheline_aligned_in_smp;
>>> +	struct blk_mq_ctx_list  list[HCTX_MAX_TYPES];
>>
>> Let's not make that use 3 cachelines. There is no good reason to split
>> these across cachelines, if we have heavy traffic to multiple of these,
>> then we're not going very fast anyway. So just make it:
>>
>> 	struct {
>> 		spinlock_t		lock;
>> 		struct list_head	rq_list[HCTX_MAX_TYPES];
>> 	}  ____cacheline_aligned_in_smp;
> 
> Did it just to check, turns out fine and is of course less changes.
> Let me know.

Tests good for me, thanks for doing this work. Just a heads up that
I'm going to queue this up, attributed to you, but just with a note
that I changed the layout of that structure.

-- 
Jens Axboe