Re: [PATCH V2 3/6] blk-mq: don't hold q->sysfs_lock in blk_mq_map_swqueue

[Bart Van Assche <bvanassche@acm.org>]

On 8/21/19 2:15 AM, Ming Lei wrote:
> blk_mq_map_swqueue() is called from blk_mq_init_allocated_queue()
> and blk_mq_update_nr_hw_queues(). For the former caller, the kobject
> isn't exposed to userspace yet. For the latter caller, sysfs/debugfs
> is un-registered before updating nr_hw_queues.
> 
> On the other hand, commit 2f8f1336a48b ("blk-mq: always free hctx after
> request queue is freed") moves freeing hctx into queue's release
> handler, so there won't be race with queue release path too.
> 
> So don't hold q->sysfs_lock in blk_mq_map_swqueue().
> 
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Mike Snitzer <snitzer@redhat.com>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   block/blk-mq.c | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 6968de9d7402..b0ee0cac737f 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2456,11 +2456,6 @@ static void blk_mq_map_swqueue(struct request_queue *q)
>   	struct blk_mq_ctx *ctx;
>   	struct blk_mq_tag_set *set = q->tag_set;
>   
> -	/*
> -	 * Avoid others reading imcomplete hctx->cpumask through sysfs
> -	 */
> -	mutex_lock(&q->sysfs_lock);
> -
>   	queue_for_each_hw_ctx(q, hctx, i) {
>   		cpumask_clear(hctx->cpumask);
>   		hctx->nr_ctx = 0;
> @@ -2521,8 +2516,6 @@ static void blk_mq_map_swqueue(struct request_queue *q)
>   					HCTX_TYPE_DEFAULT, i);
>   	}
>   
> -	mutex_unlock(&q->sysfs_lock);
> -
>   	queue_for_each_hw_ctx(q, hctx, i) {
>   		/*
>   		 * If no software queues are mapped to this hardware queue,
> 

How about adding WARN_ON_ONCE(test_bit(QUEUE_FLAG_REGISTERED, 
&q->queue_flags)) ?

Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>