Re: [PATCH v7 09/11] scsi: scsi_debug: Refuse a short WRITE ATOMIC (16) before writing it

[John Garry <john.garry@linux.dev>]

On 9/26/26 19:03, Niklas Cassel wrote:
> Hello John,
> 
> On Fri, Sep 25, 2026 at 10:30:11AM +0100, John Garry wrote:
>>> ---
>>> Tested with:
>>>
>>>     modprobe scsi_debug sector_size=512 physblk_exp=3 dev_size_mb=128 \
>>>         atomic_wr=1 lbpu=1
>>>
>>> issuing a WRITE ATOMIC (16) of eight blocks through SG_IO with a buffer
>>> of four blocks of 0xaa. It fails with DID_ERROR before and after this
>>> patch, but before it the first four blocks read back as 0xaa afterwards,
>>> and after it all eight read back as they were. A WRITE ATOMIC (16) with
>>> a full buffer writes all eight blocks, as before.
>>>
>>
>> What is the command which you use?
> 
> SG_IO, with a CDB for eight blocks and a buffer of four:
> sg_raw -s 2048 -i aa4 /dev/sdX 9c 00 00 00 00 00 00 00 00 40 00 00 00 08 00 00
> 
> 
>>
>> I am just wondering if this check should go into sd_setup_atomic_cmnd() or
>> somewhere else higher up. I mean, this check is not really specific to
>> scsi_debug, right?
> 
> sd can't produce a short atomic write: sd_setup_read_write_cmnd() derives
> nr_blocks from blk_rq_sectors(), and the data buffer is the same request,
> so the transfer length and the buffer always match.
> 
> A mismatch is only possible through SG_IO, where the caller supplies both.
> 
> SBC-6 4.28.2 puts the requirement on the device server: a failed atomic
> write must leave the LBAs unaltered. scsi_debug is the device server here,
> so I think this is the right place for it.
> 
OK, I think that the important information is "4.28.2 requires one that 
cannot complete to leave the LBAs that it specifies unaltered", so maybe 
add a small comment for that.

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