Subject: Re: [PATCH V3 3/3] blk-mq: fix dispatch from sw queue
To:     Ming Lei <ming.lei@redhat.com>
Cc:     linux-block@vger.kernel.org, Christoph Hellwig <hch@lst.de>
References: <20181217120844.8131-1-ming.lei@redhat.com>
 <20181217120844.8131-4-ming.lei@redhat.com>
From:   Jens Axboe <axboe@kernel.dk>
Message-ID: <fe4133ec-7dfa-255c-9c24-af448446bf23@kernel.dk>
Date:   Mon, 17 Dec 2018 05:55:33 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <20181217120844.8131-4-ming.lei@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-block-owner@vger.kernel.org
Precedence: bulk

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;

-- 
Jens Axboe