[19/21] scsi: sd: Add WRITE_ATOMIC_16 support
Commit Message
Add function sd_setup_atomic_cmnd() to setup an WRITE_ATOMIC_16
CDB for when REQ_ATOMIC flag is set for the request.
Also add trace info.
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
drivers/scsi/scsi_trace.c | 22 ++++++++++++++++++++++
drivers/scsi/sd.c | 20 ++++++++++++++++++++
include/scsi/scsi_proto.h | 1 +
include/trace/events/scsi.h | 1 +
4 files changed, 44 insertions(+)
Comments
On 9/29/23 03:27, John Garry wrote:
> +static blk_status_t sd_setup_atomic_cmnd(struct scsi_cmnd *cmd,
> + sector_t lba, unsigned int nr_blocks,
> + unsigned char flags)
> +{
> + cmd->cmd_len = 16;
> + cmd->cmnd[0] = WRITE_ATOMIC_16;
> + cmd->cmnd[1] = flags;
> + put_unaligned_be64(lba, &cmd->cmnd[2]);
> + cmd->cmnd[10] = 0;
> + cmd->cmnd[11] = 0;
> + put_unaligned_be16(nr_blocks, &cmd->cmnd[12]);
> + cmd->cmnd[14] = 0;
> + cmd->cmnd[15] = 0;
> +
> + return BLK_STS_OK;
> +}
Please store the 'dld' value in the GROUP NUMBER field. See e.g.
sd_setup_rw16_cmnd().
> @@ -1139,6 +1156,7 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
> unsigned int nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
> unsigned int mask = logical_to_sectors(sdp, 1) - 1;
> bool write = rq_data_dir(rq) == WRITE;
> + bool atomic_write = !!(rq->cmd_flags & REQ_ATOMIC) && write;
Please leave out the superfluous "!!".
Thanks,
Bart.
On 29/09/2023 18:59, Bart Van Assche wrote:
> On 9/29/23 03:27, John Garry wrote:
>> +static blk_status_t sd_setup_atomic_cmnd(struct scsi_cmnd *cmd,
>> + sector_t lba, unsigned int nr_blocks,
>> + unsigned char flags)
>> +{
>> + cmd->cmd_len = 16;
>> + cmd->cmnd[0] = WRITE_ATOMIC_16;
>> + cmd->cmnd[1] = flags;
>> + put_unaligned_be64(lba, &cmd->cmnd[2]);
>> + cmd->cmnd[10] = 0;
>> + cmd->cmnd[11] = 0;
>> + put_unaligned_be16(nr_blocks, &cmd->cmnd[12]);
>> + cmd->cmnd[14] = 0;
>> + cmd->cmnd[15] = 0;
>> +
>> + return BLK_STS_OK;
>> +}
>
> Please store the 'dld' value in the GROUP NUMBER field. See e.g.
> sd_setup_rw16_cmnd().
Are you sure that WRITE ATOMIC (16) supports dld?
>
>> @@ -1139,6 +1156,7 @@ static blk_status_t
>> sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
>> unsigned int nr_blocks = sectors_to_logical(sdp,
>> blk_rq_sectors(rq));
>> unsigned int mask = logical_to_sectors(sdp, 1) - 1;
>> bool write = rq_data_dir(rq) == WRITE;
>> + bool atomic_write = !!(rq->cmd_flags & REQ_ATOMIC) && write;
>
> Please leave out the superfluous "!!".
ok, fine.
Thanks,
John
On 10/2/23 04:36, John Garry wrote:
> On 29/09/2023 18:59, Bart Van Assche wrote:
>> On 9/29/23 03:27, John Garry wrote:
>>> +static blk_status_t sd_setup_atomic_cmnd(struct scsi_cmnd *cmd,
>>> + sector_t lba, unsigned int nr_blocks,
>>> + unsigned char flags)
>>> +{
>>> + cmd->cmd_len = 16;
>>> + cmd->cmnd[0] = WRITE_ATOMIC_16;
>>> + cmd->cmnd[1] = flags;
>>> + put_unaligned_be64(lba, &cmd->cmnd[2]);
>>> + cmd->cmnd[10] = 0;
>>> + cmd->cmnd[11] = 0;
>>> + put_unaligned_be16(nr_blocks, &cmd->cmnd[12]);
>>> + cmd->cmnd[14] = 0;
>>> + cmd->cmnd[15] = 0;
>>> +
>>> + return BLK_STS_OK;
>>> +}
>>
>> Please store the 'dld' value in the GROUP NUMBER field. See e.g.
>> sd_setup_rw16_cmnd().
>
> Are you sure that WRITE ATOMIC (16) supports dld?
Hi John,
I was assuming that DLD would be supported by the WRITE ATOMIC(16)
command. After having taken another look at the latest SBC-5 draft
I see that the DLD2/DLD1/DLD0 bits are not present in the WRITE
ATOMIC(16) command. So please ignore my comment above.
Thanks,
Bart.
@@ -325,6 +325,26 @@ scsi_trace_zbc_out(struct trace_seq *p, unsigned char *cdb, int len)
return ret;
}
+static const char *
+scsi_trace_atomic_write16_out(struct trace_seq *p, unsigned char *cdb, int len)
+{
+ const char *ret = trace_seq_buffer_ptr(p);
+ unsigned int boundary_size;
+ unsigned int nr_blocks;
+ sector_t lba;
+
+ lba = get_unaligned_be64(&cdb[2]);
+ boundary_size = get_unaligned_be16(&cdb[10]);
+ nr_blocks = get_unaligned_be16(&cdb[12]);
+
+ trace_seq_printf(p, "lba=%llu txlen=%u boundary_size=%u",
+ lba, nr_blocks, boundary_size);
+
+ trace_seq_putc(p, 0);
+
+ return ret;
+}
+
static const char *
scsi_trace_varlen(struct trace_seq *p, unsigned char *cdb, int len)
{
@@ -385,6 +405,8 @@ scsi_trace_parse_cdb(struct trace_seq *p, unsigned char *cdb, int len)
return scsi_trace_zbc_in(p, cdb, len);
case ZBC_OUT:
return scsi_trace_zbc_out(p, cdb, len);
+ case WRITE_ATOMIC_16:
+ return scsi_trace_atomic_write16_out(p, cdb, len);
default:
return scsi_trace_misc(p, cdb, len);
}
@@ -1129,6 +1129,23 @@ static int sd_cdl_dld(struct scsi_disk *sdkp, struct scsi_cmnd *scmd)
return (hint - IOPRIO_HINT_DEV_DURATION_LIMIT_1) + 1;
}
+static blk_status_t sd_setup_atomic_cmnd(struct scsi_cmnd *cmd,
+ sector_t lba, unsigned int nr_blocks,
+ unsigned char flags)
+{
+ cmd->cmd_len = 16;
+ cmd->cmnd[0] = WRITE_ATOMIC_16;
+ cmd->cmnd[1] = flags;
+ put_unaligned_be64(lba, &cmd->cmnd[2]);
+ cmd->cmnd[10] = 0;
+ cmd->cmnd[11] = 0;
+ put_unaligned_be16(nr_blocks, &cmd->cmnd[12]);
+ cmd->cmnd[14] = 0;
+ cmd->cmnd[15] = 0;
+
+ return BLK_STS_OK;
+}
+
static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
{
struct request *rq = scsi_cmd_to_rq(cmd);
@@ -1139,6 +1156,7 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
unsigned int nr_blocks = sectors_to_logical(sdp, blk_rq_sectors(rq));
unsigned int mask = logical_to_sectors(sdp, 1) - 1;
bool write = rq_data_dir(rq) == WRITE;
+ bool atomic_write = !!(rq->cmd_flags & REQ_ATOMIC) && write;
unsigned char protect, fua;
unsigned int dld;
blk_status_t ret;
@@ -1200,6 +1218,8 @@ static blk_status_t sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
if (protect && sdkp->protection_type == T10_PI_TYPE2_PROTECTION) {
ret = sd_setup_rw32_cmnd(cmd, write, lba, nr_blocks,
protect | fua, dld);
+ } else if (atomic_write) {
+ ret = sd_setup_atomic_cmnd(cmd, lba, nr_blocks, protect | fua);
} else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) {
ret = sd_setup_rw16_cmnd(cmd, write, lba, nr_blocks,
protect | fua, dld);
@@ -119,6 +119,7 @@
#define WRITE_SAME_16 0x93
#define ZBC_OUT 0x94
#define ZBC_IN 0x95
+#define WRITE_ATOMIC_16 0x9c
#define SERVICE_ACTION_BIDIRECTIONAL 0x9d
#define SERVICE_ACTION_IN_16 0x9e
#define SERVICE_ACTION_OUT_16 0x9f
@@ -102,6 +102,7 @@
scsi_opcode_name(WRITE_32), \
scsi_opcode_name(WRITE_SAME_32), \
scsi_opcode_name(ATA_16), \
+ scsi_opcode_name(WRITE_ATOMIC_16), \
scsi_opcode_name(ATA_12))
#define scsi_hostbyte_name(result) { result, #result }