Re: [PATCH V2 2/6] block: don't hold q->sysfs_lock in elevator_init_mq

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

On 8/21/19 2:15 AM, Ming Lei wrote:
> The original comment says:
> 
> 	q->sysfs_lock must be held to provide mutual exclusion between
> 	elevator_switch() and here.
> 
> Which is simply wrong. elevator_init_mq() is only called from
> blk_mq_init_allocated_queue, which is always called before the request
> queue is registered via blk_register_queue(), for dm-rq or normal rq
> based driver. However, queue's kobject is just exposed added to sysfs
                                             ^^^^^^^^^^^^
                                             only?
> in blk_register_queue(). So there isn't such race between elevator_switch()
> and elevator_init_mq().
> 
> So avoid to hold q->sysfs_lock in elevator_init_mq().
[ ... ]
>   	/*
> -	 * q->sysfs_lock must be held to provide mutual exclusion between
> -	 * elevator_switch() and here.
> +	 * We are called from blk_mq_init_allocated_queue() only, at that
> +	 * time the request queue isn't registered yet, so the queue
> +	 * kobject isn't exposed to userspace. No need to worry about race
> +	 * with elevator_switch(), and no need to hold q->sysfs_lock.
>   	 */

How about replacing this comment with the following:

WARN_ON_ONCE(test_bit(QUEUE_FLAG_REGISTERED, &q->queue_flags));

Anyway:

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