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

[Ming Lei <ming.lei@redhat.com>]

On Wed, Aug 21, 2019 at 08:53:52AM -0700, Bart Van Assche wrote:
> 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)) ?

q->kobject isn't un-registered before updating nr_hw_queues, and only
hctx->kobj is un-registered, so we can't add the warn here.


Thanks,
Ming