public inbox for linux-nvme@lists.infradead.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: Damien Le Moal <dlemoal@kernel.org>,
	Ming Lei <ming.lei@redhat.com>,
	Nilay Shroff <nilay@linux.ibm.com>,
	linux-block@vger.kernel.org, linux-nvme@lists.infradead.org,
	nbd@other.debian.org, linux-scsi@vger.kernel.org,
	usb-storage@lists.one-eyed-alien.net
Subject: [PATCH 05/10] block: fix queue freeze vs limits lock order in sysfs store methods
Date: Wed,  8 Jan 2025 10:25:02 +0100	[thread overview]
Message-ID: <20250108092520.1325324-6-hch@lst.de> (raw)
In-Reply-To: <20250108092520.1325324-1-hch@lst.de>

queue_attr_store() always freezes a device queue before calling the
attribute store operation. For attributes that control queue limits, the
store operation will also lock the queue limits with a call to
queue_limits_start_update(). However, some drivers (e.g. SCSI sd) may
need to issue commands to a device to obtain limit values from the
hardware with the queue limits locked. This creates a potential ABBA
deadlock situation if a user attempts to modify a limit (thus freezing
the device queue) while the device driver starts a revalidation of the
device queue limits.

Avoid such deadlock by not freezing the queue before calling the
->store_limit() method in struct queue_sysfs_entry and instead use the
queue_limits_commit_update_frozen helper to freeze the queue after taking
the limits lock.

(commit log adapted from a similar patch from  Damien Le Moal)

Fixes: ff956a3be95b ("block: use queue_limits_commit_update in queue_discard_max_store")
Fixes: 0327ca9d53bf ("block: use queue_limits_commit_update in queue_max_sectors_store")
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
---
 block/blk-sysfs.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index f36356cbde0b..2de405cb5f10 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -691,22 +691,24 @@ queue_attr_store(struct kobject *kobj, struct attribute *attr,
 	if (entry->load_module)
 		entry->load_module(disk, page, length);
 
-	mutex_lock(&q->sysfs_lock);
-	blk_mq_freeze_queue(q);
 	if (entry->store_limit) {
 		struct queue_limits lim = queue_limits_start_update(q);
 
 		res = entry->store_limit(disk, page, length, &lim);
 		if (res < 0) {
 			queue_limits_cancel_update(q);
-		} else {
-			res = queue_limits_commit_update(q, &lim);
-			if (!res)
-				res = length;
+			return res;
 		}
-	} else {
-		res = entry->store(disk, page, length);
+
+		res = queue_limits_commit_update_frozen(q, &lim);
+		if (res)
+			return res;
+		return length;
 	}
+
+	mutex_lock(&q->sysfs_lock);
+	blk_mq_freeze_queue(q);
+	res = entry->store(disk, page, length);
 	blk_mq_unfreeze_queue(q);
 	mutex_unlock(&q->sysfs_lock);
 	return res;
-- 
2.45.2



  parent reply	other threads:[~2025-01-08  9:25 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-08  9:24 fix queue freeze and limit locking order v2 Christoph Hellwig
2025-01-08  9:24 ` [PATCH 01/10] block: fix docs for freezing of queue limits updates Christoph Hellwig
2025-01-08 10:19   ` Ming Lei
2025-01-13  7:19   ` Hannes Reinecke
2025-01-08  9:24 ` [PATCH 02/10] block: add a queue_limits_commit_update_frozen helper Christoph Hellwig
2025-01-08 10:20   ` Ming Lei
2025-01-13  7:20   ` Hannes Reinecke
2025-01-08  9:25 ` [PATCH 03/10] block: don't update BLK_FEAT_POLL in __blk_mq_update_nr_hw_queues Christoph Hellwig
2025-01-08 10:17   ` Damien Le Moal
2025-01-08 10:31   ` Ming Lei
2025-01-08 15:27     ` Christoph Hellwig
2025-01-09  0:05       ` Damien Le Moal
2025-01-09  2:18         ` Ming Lei
2025-01-08 10:54   ` Nilay Shroff
2025-01-13  7:23   ` Hannes Reinecke
2025-01-08  9:25 ` [PATCH 04/10] block: add a store_limit operations for sysfs entries Christoph Hellwig
2025-01-08 10:33   ` Ming Lei
2025-01-13  7:24   ` Hannes Reinecke
2025-01-08  9:25 ` Christoph Hellwig [this message]
2025-01-08 10:18   ` [PATCH 05/10] block: fix queue freeze vs limits lock order in sysfs store methods Damien Le Moal
2025-01-08 10:38   ` Ming Lei
2025-01-08 15:29     ` Christoph Hellwig
2025-01-13  7:25   ` Hannes Reinecke
2025-01-08  9:25 ` [PATCH 06/10] nvme: fix queue freeze vs limits lock order Christoph Hellwig
2025-01-08 10:39   ` Ming Lei
2025-01-08  9:25 ` [PATCH 07/10] nbd: " Christoph Hellwig
2025-01-08 10:40   ` Ming Lei
2025-01-08  9:25 ` [PATCH 08/10] usb-storage: " Christoph Hellwig
2025-01-08 10:41   ` Ming Lei
2025-01-08  9:25 ` [PATCH 09/10] loop: refactor queue limits updates Christoph Hellwig
2025-01-08 10:20   ` Damien Le Moal
2025-01-08 10:42   ` Ming Lei
2025-01-08 10:56   ` Nilay Shroff
2025-01-08  9:25 ` [PATCH 10/10] loop: fix queue freeze vs limits lock order Christoph Hellwig
2025-01-08 10:20   ` Damien Le Moal
2025-01-08 10:44   ` Ming Lei
2025-01-08 10:57   ` Nilay Shroff
2025-01-08 10:51 ` fix queue freeze and limit locking order v2 Johannes Thumshirn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250108092520.1325324-6-hch@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=dlemoal@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=nbd@other.debian.org \
    --cc=nilay@linux.ibm.com \
    --cc=usb-storage@lists.one-eyed-alien.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox