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 v4 04/10] scsi: scsi_debug: Evaluate scsi_debug_lbp() only once
Date: Fri, 18 Sep 2026 08:29:15 +0200	[thread overview]
Message-ID: <20260918062910.1709791-16-cassel@kernel.org> (raw)
In-Reply-To: <20260918062910.1709791-12-cassel@kernel.org>

corrupt_lbas(), resp_write_dt0() and resp_write_same() call
scsi_debug_lbp() to decide whether to take the zone metadata lock, and
then call it again to decide whether to read or write the provisioning
map that the lock protects.

The result is not a constant. scsi_debug_lbp() is false while the
fake_rw module parameter is set, and fake_rw can be written at any time,
both as a module parameter and through its driver attribute in sysfs.
The two calls can therefore disagree, and the later one can decide to
touch the provisioning map after the earlier one decided not to take the
lock that protects it.

Call it once and use the result throughout.

Assisted-by: LLM
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Tested with:

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

A WRITE(16) completes and GET LBA STATUS then reports the region as
mapped, which covers resp_write_dt0(). A WRITE SAME and a WRITE SAME
with the UNMAP bit set complete, and GET LBA STATUS reports the first
region as mapped and the second as deallocated, which covers all three
uses of the result in resp_write_same().

corrupt_lbas() is not covered. It is reached by writing to the corrupt
file in debugfs, and that interface rejected the requests that were
tried, for a reason that has nothing to do with this patch.

The window that the change closes was not reproduced. It needs fake_rw
to be written between two calls, and was found by review.
---
 drivers/scsi/scsi_debug.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 18aefe83b7b6..c68dba6dbbdd 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4953,12 +4953,13 @@ static int corrupt_lbas(struct sdebug_dev_info *devip, u64 lba, u32 num,
 {
 	struct sdeb_store_info *sip = devip2sip(devip, false);
 	bool meta_data_locked = false;
+	bool lbp = scsi_debug_lbp();
 	u32 block, num_mapped, b, i;
 	int error = 0;
 
 	if (sdebug_dev_is_zoned(devip) ||
 	    sdebug_dix ||
-	    scsi_debug_lbp())  {
+	    lbp)  {
 		sdeb_meta_write_lock(sip);
 		meta_data_locked = true;
 	}
@@ -4975,7 +4976,7 @@ static int corrupt_lbas(struct sdebug_dev_info *devip, u64 lba, u32 num,
 		goto out_unlock;
 	}
 
-	if (scsi_debug_lbp() &&
+	if (lbp &&
 	    (!map_state(sip, lba, &num_mapped) || num > num_mapped)) {
 		pr_err("can't modify unmapped logical blocks: %llu:%u",
 			lba, num);
@@ -5035,6 +5036,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	struct sdeb_store_info *sip = devip2sip(devip, true);
 	u8 *cmd = scp->cmnd;
 	bool meta_data_locked = false;
+	bool lbp = scsi_debug_lbp();
 
 	if (unlikely(sdebug_opts & SDEBUG_OPT_UNALIGNED_WRITE &&
 		     atomic_read(&sdeb_inject_pending))) {
@@ -5102,7 +5104,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 
 	if (sdebug_dev_is_zoned(devip) ||
 	    (sdebug_dix && scsi_prot_sg_count(scp)) ||
-	    scsi_debug_lbp())  {
+	    lbp)  {
 		sdeb_meta_write_lock(sip);
 		meta_data_locked = true;
 	}
@@ -5147,7 +5149,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	}
 
 	ret = do_device_access(sip, scp, 0, lba, num, group, true, false);
-	if (unlikely(scsi_debug_lbp()))
+	if (unlikely(lbp))
 		map_region(sip, lba, num);
 
 	/* If ZBC zone then bump its write pointer */
@@ -5374,8 +5376,9 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 	u8 *fs1p;
 	u8 *fsp;
 	bool meta_data_locked = false;
+	bool lbp = scsi_debug_lbp();
 
-	if (sdebug_dev_is_zoned(devip) || scsi_debug_lbp()) {
+	if (sdebug_dev_is_zoned(devip) || lbp) {
 		sdeb_meta_write_lock(sip);
 		meta_data_locked = true;
 	}
@@ -5384,7 +5387,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 	if (ret)
 		goto out;
 
-	if (unmap && scsi_debug_lbp()) {
+	if (unmap && lbp) {
 		unmap_region(sip, lba, num);
 		goto out;
 	}
@@ -5414,7 +5417,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num,
 		block = do_div(lbaa, sdebug_store_sectors);
 		memmove(fsp + (block * lb_size), fs1p, lb_size);
 	}
-	if (scsi_debug_lbp())
+	if (lbp)
 		map_region(sip, lba, num);
 	/* If ZBC zone then bump its write pointer */
 	if (sdebug_dev_is_zoned(devip))
-- 
2.55.0


  parent reply	other threads:[~2026-09-18  6:29 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18  6:29 [PATCH v4 00/10] scsi: scsi_debug: fix zoned write validation Niklas Cassel
2026-09-18  6:29 ` [PATCH v4 01/10] scsi: scsi_debug: Refuse a zoned device with a non-zero lowest aligned LBA Niklas Cassel
2026-09-18  9:26   ` Damien Le Moal
2026-09-18  6:29 ` [PATCH v4 02/10] scsi: scsi_debug: Make atomic writes and ZBC emulation mutually exclusive Niklas Cassel
2026-09-18  7:21   ` John Garry
2026-09-18  9:26   ` Damien Le Moal
2026-09-18  6:29 ` [PATCH v4 03/10] scsi: scsi_debug: Take the zone metadata lock before the data lock Niklas Cassel
2026-09-18  9:28   ` Damien Le Moal
2026-09-18  6:29 ` Niklas Cassel [this message]
2026-09-18  9:29   ` [PATCH v4 04/10] scsi: scsi_debug: Evaluate scsi_debug_lbp() only once Damien Le Moal
2026-09-18  6:29 ` [PATCH v4 05/10] scsi: scsi_debug: Report the residual of a write Niklas Cassel
2026-09-18  6:29 ` [PATCH v4 06/10] scsi: scsi_debug: Enforce physical block alignment of zoned writes Niklas Cassel
2026-09-18  6:29 ` [PATCH v4 07/10] scsi: scsi_debug: Do not write a partial physical block to a zoned device Niklas Cassel
2026-09-18  9:31   ` Damien Le Moal
2026-09-18 10:25     ` Niklas Cassel
2026-09-18  6:29 ` [PATCH v4 08/10] scsi: scsi_debug: Advance the write pointer over the data written Niklas Cassel
2026-09-18  6:29 ` [PATCH v4 09/10] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16) Niklas Cassel
2026-09-18  7:29   ` John Garry
2026-09-18  8:09     ` Niklas Cassel
2026-09-18  6:29 ` [PATCH v4 10/10] scsi: scsi_debug: Validate the access parameters of " Niklas Cassel
2026-09-18  7:33   ` John Garry
2026-09-18  7:53     ` Niklas Cassel
2026-09-18  8:19       ` John Garry
2026-09-18  8:55         ` Niklas Cassel
2026-09-18  9:32   ` Damien Le Moal

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=20260918062910.1709791-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