[PATCH 1/2] block: avoid to hold q->limits_lock across APIs for atomic update queue limits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux