diff mbox series

[19/21] scsi: sd: Add WRITE_ATOMIC_16 support

Message ID 20230929102726.2985188-20-john.g.garry@oracle.com
State New
Headers
Series block atomic writes |

Commit Message

John Garry Sept. 29, 2023, 10:27 a.m. UTC 
  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

Bart Van Assche Sept. 29, 2023, 5:59 p.m. UTC | #1 
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.
John Garry Oct. 2, 2023, 11:36 a.m. UTC | #2 
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
Bart Van Assche Oct. 2, 2023, 7:21 p.m. UTC | #3 
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.
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_trace.c b/drivers/scsi/scsi_trace.c
index 41a950075913..3e47c4472a80 100644
--- a/drivers/scsi/scsi_trace.c
+++ b/drivers/scsi/scsi_trace.c
@@ -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);
 	}
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 7f6cadd1f8f3..1a41656dac2d 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -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);
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index 07d65c1f59db..833de67305b5 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -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
diff --git a/include/trace/events/scsi.h b/include/trace/events/scsi.h
index 8e2d9b1b0e77..05f1945ed204 100644
--- a/include/trace/events/scsi.h
+++ b/include/trace/events/scsi.h
@@ -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 }