Expected payload size for WRITE_SAME_16?

DISCUSSION Nicholas A. Bellinger 2 participants 2 messages May 7, 2011 – May 9, 2011 1 message not in the archive SCSI

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Nicholas A. Bellinger

author

2011-05-07 01:06 UTC #1 lore raw

Quoted 2 lines · Chris Greiveldinger

Hello again,

Hi Chris,

Quoted 6 lines

From sbc3r25: "The WRITE SAME (16) command (see table 112) requests
that the device server transfer a single logical block from the data-out buffer." The code for WRITE_SAME_16 in target_core_transport.c:transport_generic_cmd_sequencer() calculates the expected size to be sectors * block size (via transport_get_size), which I expect is too large if sectors is greater than one.

Not exactly..

We use the per CDB 'size = transport_get_size()' assignment with WRITE_SAME_16+UNMAP=1 in transport_generic_cmd_sequencer() to compare the SCSI CDB level expected data transfer length (size) against the fabric dependent expected transfer length (struct se_cmd->data_length) at the bottom of transport_generic_cmd_sequencer().

The value in se_cmd->data_length is then used to determine the 'range' and makes the backend calls via:

target_core_cdb.c:target_emulate_write_same()

       dev->transport->do_discard() -> 
         target_core_iblock.c:iblock_do_discard() -> 
              block/blk-lib.c:blkdev_issue_discard()

Quoted 5 lines

Since the sg3_utils sg_write_same utility allows me to specify the the payload size, I can issue a command that has the payload length that transport_generic_cmd_sequencer() expects, but I'm not sure what the correct size should be.

It was my understanding that you need to match the sg_write_same parameters of --num and --xferlen depending on the SCSI block_size (512) used for the SCSI devices:

sg_write_same -S --unmap --in=/dev/zero --lba=10 --num=1 --xferlen=512 /dev/sdd

sg_write_same -S --unmap --in=/dev/zero --lba=10 --num=100 --xferlen=51200 /dev/sdd

Note that sg_write_same does check --xferlen against a hardcoded max of 64k, which is obviously somewhat limiting.

Quoted 3 lines

Am I wrong in my interpretation of the WRITE_SAME(16) command, or is this a bug in transport_generic_cmd_sequencer()?

So the above case --num > 0 case things should still be working as expected with recent upstream LIO code and your last Reported-by: patch, but there does appear to be an issue with the --num=0 case being rejected by the write underflow/overflow check inside transport_generic_cmd_sequencer().

I will send out a patch shortly against lio-4.1 for you to test that makes the sg_write_same --unmap + --num=0 case work again (Christoph CC'ed and linux-scsi CC'ed).

Thanks for your review!

--nab

Douglas Gilbert

2011-05-09 19:00 UTC #2 lore raw

Quoted 40 lines · Nicholas A. Bellinger

Quoted 2 lines · Chris Greiveldinger
Hello again,
Hi Chris,
Quoted 6 lines
From sbc3r25: "The WRITE SAME (16) command (see table 112) requests
that the device server transfer a single logical block from the data-out buffer." The code for WRITE_SAME_16 in target_core_transport.c:transport_generic_cmd_sequencer() calculates the expected size to be sectors * block size (via transport_get_size), which I expect is too large if sectors is greater than one.
Not exactly..
We use the per CDB 'size = transport_get_size()' assignment with WRITE_SAME_16+UNMAP=1 in transport_generic_cmd_sequencer() to compare the SCSI CDB level expected data transfer length (size) against the fabric dependent expected transfer length (struct se_cmd->data_length) at the bottom of transport_generic_cmd_sequencer().
The value in se_cmd->data_length is then used to determine the 'range' and makes the backend calls via:
    target_core_cdb.c:target_emulate_write_same()
        dev->transport->do_discard() ->
          target_core_iblock.c:iblock_do_discard() ->
               block/blk-lib.c:blkdev_issue_discard()
Quoted 5 lines
Since the sg3_utils sg_write_same utility allows me to specify the the payload size, I can issue a command that has the payload length that transport_generic_cmd_sequencer() expects, but I'm not sure what the correct size should be.
It was my understanding that you need to match the sg_write_same parameters of --num and --xferlen depending on the SCSI block_size (512) used for the SCSI devices:

Quoted 5 lines

sg_write_same -S --unmap --in=/dev/zero --lba=10 --num=1 --xferlen=512 /dev/sdd
sg_write_same -S --unmap --in=/dev/zero --lba=10 --num=100 --xferlen=51200 /dev/sdd

No, it should be '--xferlen=512' in both cases or simply don't give that option. If it is not given then the READ CAPACITY response is consulted to read the 'Logical block length in bytes' field which I guess will be 512 in the cases you are looking at.

Perhaps 'man sg_write_same' needs some examples (or just needs to be read ...).

Quoted 2 lines

Note that sg_write_same does check --xferlen against a hardcoded max of 64k, which is obviously somewhat limiting.

The 64Kb limit is the biggest block size that sg_write_same can handle. I'm not aware that is a practical restriction yet.

Doug Gilbert

Quoted 19 lines

Quoted 3 lines
Am I wrong in my interpretation of the WRITE_SAME(16) command, or is this a bug in transport_generic_cmd_sequencer()?
So the above case --num> 0 case things should still be working as expected with recent upstream LIO code and your last Reported-by: patch, but there does appear to be an issue with the --num=0 case being rejected by the write underflow/overflow check inside transport_generic_cmd_sequencer().
I will send out a patch shortly against lio-4.1 for you to test that makes the sg_write_same --unmap + --num=0 case work again (Christoph CC'ed and linux-scsi CC'ed).
Thanks for your review!
--nab