Re: [PATCH v3 2/6] scsi: scsi_debug: Do not write a partial physical block

[Niklas Cassel <cassel@kernel.org>]

On Thu, Sep 17, 2026 at 08:11:39PM +0700, Damien Le Moal wrote:
> > @@ -4290,6 +4290,19 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
> >  
> >  	fsp = sip->storep;
> >  
> > +	/*
> > +	 * A data-out buffer that does not hold all of the data that the
> > +	 * command asks for is written up to the last whole physical block
> > +	 * that it does hold, so that a partial physical block is never
> > +	 * written. The bytes that are left over are reported as a residual.
> > +	 */
> > +	if (do_write) {
> > +		u32 avail = (sdb->length - sg_skip) / sdebug_sector_size;
> > +
> > +		if (avail < num)
> > +			num = round_down(avail, 1U << sdebug_physblk_exp);
> 
> Nope, that is not correct. physical sector unaligned writes are OK with regular
> disks. They are not for ZBC, but we should check that in
> check_zbc_access_params() I think.

Patch [4/6] scsi: scsi_debug: Enforce physical block alignment of zonedwrites

does add a check for SWR zones, and for SWR zone only, which errors out if
the write is not aligned to the physical block size.

However, in the case of a short data-out buffer, the request is valid, so I
don't think that check_zbc_access_params() is the right place for the above
check.

But you are right that the check in do_device_access() should be gated on
SWR zones as well...


Something like this:

@@ -4263,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;
 
@@ -4290,6 +4302,23 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
 
        fsp = sip->storep;
 
+       /*
+        * For SWR zones, a data-out buffer that does not hold all of the data
+        * that the command asks for is written up to the last whole physical
+        * block that it does hold, so that a partial physical block is never
+        * written. The bytes that are left over are reported as a 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) / 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 */



Kind regards,
Niklas