[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e312219e-aa6c-a9a6-fa01-cf38435010be@huaweicloud.com>
Date: Mon, 4 Aug 2025 15:51:48 +0800
From: Yu Kuai <yukuai1@...weicloud.com>
To: Ujwal Kundur <ujwal.kundur@...il.com>, axboe@...nel.dk
Cc: ming.lei@...hat.com, linux-block@...r.kernel.org,
linux-kernel@...r.kernel.org,
syzbot+2e9e529ac0b319316453@...kaller.appspotmail.com,
"yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH] block: prevent deadlock in del_gendisk()
Hi,
在 2025/08/03 21:41, Ujwal Kundur 写道:
> A potential unsafe locking scenario presents itself when
> mutex_lock(&disk->open_mutex) is called with reader's lock held on
> update_nr_hwq_lock:
> CPU0 CPU1
> ---- ----
> rlock(&set->update_nr_hwq_lock)
> lock(&nbd->config_lock);
> lock(&set->update_nr_hwq_lock);
> lock(&disk->open_mutex)
>
This problem is already fixed inside nbd by:
8b428f42f3ed ("nbd: fix lockdep deadlock warning")
Thanks,
Kuai
> When the gendisk is added back concurrently, a writer's lock is
> attempted to be held on update_nr_hwq_lock while holding other locks in
> the call-path, becoming a potential source of deadlock(s).
>
> Scope read-critical section to blk_unregister_queue, which is the only
> function that interacts with switching elevator and requires
> synchronization with update_nr_hwq_lock.
>
> Reported-by: syzbot+2e9e529ac0b319316453@...kaller.appspotmail.com
> Signed-off-by: Ujwal Kundur <ujwal.kundur@...il.com>
> ---
> block/genhd.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
> mode change 100644 => 100755 block/genhd.c
>
> diff --git a/block/genhd.c b/block/genhd.c
> old mode 100644
> new mode 100755
> index c26733f6324b..b56f09f5699b
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -696,6 +696,7 @@ static void __del_gendisk(struct gendisk *disk)
> struct block_device *part;
> unsigned long idx;
> bool start_drain;
> + struct blk_mq_tag_set *set = q->tag_set;
>
> might_sleep();
>
> @@ -740,7 +741,9 @@ static void __del_gendisk(struct gendisk *disk)
> bdi_unregister(disk->bdi);
> }
>
> + down_read(&set->update_nr_hwq_lock);
> blk_unregister_queue(disk);
> + up_read(&set->update_nr_hwq_lock);
>
> kobject_put(disk->part0->bd_holder_dir);
> kobject_put(disk->slave_dir);
> @@ -808,20 +811,15 @@ static void disable_elv_switch(struct request_queue *q)
> */
> void del_gendisk(struct gendisk *disk)
> {
> - struct blk_mq_tag_set *set;
> unsigned int memflags;
>
> if (!queue_is_mq(disk->queue)) {
> __del_gendisk(disk);
> } else {
> - set = disk->queue->tag_set;
> -
> disable_elv_switch(disk->queue);
>
> memflags = memalloc_noio_save();
> - down_read(&set->update_nr_hwq_lock);
> __del_gendisk(disk);
> - up_read(&set->update_nr_hwq_lock);
> memalloc_noio_restore(memflags);
> }
> }
>
Powered by blists - more mailing lists