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
