Linux SCSI subsystem development
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
	"Martin K. Petersen" <mkp@kernel.org>
Cc: linux-scsi@vger.kernel.org, Damien Le Moal <dlemoal@kernel.org>,
	John Garry <john.garry@linux.dev>,
	Niklas Cassel <cassel@kernel.org>
Subject: [PATCH v6 03/11] scsi: scsi_debug: Take the zone metadata lock before the data lock
Date: Thu, 24 Sep 2026 13:21:31 +0200	[thread overview]
Message-ID: <20260924112127.3815255-16-cassel@kernel.org> (raw)
In-Reply-To: <20260924112127.3815255-13-cassel@kernel.org>

resp_comp_write() takes the data lock and then the zone metadata lock,
while every other command that takes both takes the metadata lock first
and reaches the data lock through do_device_access(). A COMPARE AND
WRITE and any other write to the same store can therefore deadlock each
other.

Take the locks in the same order as everywhere else.

Correct the comment above map_region() while here: the provisioning map
is covered by the metadata lock rather than the data lock, which is why
resp_unmap() takes only the metadata lock.

Assisted-by: LLM
Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Tested with:

  modprobe scsi_debug sector_size=512 dev_size_mb=128 lbpu=1 lbpws=1

COMPARE AND WRITE completes, and a READ(16) of the block that it wrote
returns, so the reordered locks are still taken and released correctly.

The deadlock itself was not reproduced. This kernel is built without
lockdep, and the window needs a COMPARE AND WRITE and another write to
the same store to interleave between the two acquisitions. Found by
review.
---
 drivers/scsi/scsi_debug.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 8e45a57ae406..18aefe83b7b6 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5575,8 +5575,8 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 			    "indicated=%u, IO sent=%d bytes\n", my_name,
 			    dnum * lb_size, ret);
 
-	sdeb_data_write_lock(sip);
 	sdeb_meta_write_lock(sip);
+	sdeb_data_write_lock(sip);
 	if (!comp_write_worker(sip, lba, num, arr, false)) {
 		mk_sense_buffer(scp, MISCOMPARE,
 				MISCOMPARE_DURING_VERIFY_OPERATION);
@@ -5584,12 +5584,12 @@ static int resp_comp_write(struct scsi_cmnd *scp,
 		goto cleanup_unlock;
 	}
 
-	/* Cover sip->map_storep (which map_region()) sets with data lock */
+	/* Cover sip->map_storep (which map_region() sets) with the meta lock */
 	if (scsi_debug_lbp())
 		map_region(sip, lba, num);
 cleanup_unlock:
-	sdeb_meta_write_unlock(sip);
 	sdeb_data_write_unlock(sip);
+	sdeb_meta_write_unlock(sip);
 cleanup_free:
 	kfree(arr);
 	return retval;
-- 
2.55.0


  parent reply	other threads:[~2026-09-24 11:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24 11:21 [PATCH v6 00/11] scsi: scsi_debug: fix zoned write validation Niklas Cassel
2026-09-24 11:21 ` [PATCH v6 01/11] scsi: scsi_debug: Refuse a zoned device with a non-zero lowest aligned LBA Niklas Cassel
2026-09-24 11:21 ` [PATCH v6 02/11] scsi: scsi_debug: Make atomic writes and ZBC emulation mutually exclusive Niklas Cassel
2026-09-24 11:21 ` Niklas Cassel [this message]
2026-09-24 11:21 ` [PATCH v6 04/11] scsi: scsi_debug: Evaluate scsi_debug_lbp() only once Niklas Cassel
2026-09-24 11:21 ` [PATCH v6 05/11] scsi: scsi_debug: Report the residual of a write Niklas Cassel
2026-09-24 11:32   ` sashiko-bot
2026-09-24 11:21 ` [PATCH v6 06/11] scsi: scsi_debug: Enforce physical block alignment of zoned writes Niklas Cassel
2026-09-24 11:21 ` [PATCH v6 07/11] scsi: scsi_debug: Do not write a partial physical block to a zoned device Niklas Cassel
2026-09-24 11:21 ` [PATCH v6 08/11] scsi: scsi_debug: Advance the write pointer over the data written Niklas Cassel
2026-09-24 11:21 ` [PATCH v6 09/11] scsi: scsi_debug: Refuse a short WRITE ATOMIC (16) before writing it Niklas Cassel
2026-09-24 23:53   ` Damien Le Moal
2026-09-24 11:21 ` [PATCH v6 10/11] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16) Niklas Cassel
2026-09-24 11:21 ` [PATCH v6 11/11] scsi: scsi_debug: Validate the access parameters of " Niklas Cassel

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=20260924112127.3815255-16-cassel@kernel.org \
    --to=cassel@kernel.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=dlemoal@kernel.org \
    --cc=john.garry@linux.dev \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mkp@kernel.org \
    /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