[PATCH v5 00/10] scsi: scsi_debug: fix zoned write validation

[PATCH v5 00/10] scsi: scsi_debug: fix zoned write validation

[Niklas Cassel]

This series fixes scsi_debug with regards to writes to a ZBC drive, and
to WRITE ATOMIC (16). Each patch describes the problem that it fixes.

Two configurations are refused rather than emulated, and those two
patches come first, as the rest of the series relies on what they
exclude: a zoned device whose lowest aligned LBA is not zero, which no
host writing in units of the reported zone write granularity could ever
write, and atomic writes on a zoned device, which no drive supports.

Little of this is reachable in a default configuration. Most of it needs
physblk_exp, atomic_wr or logical block provisioning to be set, or an
initiator that does not provide a data buffer matching the transfer
length of the command.

The series is based on 7.4/scsi-staging rather than 7.3/scsi-fixes (as
Damien suggested on v1) to avoid a build failure that would have
happened if this series was simply merged with linux-next:

  error: too many arguments to function 'mk_sense_buffer'

Tested on a zoned scsi_debug device with 512 byte logical blocks and a
4096 byte physical block, and on a device that is not zoned for the
WRITE ATOMIC (16) patches. The notes below each patch describe what was
observed before and after it.

Changes since v4:
- Patches 7 and 8 use a bit shift rather than a division, as Damien
  suggested. sector_size is always a power of two.
- Patch 9 drops the meta_data_locked variable, as John suggested.
- Patch 10 drops the paragraph describing the two sense codes that the
  missing check fails to generate.
- Picked up Reviewed-by tags from Damien and John.

Niklas Cassel (10):
  scsi: scsi_debug: Refuse a zoned device with a non-zero lowest aligned
    LBA
  scsi: scsi_debug: Make atomic writes and ZBC emulation mutually
    exclusive
  scsi: scsi_debug: Take the zone metadata lock before the data lock
  scsi: scsi_debug: Evaluate scsi_debug_lbp() only once
  scsi: scsi_debug: Report the residual of a write
  scsi: scsi_debug: Enforce physical block alignment of zoned writes
  scsi: scsi_debug: Do not write a partial physical block to a zoned
    device
  scsi: scsi_debug: Advance the write pointer over the data written
  scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)
  scsi: scsi_debug: Validate the access parameters of WRITE ATOMIC (16)

 drivers/scsi/scsi_debug.c | 102 ++++++++++++++++++++++++++++++++------
 1 file changed, 86 insertions(+), 16 deletions(-)


base-commit: c3cff7fac01638ab58e85fe7df41a04fa25c5bae
-- 
2.55.0

[PATCH v5 01/10] scsi: scsi_debug: Refuse a zoned device with a non-zero lowest aligned LBA

[Niklas Cassel]

The LOWEST ALIGNED LOGICAL BLOCK ADDRESS field indicates the LBA of the
first logical block that is located at the beginning of a physical
block, see SBC-6 r02 (T10/BSR INCITS 587), 5.20.2. scsi_debug reports
the lowest_aligned module parameter in that field of the READ CAPACITY
(16) parameter data, but nothing else in the driver acts on it: the data
it stores begins at LBA 0 and has no physical block structure behind it.

That does not matter for a device that is not zoned, where a write need
not end on a physical block boundary at all. It does matter for a zoned
device: ZBC requires a write to a sequential write required zone to end
on a physical block boundary, and a zone begins at a multiple of the
zone size, so a non-zero lowest aligned LBA puts the beginning of every
zone in the middle of a physical block. A host that writes in units of
the zone write granularity that it was given can then never satisfy the
requirement, and such a zone cannot be written at all.

Refuse the combination rather than emulate a device that cannot be used.

Assisted-by: LLM
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Tested by loading the module with each combination:

  zbc=managed lowest_aligned=8    refused, -EINVAL
  zbc=aware   lowest_aligned=8    refused, -EINVAL
  zbc=managed lowest_aligned=0    loads
  lowest_aligned=8                loads, as the device is not zoned
---
 drivers/scsi/scsi_debug.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 6941809dfdb7..c664966e95cb 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -8734,6 +8734,11 @@ static int __init scsi_debug_init(void)
 			sdebug_dev_size_mb = DEF_ZBC_DEV_SIZE_MB;
 	}
 
+	if (sdeb_zbc_in_use && sdebug_lowest_aligned) {
+		pr_err("lowest_aligned is not supported by a zoned device\n");
+		return -EINVAL;
+	}
+
 	if (sdebug_dev_size_mb == DEF_DEV_SIZE_PRE_INIT)
 		sdebug_dev_size_mb = DEF_DEV_SIZE_MB;
 	if (sdebug_dev_size_mb < 1)
-- 
2.55.0

[PATCH v5 02/10] scsi: scsi_debug: Make atomic writes and ZBC emulation mutually exclusive

[Niklas Cassel]

There are no SMR drives that support atomic writes. Emulating a device
that supports both is emulating something that a user cannot encounter,
rather than the drives that exist, and it means maintaining the zone
handling of a command that no zoned device implements.

Nothing in the standards forbids the combination. For what would be by
far the most common case, a host managed SMR drive whose logical block
size equals its physical block size, there is no problem in the two
features being combined: the drive vendor only has to give atomic
writes an alignment that makes sense with regards to ZBC, for instance
equal to the physical block size. This is therefore a choice not to
complicate scsi_debug, rather than a restriction that the standards
impose.

Refuse the combination at initialisation. If SMR drives that support
atomic writes ever become a thing, this can be changed back.

Suggested-by: Damien Le Moal <dlemoal@kernel.org>
Assisted-by: LLM
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: John Garry <john.garry@linux.dev>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Tested by loading the module with each combination:

  zbc=managed atomic_wr=1    refused, -EINVAL
  zbc=aware   atomic_wr=1    refused, -EINVAL
  zbc=managed                loads
  atomic_wr=1                loads
---
 drivers/scsi/scsi_debug.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c664966e95cb..8e45a57ae406 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -8739,6 +8739,11 @@ static int __init scsi_debug_init(void)
 		return -EINVAL;
 	}
 
+	if (sdeb_zbc_in_use && sdebug_atomic_wr) {
+		pr_err("atomic_wr is not supported by a zoned device\n");
+		return -EINVAL;
+	}
+
 	if (sdebug_dev_size_mb == DEF_DEV_SIZE_PRE_INIT)
 		sdebug_dev_size_mb = DEF_DEV_SIZE_MB;
 	if (sdebug_dev_size_mb < 1)
-- 
2.55.0

[PATCH v5 03/10] scsi: scsi_debug: Take the zone metadata lock before the data lock

[Niklas Cassel]

resp_comp_write() takes the data lock and then the zone metadata lock.
Every other command that takes both takes them the other way round:
resp_write_dt0(), resp_write_scat(), resp_write_same() and
resp_write_tape() take the metadata lock and then call
do_device_access(), which takes the data lock.

Two commands that take the same two locks in opposite orders can
deadlock against each other, so a COMPARE AND WRITE and any other write
to the same store can hang one another.

Take the locks in the same order as everywhere else, and release them in
the reverse of that order.

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

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

[PATCH v5 04/10] scsi: scsi_debug: Evaluate scsi_debug_lbp() only once

[Niklas Cassel]

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

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

[PATCH v5 05/10] scsi: scsi_debug: Report the residual of a write

[Niklas Cassel]

A command transfers the number of logical blocks that it asks for,
bounded by the data buffer that the initiator provided. When the buffer
is larger than that, the bytes beyond are not transferred, and the
difference is a residual that the initiator is entitled to be told
about.

resp_read_dt0() reports it, as does resp_write_tape(), but the write
paths for a disk do not: scsi_get_resid() keeps the zero that
scsi_debug_queuecommand() initialised it with, so a write with an
oversized buffer completes with GOOD status and a residual of zero, as
though the whole buffer had been consumed. A READ of the same length
into the same buffer reports the residual correctly, so the two
directions disagree.

Report it in resp_write_dt0(), resp_write_scat() and
resp_atomic_write().

resp_write_scat() needs a different expression from the other two. Its
data-out buffer holds the parameter list header and the LBA range
descriptors as well as the data, and sg_off walks over all of it, from
the offset at which the data begins to the end of the last range that
was written, so what the command consumed is sg_off and not the number
of bytes that the last range transferred.

It also needs a guard that the other two do not. sg_off counts what the
command asked for rather than what was transferred: lbdof is not
validated against the length of the buffer, and a range is counted in
full even when do_device_access() copied less of it, so sg_off can
exceed the buffer. The other two subtract the number of bytes that were
copied, which the buffer bounds.

Assisted-by: LLM
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Tested with:

  modprobe scsi_debug zbc=managed sector_size=512 physblk_exp=3 \
      zone_size_mb=8 dev_size_mb=128 zone_nr_conv=2

An eight logical block WRITE(16) with a 5000 byte buffer reports
resid=904, having transferred 4096. A READ(16) of the same length into
the same buffer reported that before this patch and the WRITE reported
0. A buffer of exactly 4096 bytes reports 0, as does a 512 byte buffer,
which is consumed in its entirety. WRITE ATOMIC (16) behaves like the
WRITE, on a device that is not zoned.

WRITE SCATTERED (16) with one LBA range descriptor of eight blocks and a
5632 byte buffer reports resid=1024, having consumed 512 bytes of
parameter list and 4096 bytes of data. The same command with a 1024 byte
buffer completes and reports resid=0: sg_off reaches 4608, past the end
of the buffer, and without the guard the residual would have been
reported as 4294963712.
---
 drivers/scsi/scsi_debug.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index c68dba6dbbdd..170d5c944b61 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5166,6 +5166,8 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 			    "%s: write: cdb indicated=%u, IO sent=%d bytes\n",
 			    my_name, num * sdebug_sector_size, ret);
 
+	scsi_set_resid(scp, scsi_bufflen(scp) - ret);
+
 	if (unlikely((sdebug_opts & SDEBUG_OPT_RECOV_DIF_DIX) &&
 		     atomic_read(&sdeb_inject_pending))) {
 		if (sdebug_opts & SDEBUG_OPT_RECOVERED_ERR) {
@@ -5354,6 +5356,13 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 		sg_off += num_by;
 		cum_lb += num;
 	}
+	/*
+	 * sg_off counts what the command asked for, which can exceed the
+	 * buffer: lbdof is not validated against it, and a range is counted
+	 * in full even if do_device_access() copied less.
+	 */
+	if (scsi_bufflen(scp) > sg_off)
+		scsi_set_resid(scp, scsi_bufflen(scp) - sg_off);
 	ret = 0;
 err_out_unlock:
 	sdeb_meta_write_unlock(sip);
@@ -6210,6 +6219,8 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
 		return DID_ERROR << 16;
 	if (unlikely(ret != len * sdebug_sector_size))
 		return DID_ERROR << 16;
+
+	scsi_set_resid(scp, scsi_bufflen(scp) - ret);
 	return 0;
 }
 
-- 
2.55.0

[PATCH v5 06/10] scsi: scsi_debug: Enforce physical block alignment of zoned writes

[Niklas Cassel]

ZBC-3 r06 (T10/BSR INCITS 579), 4.5.3.3.2 Write access pattern
requirements for sequential write required zones, states:

  The device server terminates with CHECK CONDITION status, with the
  sense key set to ILLEGAL REQUEST, and the additional sense code set
  to UNALIGNED WRITE COMMAND a write command, other than an entire
  medium write same command, that specifies:
    a) the starting LBA in a sequential write required zone set to a
       value that is not equal to the write pointer for that sequential
       write required zone; or
    b) an ending LBA that is not equal to the last logical block within
       a physical block (see SBC-5).

That is why sd_zbc_read_zones() sets the zone_write_granularity queue
limit to the physical block size of a host-managed device, exposing the
constraint to user space.

check_zbc_access_params() implements condition a) but not condition b):
it verifies that a write to a sequential write required zone starts at
the write pointer of the zone, but never validates the ending LBA. As a
consequence, when scsi_debug emulates a host-managed device whose
physical block size is larger than its logical block size, for instance
with zbc=managed sector_size=512 physblk_exp=3, a write of a single
logical block at the write pointer of a sequential zone is accepted and
advances the write pointer by one logical block. The write pointer is
then no longer a multiple of the zone_write_granularity reported for the
device, so nothing can write at it at the granularity that was
advertised, and the zone can only be used again after being reset.

Implement condition b) as well, with the same sense data as the write
pointer check, as the standard gives both conditions the same sense key
and additional sense code. The exclusion of an entire medium write same
command needs no special case: such a command spans the whole medium, so
it is already terminated with WRITE BOUNDARY VIOLATION by the preceding
check.

The check is placed in check_zbc_access_params(), which every command
that advances a zone write pointer reaches first: WRITE, WRITE SCATTERED
and WRITE SAME. Reads return earlier in the function and are unaffected.

Sequential write preferred zones, which are emulated for host-aware
devices with zbc=aware, are left alone: writes to them are not required
to be sequential, and Linux does not restrict the write granularity of
host-aware devices.

With the default physblk_exp=0, the physical block size equals the
logical block size and the new check is a no-op.

Assisted-by: LLM
Fixes: f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Tested with:

  modprobe scsi_debug zbc=managed sector_size=512 physblk_exp=3 \
      zone_size_mb=8 dev_size_mb=128 zone_nr_conv=2

Before this patch, a one logical block WRITE(16) at the write pointer of
an empty sequential write required zone completes with GOOD status and
leaves the write pointer at LBA 0x18001, which is not a multiple of the
4096 byte zone_write_granularity reported for the device. After it, the
same command is terminated with ILLEGAL REQUEST / UNALIGNED WRITE
COMMAND and the zone is left EMPTY, while an aligned eight block write
is still accepted.

Compared to the version reviewed in v1, the only change is that the new
mk_sense_buffer() call uses the combined UNALIGNED_WRITE_COMMAND sense
code, as this is based on 7.4/scsi-staging.
---
 drivers/scsi/scsi_debug.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 170d5c944b61..a4db282ac971 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3980,6 +3980,16 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
 					UNALIGNED_WRITE_COMMAND);
 			return check_condition_result;
 		}
+		/*
+		 * Writes must end on a physical block boundary, that is, the
+		 * transfer length must be a multiple of the physical block
+		 * size.
+		 */
+		if (!IS_ALIGNED(lba + num, 1U << sdebug_physblk_exp)) {
+			mk_sense_buffer(scp, ILLEGAL_REQUEST,
+					UNALIGNED_WRITE_COMMAND);
+			return check_condition_result;
+		}
 	}
 
 	/* Handle implicit open of closed and empty zones */
-- 
2.55.0

[PATCH v5 07/10] scsi: scsi_debug: Do not write a partial physical block to a zoned device

[Niklas Cassel]

do_device_access() copies one logical block at a time and stops at the
first short copy, so when the data-out buffer is smaller than the
transfer length of the command it can write part of a physical block.
An initiator can arrange that with SG_IO.

A write that does not end on a physical block boundary is perfectly
acceptable to a device that is not zoned, and to a conventional zone,
where the device reads, modifies and writes the physical block that the
write falls in. ZBC-3 r06 (T10/BSR INCITS 579), 4.5.3.3.2, does require
a write to a sequential write required zone to end on a physical block
boundary, though, so a partly written physical block is not a state that
such a zone can be left in.

Stop at the last whole physical block that the buffer holds, for a
sequential write required zone only. The bytes that are left over are
not written, and are reported to the initiator as part of the residual.

With the default physblk_exp=0 the physical block size equals the
logical block size and this changes nothing. Nothing changes either when
the buffer holds all of the data that the command asks for, so a command
that transfers fewer logical blocks than a physical block is unaffected
wherever it is legal.

Assisted-by: LLM
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Tested with:

  modprobe scsi_debug zbc=managed sector_size=512 physblk_exp=3 \
      zone_size_mb=8 dev_size_mb=128 zone_nr_conv=2

issuing a sixteen logical block WRITE(16) through SG_IO with a buffer of
twelve blocks. To a sequential write required zone it now transfers
eight blocks, one whole physical block, and reports the remaining 2048
bytes as the residual; before this patch it transferred twelve. To a
conventional zone it still transfers all twelve and reports no residual.

With physblk_exp=0 an eight block write with a buffer of one block still
transfers one block, wherever it is addressed.
---

Changes since v4: the division is a bit shift now, as suggested.
 drivers/scsi/scsi_debug.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index a4db282ac971..cf5037e81952 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4273,6 +4273,8 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
 	u64 block;
 	enum dma_data_direction dir;
 	struct scsi_data_buffer *sdb = &scp->sdb;
+	struct scsi_device *sdp = scp->device;
+	struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
 	u8 *fsp;
 	int i, total = 0;
 
@@ -4300,6 +4302,25 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
 
 	fsp = sip->storep;
 
+	/*
+	 * A write to a sequential write required zone has to end on a physical
+	 * block boundary, so if the data-out buffer does not hold all of the
+	 * data that the command asks for, write up to the last whole physical
+	 * block that it does hold. What is left over is reported as part of
+	 * the residual.
+	 */
+	if (do_write && sdebug_dev_is_zoned(devip)) {
+		struct sdeb_zone_state *zsp = zbc_zone(devip, lba);
+
+		if (zsp->z_type == ZBC_ZTYPE_SWR) {
+			u32 avail = (sdb->length - sg_skip)
+				>> ilog2(sdebug_sector_size);
+
+			if (avail < num)
+				num = round_down(avail, 1U << sdebug_physblk_exp);
+		}
+	}
+
 	block = do_div(lba, sdebug_store_sectors);
 
 	/* Only allow 1x atomic write or multiple non-atomic writes at any given time */
-- 
2.55.0

[PATCH v5 08/10] scsi: scsi_debug: Advance the write pointer over the data written

[Niklas Cassel]

resp_write_dt0() and resp_write_scat() advance the write pointer of a
sequential write required zone by the transfer length of the command
before looking at what do_device_access() returned. It does not always
move that much data: it returns -1 when the data direction of the
command does not match the operation, and a short byte count when the
data-out buffer is smaller than the transfer length, both of which an
initiator can produce with SG_IO. The first terminates the command, the
second completes it with GOOD status and a residual.

The zone state then describes more data than is on the medium, and a
write at the position where the data really ends is terminated with
UNALIGNED WRITE COMMAND, because the write pointer has moved beyond it.
The zone has to be reset before it can be written to again.

Advance the write pointer over the data that was written instead. As
do_device_access() does not write a partial physical block to a
sequential write required zone, what it reports for such a zone is a
whole number of physical blocks, so the write pointer is left on a
physical block boundary, which is where a write can end.

resp_write_same() does not need the same treatment, as it writes with
memmove() and cannot fail part way through.

Assisted-by: LLM
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Fixes: f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Tested with:

  modprobe scsi_debug zbc=managed sector_size=512 physblk_exp=3 \
      zone_size_mb=8 dev_size_mb=128 zone_nr_conv=2

issuing WRITE(16) through SG_IO with a data-out buffer shorter than the
transfer length of the command. A sixteen block write with a buffer of
twelve blocks leaves the write pointer of an empty sequential write
required zone eight blocks on, which is where the data it wrote ends,
and a following eight block write continues from there. An eight block
write with a buffer of one block writes nothing and leaves the write
pointer where it was.

Before this patch both advanced the write pointer by the full transfer
length, and a write at the end of the data that had actually been
written was then terminated with UNALIGNED WRITE COMMAND.
---

Changes since v4: the two divisions are bit shifts now, as suggested.
 drivers/scsi/scsi_debug.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cf5037e81952..d07f6f891951 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5183,9 +5183,9 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 	if (unlikely(lbp))
 		map_region(sip, lba, num);
 
-	/* If ZBC zone then bump its write pointer */
-	if (sdebug_dev_is_zoned(devip))
-		zbc_inc_wp(devip, lba, num);
+	/* If ZBC zone then bump its write pointer over the data written */
+	if (sdebug_dev_is_zoned(devip) && ret > 0)
+		zbc_inc_wp(devip, lba, ret >> ilog2(sdebug_sector_size));
 	if (meta_data_locked)
 		sdeb_meta_write_unlock(sip);
 
@@ -5349,9 +5349,10 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 		 * writes behaviour as possible.
 		 */
 		ret = do_device_access(sip, scp, sg_off, lba, num, group, true, true);
-		/* If ZBC zone then bump its write pointer */
-		if (sdebug_dev_is_zoned(devip))
-			zbc_inc_wp(devip, lba, num);
+		/* If ZBC zone then bump its write pointer over the data written */
+		if (sdebug_dev_is_zoned(devip) && ret > 0)
+			zbc_inc_wp(devip, lba,
+				   ret >> ilog2(sdebug_sector_size));
 		if (unlikely(scsi_debug_lbp()))
 			map_region(sip, lba, num);
 		if (unlikely(-1 == ret)) {
-- 
2.55.0

[PATCH v5 09/10] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[Niklas Cassel]

When logical block provisioning is enabled, a command that writes user
data marks the region that it wrote in the provisioning map, so that
GET LBA STATUS reports the region as mapped. resp_write_dt0(),
resp_write_scat() and resp_write_same() all call map_region() for that.

resp_atomic_write() does not, so a WRITE ATOMIC (16) leaves the
provisioning map untouched, and GET LBA STATUS keeps reporting the
region as deallocated after it has been written. map_state(), which
GET LBA STATUS uses, is the only reader of the map, so that is the whole
of the effect.

Call map_region() the way resp_write_dt0() does, and take the zone
metadata write lock across the access as it does when logical block
provisioning is enabled. That lock is what serialises the provisioning
map against resp_unmap(), which holds it while unmap_region() clears map
bits and zeroes the data that they cover. resp_unmap() does nothing
unless logical block provisioning is enabled, so the lock is only needed
in that case.

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

  modprobe scsi_debug sector_size=512 physblk_exp=3 dev_size_mb=128 \
      atomic_wr=1 lbpu=1

GET LBA STATUS reports a region that has never been written as
deallocated, and reports it as mapped after a WRITE ATOMIC (16) of eight
blocks. Only an ordinary WRITE did so before this patch.
---

Changes since v4: the meta_data_locked variable is gone, as suggested.
 drivers/scsi/scsi_debug.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d07f6f891951..2e1a02c383a3 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -6202,6 +6202,7 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
 	u8 *cmd = scp->cmnd;
 	u16 boundary, len;
 	u64 lba, lba_tmp;
+	bool lbp = scsi_debug_lbp();
 	int ret;
 
 	if (!scsi_debug_atomic_write()) {
@@ -6246,7 +6247,16 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
 		}
 	}
 
+	if (lbp)
+		sdeb_meta_write_lock(sip);
+
 	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
+	if (unlikely(lbp))
+		map_region(sip, lba, len);
+
+	if (lbp)
+		sdeb_meta_write_unlock(sip);
+
 	if (unlikely(ret == -1))
 		return DID_ERROR << 16;
 	if (unlikely(ret != len * sdebug_sector_size))
-- 
2.55.0

[PATCH v5 10/10] scsi: scsi_debug: Validate the access parameters of WRITE ATOMIC (16)

[Niklas Cassel]

resp_atomic_write() validates the fields that are specific to an atomic
write, the alignment and granularity of the transfer, the atomic
boundary and the maximum transfer length, but it never validates the
range that the command addresses. Every other command that writes user
data calls check_device_access_params() first, which rejects a transfer
that ends beyond the capacity of the device, one whose length exceeds
the size of the store, and any write to a write protected device.

Call check_device_access_params(). The zone checks that it ends with are
unreachable, as atomic writes and ZBC emulation are mutually exclusive.

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

  modprobe scsi_debug sector_size=512 physblk_exp=3 dev_size_mb=128 \
      atomic_wr=1

The device has a capacity of 262144 logical blocks. A WRITE ATOMIC (16)
of eight blocks at LBA 262140 is now terminated with ILLEGAL REQUEST /
LOGICAL BLOCK ADDRESS OUT OF RANGE; before this patch it completed with
GOOD status, having written eight blocks at LBA 0 instead.

With the wp module parameter set to 1, a WRITE ATOMIC (16) is now
terminated with DATA PROTECT / LOGICAL UNIT SOFTWARE WRITE PROTECTED,
which is what an ordinary WRITE(16) has always returned; before this
patch it completed with GOOD status and wrote the data.
---
 drivers/scsi/scsi_debug.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 2e1a02c383a3..4f4d8f9f19c4 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -6247,6 +6247,10 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
 		}
 	}
 
+	ret = check_device_access_params(scp, lba, len, true);
+	if (ret)
+		return ret;
+
 	if (lbp)
 		sdeb_meta_write_lock(sip);
 
-- 
2.55.0

Re: [PATCH v5 09/10] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[John Garry]

On 9/21/26 16:40, Niklas Cassel wrote:
> When logical block provisioning is enabled, a command that writes user
> data marks the region that it wrote in the provisioning map, so that
> GET LBA STATUS reports the region as mapped. resp_write_dt0(),
> resp_write_scat() and resp_write_same() all call map_region() for that.
> 
> resp_atomic_write() does not, so a WRITE ATOMIC (16) leaves the
> provisioning map untouched, and GET LBA STATUS keeps reporting the
> region as deallocated after it has been written. map_state(), which
> GET LBA STATUS uses, is the only reader of the map, so that is the whole
> of the effect.
> 
> Call map_region() the way resp_write_dt0() does, and take the zone
> metadata write lock across the access as it does when logical block
> provisioning is enabled. That lock is what serialises the provisioning
> map against resp_unmap(), which holds it while unmap_region() clears map
> bits and zeroes the data that they cover. resp_unmap() does nothing
> unless logical block provisioning is enabled, so the lock is only needed
> in that case.
> 
> Assisted-by: LLM
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> Tested with:
> 
>    modprobe scsi_debug sector_size=512 physblk_exp=3 dev_size_mb=128 \
>        atomic_wr=1 lbpu=1
> 
> GET LBA STATUS reports a region that has never been written as
> deallocated, and reports it as mapped after a WRITE ATOMIC (16) of eight
> blocks. Only an ordinary WRITE did so before this patch.
> ---
> 
> Changes since v4: the meta_data_locked variable is gone, as suggested.
>   drivers/scsi/scsi_debug.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index d07f6f891951..2e1a02c383a3 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -6202,6 +6202,7 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
>   	u8 *cmd = scp->cmnd;
>   	u16 boundary, len;
>   	u64 lba, lba_tmp;
> +	bool lbp = scsi_debug_lbp();
>   	int ret;
>   
>   	if (!scsi_debug_atomic_write()) {
> @@ -6246,7 +6247,16 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
>   		}
>   	}
>   
> +	if (lbp)
> +		sdeb_meta_write_lock(sip);
> +
>   	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
> +	if (unlikely(lbp))

Is this supposed to be called if ret <= 0?

> +		map_region(sip, lba, len);
> +
> +	if (lbp)

Ignoring comment above, why not combine into a single if statement?

> +		sdeb_meta_write_unlock(sip);
 > +>   	if (unlikely(ret == -1))
>   		return DID_ERROR << 16;
>   	if (unlikely(ret != len * sdebug_sector_size))

Re: [PATCH v5 04/10] scsi: scsi_debug: Evaluate scsi_debug_lbp() only once

[John Garry]

On 9/21/26 16:40, Niklas Cassel wrote:
> 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
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: John Garry <john.garry@linux.dev>

> ---
> 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)) {

combine lines? Please consider elsewhere in this patch. However I think 
that we still like to enforce the 80 character line limit... well, some do.

Thanks

>   		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))

Re: [PATCH v5 04/10] scsi: scsi_debug: Evaluate scsi_debug_lbp() only once

[Johannes Thumshirn]

On Mon, Sep 21, 2026 at 05:40:20PM +0200, Niklas Cassel wrote:
>  	if (sdebug_dev_is_zoned(devip) ||
>  	    sdebug_dix ||
> -	    scsi_debug_lbp())  {
> +	    lbp)  {

Nit, lbp would now perfectly fit on the previous line.

>  		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)) {

Same

>  		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;
>  	}

And here as well

Re: [PATCH v5 09/10] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[Niklas Cassel]

Hello John,

On Mon, Sep 21, 2026 at 04:50:31PM +0100, John Garry wrote:
> On 9/21/26 16:40, Niklas Cassel wrote:
> > @@ -6246,7 +6247,16 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
> >		}
> >	}
> > +	if (lbp)
> > +		sdeb_meta_write_lock(sip);
> > +
> >	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
> > +	if (unlikely(lbp))
>
> Is this supposed to be called if ret <= 0?
>
> > +		map_region(sip, lba, len);
> > +
> > +	if (lbp)

It does not have to be skipped: SBC-6 4.7.4.6.2 allows a device to map a
deallocated LBA at any time, and resp_write_dt0() and resp_write_scat()
also call map_region() without looking at the result, so I will keep it
unconditional.

Looking at this, I did however find another bug in resp_atomic_write().
While scsi_debug does fail a short WRITE ATOMIC (16) with DID_ERROR,
a short write will currently modify the logical blocks up to the short
write. I will add a fix for this in my series.


Kind regards,
Niklas

Re: [PATCH v5 09/10] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[John Garry]

On 9/24/26 12:10, Niklas Cassel wrote:
> It does not have to be skipped: SBC-6 4.7.4.6.2 allows a device to map a
> deallocated LBA at any time, and resp_write_dt0() and resp_write_scat()
> also call map_region() without looking at the result, so I will keep it
> unconditional.

Maybe you comment on this in map_region()