| Message ID | 20201005084130.143273-5-hch@lst.de |
|---|---|
| State | New |
| Headers | show |
| Series | [01/10] scsi: don't export scsi_device_from_queue | expand |
On 10/5/20 10:41 AM, Christoph Hellwig wrote: > Directly access the cdb array like we do everywhere else insted of > overlaying a structure on top of it. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/scsi/scsi_logging.c | 2 +- > include/scsi/scsi_common.h | 11 +++-------- > include/scsi/scsi_proto.h | 10 ---------- > 3 files changed, 4 insertions(+), 19 deletions(-) > > diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c > index 8ea44c6595efa7..b6222df7254a3a 100644 > --- a/drivers/scsi/scsi_logging.c > +++ b/drivers/scsi/scsi_logging.c > @@ -111,7 +111,7 @@ static size_t scsi_format_opcode_name(char *buffer, size_t buf_len, > > cdb0 = cdbp[0]; > if (cdb0 == VARIABLE_LENGTH_CMD) { > - int len = scsi_varlen_cdb_length(cdbp); > + int len = cdbp[7] + 8; > > if (len < 10) { > off = scnprintf(buffer, buf_len, > diff --git a/include/scsi/scsi_common.h b/include/scsi/scsi_common.h > index 731ac09ed23135..297fc1881607b6 100644 > --- a/include/scsi/scsi_common.h > +++ b/include/scsi/scsi_common.h > @@ -9,20 +9,15 @@ > #include <linux/types.h> > #include <scsi/scsi_proto.h> > > -static inline unsigned > -scsi_varlen_cdb_length(const void *hdr) > -{ > - return ((struct scsi_varlen_cdb_hdr *)hdr)->additional_cdb_length + 8; > -} > - > extern const unsigned char scsi_command_size_tbl[8]; > #define COMMAND_SIZE(opcode) scsi_command_size_tbl[((opcode) >> 5) & 7] > > static inline unsigned > scsi_command_size(const unsigned char *cmnd) > { > - return (cmnd[0] == VARIABLE_LENGTH_CMD) ? > - scsi_varlen_cdb_length(cmnd) : COMMAND_SIZE(cmnd[0]); > + if (cmnd[0] == VARIABLE_LENGTH_CMD) > + return cmnd[7] + 8; > + return COMMAND_SIZE(cmnd[0]); > } > > /* Returns a human-readable name for the device */ > diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h > index c3686011193224..c57f9cd8185526 100644 > --- a/include/scsi/scsi_proto.h > +++ b/include/scsi/scsi_proto.h > @@ -176,16 +176,6 @@ > > #define SCSI_MAX_VARLEN_CDB_SIZE 260 > > -/* defined in T10 SCSI Primary Commands-2 (SPC2) */ > -struct scsi_varlen_cdb_hdr { > - __u8 opcode; /* opcode always == VARIABLE_LENGTH_CMD */ > - __u8 control; > - __u8 misc[5]; > - __u8 additional_cdb_length; /* total cdb length - 8 */ > - __be16 service_action; > - /* service specific data follows */ > -}; > - > /* > * SCSI Architecture Model (SAM) Status codes. Taken from SAM-3 draft > * T10/1561-D Revision 4 Draft dated 7th November 2002. > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
On 10/5/20 1:41 AM, Christoph Hellwig wrote: > diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c > index 8ea44c6595efa7..b6222df7254a3a 100644 > --- a/drivers/scsi/scsi_logging.c > +++ b/drivers/scsi/scsi_logging.c > @@ -111,7 +111,7 @@ static size_t scsi_format_opcode_name(char *buffer, size_t buf_len, > > cdb0 = cdbp[0]; > if (cdb0 == VARIABLE_LENGTH_CMD) { > - int len = scsi_varlen_cdb_length(cdbp); > + int len = cdbp[7] + 8; > > if (len < 10) { > off = scnprintf(buffer, buf_len, > diff --git a/include/scsi/scsi_common.h b/include/scsi/scsi_common.h > index 731ac09ed23135..297fc1881607b6 100644 > --- a/include/scsi/scsi_common.h > +++ b/include/scsi/scsi_common.h > @@ -9,20 +9,15 @@ > #include <linux/types.h> > #include <scsi/scsi_proto.h> > > -static inline unsigned > -scsi_varlen_cdb_length(const void *hdr) > -{ > - return ((struct scsi_varlen_cdb_hdr *)hdr)->additional_cdb_length + 8; > -} > - > extern const unsigned char scsi_command_size_tbl[8]; > #define COMMAND_SIZE(opcode) scsi_command_size_tbl[((opcode) >> 5) & 7] > > static inline unsigned > scsi_command_size(const unsigned char *cmnd) > { > - return (cmnd[0] == VARIABLE_LENGTH_CMD) ? > - scsi_varlen_cdb_length(cmnd) : COMMAND_SIZE(cmnd[0]); > + if (cmnd[0] == VARIABLE_LENGTH_CMD) > + return cmnd[7] + 8; > + return COMMAND_SIZE(cmnd[0]); > } > > /* Returns a human-readable name for the device */ > diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h > index c3686011193224..c57f9cd8185526 100644 > --- a/include/scsi/scsi_proto.h > +++ b/include/scsi/scsi_proto.h > @@ -176,16 +176,6 @@ > > #define SCSI_MAX_VARLEN_CDB_SIZE 260 > > -/* defined in T10 SCSI Primary Commands-2 (SPC2) */ > -struct scsi_varlen_cdb_hdr { > - __u8 opcode; /* opcode always == VARIABLE_LENGTH_CMD */ > - __u8 control; > - __u8 misc[5]; > - __u8 additional_cdb_length; /* total cdb length - 8 */ > - __be16 service_action; > - /* service specific data follows */ > -}; I'm OK with removing struct scsi_varlen_cdb_hdr but not with the removal of the scsi_varlen_cdb_length() function. I'd like to keep that function because I think it makes code that handles variable length CDBs easier to read. Thanks, Bart.
Bart, > I'm OK with removing struct scsi_varlen_cdb_hdr but not with the > removal of the scsi_varlen_cdb_length() function. I'd like to keep > that function because I think it makes code that handles variable > length CDBs easier to read. I agree. Please drop the header and simplify the wrapper function.
diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c index 8ea44c6595efa7..b6222df7254a3a 100644 --- a/drivers/scsi/scsi_logging.c +++ b/drivers/scsi/scsi_logging.c @@ -111,7 +111,7 @@ static size_t scsi_format_opcode_name(char *buffer, size_t buf_len, cdb0 = cdbp[0]; if (cdb0 == VARIABLE_LENGTH_CMD) { - int len = scsi_varlen_cdb_length(cdbp); + int len = cdbp[7] + 8; if (len < 10) { off = scnprintf(buffer, buf_len, diff --git a/include/scsi/scsi_common.h b/include/scsi/scsi_common.h index 731ac09ed23135..297fc1881607b6 100644 --- a/include/scsi/scsi_common.h +++ b/include/scsi/scsi_common.h @@ -9,20 +9,15 @@ #include <linux/types.h> #include <scsi/scsi_proto.h> -static inline unsigned -scsi_varlen_cdb_length(const void *hdr) -{ - return ((struct scsi_varlen_cdb_hdr *)hdr)->additional_cdb_length + 8; -} - extern const unsigned char scsi_command_size_tbl[8]; #define COMMAND_SIZE(opcode) scsi_command_size_tbl[((opcode) >> 5) & 7] static inline unsigned scsi_command_size(const unsigned char *cmnd) { - return (cmnd[0] == VARIABLE_LENGTH_CMD) ? - scsi_varlen_cdb_length(cmnd) : COMMAND_SIZE(cmnd[0]); + if (cmnd[0] == VARIABLE_LENGTH_CMD) + return cmnd[7] + 8; + return COMMAND_SIZE(cmnd[0]); } /* Returns a human-readable name for the device */ diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h index c3686011193224..c57f9cd8185526 100644 --- a/include/scsi/scsi_proto.h +++ b/include/scsi/scsi_proto.h @@ -176,16 +176,6 @@ #define SCSI_MAX_VARLEN_CDB_SIZE 260 -/* defined in T10 SCSI Primary Commands-2 (SPC2) */ -struct scsi_varlen_cdb_hdr { - __u8 opcode; /* opcode always == VARIABLE_LENGTH_CMD */ - __u8 control; - __u8 misc[5]; - __u8 additional_cdb_length; /* total cdb length - 8 */ - __be16 service_action; - /* service specific data follows */ -}; - /* * SCSI Architecture Model (SAM) Status codes. Taken from SAM-3 draft * T10/1561-D Revision 4 Draft dated 7th November 2002.
Directly access the cdb array like we do everywhere else insted of overlaying a structure on top of it. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/scsi/scsi_logging.c | 2 +- include/scsi/scsi_common.h | 11 +++-------- include/scsi/scsi_proto.h | 10 ---------- 3 files changed, 4 insertions(+), 19 deletions(-)