On 9/9/25 1:07 PM, Yu Kuai wrote: > Hi, > > 在 2025/09/09 14:39, Nilay Shroff 写道: >> >> >> On 9/8/25 11:45 AM, Yu Kuai wrote: >>> From: Yu Kuai <yukuai3@xxxxxxxxxx> >>> >>> Allocate and free sched_tags while queue is freezed can deadlock[1], >>> this is a long term problem, hence allocate memory before freezing >>> queue and free memory after queue is unfreezed. >>> >>> [1] https://lore.kernel.org/all/0659ea8d-a463-47c8-9180-43c719e106eb@xxxxxxxxxxxxx/ >>> Fixes: e3a2b3f931f5 ("blk-mq: allow changing of queue depth through sysfs") >>> >>> Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx> >> [...] >> [...] >> >>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c >>> index 7ea15bf68b4b..a0a7ebad378f 100644 >>> --- a/block/blk-sysfs.c >>> +++ b/block/blk-sysfs.c >>> @@ -64,11 +64,12 @@ static ssize_t queue_requests_show(struct gendisk *disk, char *page) >>> static ssize_t >>> queue_requests_store(struct gendisk *disk, const char *page, size_t count) >>> { >>> - unsigned long nr; >>> - int ret, err; >>> - unsigned int memflags; >>> struct request_queue *q = disk->queue; >>> struct blk_mq_tag_set *set = q->tag_set; >>> + struct elevator_tags *et = NULL; >>> + unsigned int memflags; >>> + unsigned long nr; >>> + int ret; >>> ret = queue_var_store(&nr, page, count); >>> if (ret < 0) >>> @@ -90,16 +91,24 @@ queue_requests_store(struct gendisk *disk, const char *page, size_t count) >>> goto unlock; >>> } >>> + if (q->elevator && nr > q->elevator->et->nr_requests) { >>> + /* allocate memory before freezing queue to prevent deadlock */ >>> + et = blk_mq_alloc_sched_tags(set, q->nr_hw_queues, nr); >>> + if (!et) { >>> + ret = -ENOMEM; >>> + goto unlock; >>> + } >>> + } >>> + >> I think we should add a comment above explaining why is it safe >> to access q->elevator without holding ->elevator_lock. >> > > I already access q->elevator to check input nr from patch 4, and that's > why I add comments to explain switching elevator is serialized, is this > enough? > yes in patch 04/10 you moved the ->elevator_lock after the usual sanity checks. However when we run those sanity checks or the code in this patch where we have to access q->elevator, it's good to add a comment here in the code (not in commit). For reference, you may check blk_mq_alloc_sched_tags_batch. I think similar comment may be added here as well. Thanks, --Nilay
