Linux SCSI subsystem development
 help / color / mirror / Atom feed
From: Damien Le Moal <dlemoal@kernel.org>
To: Niklas Cassel <cassel@kernel.org>,
	"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
	"Martin K. Petersen" <mkp@kernel.org>
Cc: linux-scsi@vger.kernel.org, John Garry <john.garry@linux.dev>
Subject: Re: [PATCH v10 06/12] scsi: scsi_debug: Report the residual of a write
Date: Mon, 28 Sep 2026 10:18:33 +0200	[thread overview]
Message-ID: <282c5adb-529b-4e02-b741-15082cc8a82b@kernel.org> (raw)
In-Reply-To: <20260928072102.725566-20-cassel@kernel.org>

On 2026/09/28 9:21, Niklas Cassel wrote:
> Documentation/scsi/scsi_mid_low_api.rst defines the residual as the
> length of the data buffer less the number of bytes actually transferred,
> so a write whose data buffer is larger than its transfer length leaves
> one. resp_read_dt0() and resp_write_tape() report it, but the write
> paths for a disk do not, so such a write completes with GOOD status and
> a residual of zero while a READ of the same length into the same buffer
> reports it correctly.
> 
> Report it in resp_write_dt0(), resp_write_scat() and resp_atomic_write().
> 
> resp_write_scat() differs twice over. Its data-out buffer also holds the
> parameter list header and the LBA range descriptors, so what the command
> consumed is sg_off rather than the bytes that the last range
> transferred; and sg_off counts what was asked for rather than what was
> transferred, so it can exceed the buffer and needs a guard. A command
> with no LBA range descriptors or a buffer transfer length of zero
> transfers nothing, so the whole buffer is residual. An error, real or
> injected, can end the command after some ranges have been written, so
> the residual is reported on every exit once the parameter list has been
> fetched.
> 
> Assisted-by: LLM
> 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.
> 
> Changes since v9: the commit message cites the definition of the
> residual in Documentation/scsi/scsi_mid_low_api.rst.
> ---
>  drivers/scsi/scsi_debug.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 8f9d54269dce..691a5ad56161 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -5162,6 +5162,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);

May be do this under a if:

	if (ret < scsi_bufflen(scp))
		scsi_set_resid(scp, scsi_bufflen(scp) - ret);

> @@ -6208,6 +6221,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);

And here too.

>  	return 0;
>  }

With that,

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

  reply	other threads:[~2026-09-28  8:18 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28  7:21 [PATCH v10 00/12] scsi: scsi_debug: fix zoned write validation Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 01/12] scsi: scsi_debug: Refuse a zoned device with a non-zero lowest aligned LBA Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 02/12] scsi: scsi_debug: Make atomic writes and ZBC emulation mutually exclusive Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 03/12] scsi: scsi_debug: Take the zone metadata lock before the data lock Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 04/12] scsi: scsi_debug: Evaluate scsi_debug_lbp() only once Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 05/12] scsi: scsi_debug: Avoid 32-bit overflow in WRITE SCATTERED offsets Niklas Cassel
2026-09-28  7:39   ` sashiko-bot
2026-09-28  7:21 ` [PATCH v10 06/12] scsi: scsi_debug: Report the residual of a write Niklas Cassel
2026-09-28  8:18   ` Damien Le Moal [this message]
2026-09-28  7:21 ` [PATCH v10 07/12] scsi: scsi_debug: Enforce physical block alignment of zoned writes Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 08/12] scsi: scsi_debug: Do not write a partial physical block to a zoned device Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 09/12] scsi: scsi_debug: Advance the write pointer over the data written Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 10/12] scsi: scsi_debug: Refuse a short WRITE ATOMIC (16) before writing it Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 11/12] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16) Niklas Cassel
2026-09-28  7:21 ` [PATCH v10 12/12] 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=282c5adb-529b-4e02-b741-15082cc8a82b@kernel.org \
    --to=dlemoal@kernel.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=cassel@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