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@xxxxxxxxxxxxx>
> > Cc: Hannes Reinecke <hare@xxxxxxxx>
> > Cc: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Mike Snitzer <snitzer@xxxxxxxxxx>
> > Cc: Bart Van Assche <bvanassche@xxxxxxx>
> > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> > ---
> > 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
