Messages in this thread | | | Date | Sun, 19 Jul 2015 20:30:59 +0800 | Subject | Re: [PATCH v3 7/7] blk-mq: fix deadlock when reading cpu_list | From | Ming Lei <> |
| |
On Sun, Jul 19, 2015 at 12:28 AM, Akinobu Mita <akinobu.mita@gmail.com> wrote: > CPU hotplug handling for blk-mq (blk_mq_queue_reinit) acquires > all_q_mutex in blk_mq_queue_reinit_notify() and then removes sysfs > entries by blk_mq_sysfs_unregister(). Removing sysfs entry needs to > be blocked until the active reference of the kernfs_node to be zero. > > On the other hand, reading blk_mq_hw_sysfs_cpu sysfs entry (e.g. > /sys/block/nullb0/mq/0/cpu_list) acquires all_q_mutex in > blk_mq_hw_sysfs_cpus_show(). > > If these happen at the same time, a deadlock can happen. Because one > can wait for the active reference to be zero with holding all_q_mutex, > and the other tries to acquire all_q_mutex with holding the active > reference. > > The reason that all_q_mutex is acquired in blk_mq_hw_sysfs_cpus_show() > is to avoid reading an imcomplete hctx->cpumask. Since reading sysfs > entry for blk-mq needs to acquire q->sysfs_lock, we can avoid deadlock > and reading an imcomplete hctx->cpumask by protecting q->sysfs_lock > while hctx->cpumask is being updated. > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > Cc: Jens Axboe <axboe@kernel.dk> > Cc: Ming Lei <tom.leiming@gmail.com>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
> --- > block/blk-mq-sysfs.c | 4 ---- > block/blk-mq.c | 7 +++++++ > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c > index f63b464..e0f71bf 100644 > --- a/block/blk-mq-sysfs.c > +++ b/block/blk-mq-sysfs.c > @@ -218,8 +218,6 @@ static ssize_t blk_mq_hw_sysfs_cpus_show(struct blk_mq_hw_ctx *hctx, char *page) > unsigned int i, first = 1; > ssize_t ret = 0; > > - blk_mq_disable_hotplug(); > - > for_each_cpu(i, hctx->cpumask) { > if (first) > ret += sprintf(ret + page, "%u", i); > @@ -229,8 +227,6 @@ static ssize_t blk_mq_hw_sysfs_cpus_show(struct blk_mq_hw_ctx *hctx, char *page) > first = 0; > } > > - blk_mq_enable_hotplug(); > - > ret += sprintf(ret + page, "\n"); > return ret; > } > diff --git a/block/blk-mq.c b/block/blk-mq.c > index b931e38..1a5e7d1 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -1815,6 +1815,11 @@ 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; > @@ -1834,6 +1839,8 @@ static void blk_mq_map_swqueue(struct request_queue *q, > hctx->ctxs[hctx->nr_ctx++] = ctx; > } > > + mutex_unlock(&q->sysfs_lock); > + > queue_for_each_hw_ctx(q, hctx, i) { > struct blk_mq_ctxmap *map = &hctx->ctx_map; > > -- > 1.9.1 >
-- Ming Lei
| |