Commit d690cb8ae14b ("block: add an API to atomically update queue limits") adds APIs for updating queue limits atomically. And q->limits_lock is grabbed in queue_limits_start_update(), and released in queue_limits_commit_update(). This way is very fragile and easy to introduce deadlock[1][2]. More importantly, queue_limits_start_update() returns one local copy of q->limits, then the API user overwrites the local copy, and commit the copy by queue_limits_commit_update() finally. So holding q->limits_lock protects nothing for the local copy, and not see any real help by preventing new update & commit from happening, cause what matters is that we do validation & commit atomically. Changes the API to not hold q->limits_lock across atomic queue limits update APIs for fixing deadlock & making it easy to use. [1] https://lore.kernel.org/linux-block/Z1A8fai9_fQFhs1s@xxxxxxxxxxxxxxxxxxxx/ [2] https://lore.kernel.org/linux-scsi/ZxG38G9BuFdBpBHZ@fedora/ Fixes: d690cb8ae14b ("block: add an API to atomically update queue limits") Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx> --- block/blk-settings.c | 2 +- include/linux/blkdev.h | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/block/blk-settings.c b/block/blk-settings.c index 8f09e33f41f6..b737428c6084 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -422,6 +422,7 @@ int queue_limits_commit_update(struct request_queue *q, { int error; + mutex_lock(&q->limits_lock); error = blk_validate_limits(lim); if (error) goto out_unlock; @@ -456,7 +457,6 @@ EXPORT_SYMBOL_GPL(queue_limits_commit_update); */ int queue_limits_set(struct request_queue *q, struct queue_limits *lim) { - mutex_lock(&q->limits_lock); return queue_limits_commit_update(q, lim); } EXPORT_SYMBOL_GPL(queue_limits_set); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 378d3a1a22fc..6cc20ca79adc 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -944,8 +944,13 @@ static inline unsigned int blk_boundary_sectors_left(sector_t offset, static inline struct queue_limits queue_limits_start_update(struct request_queue *q) { + struct queue_limits lim; + mutex_lock(&q->limits_lock); - return q->limits; + lim = q->limits; + mutex_unlock(&q->limits_lock); + + return lim; } int queue_limits_commit_update(struct request_queue *q, struct queue_limits *lim); @@ -962,7 +967,6 @@ int blk_validate_limits(struct queue_limits *lim); */ static inline void queue_limits_cancel_update(struct request_queue *q) { - mutex_unlock(&q->limits_lock); } /* -- 2.47.0