Re: [PATCH 011/117] Introduce the scsi_status union

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-04-20 08:06, Bart Van Assche wrote:
Introduce the scsi_status union, a data structure that will be used in the next patches to replace SCSI status codes represented as an integer. Define
that data structure as follows:

	union scsi_status {
		int32_t combined;
		struct {
	#if defined(__BIG_ENDIAN)
			enum driver_status driver;
			enum host_status host;
			enum msg_byte msg;
			enum sam_status status;
	#elif defined(__LITTLE_ENDIAN)
			enum sam_status status;
			enum msg_byte msg;
			enum host_status host;
			enum driver_status driver;
	#else
	#error Endianness?
	#endif
		} b;
	};

The 'combined' member makes it easy to convert existing SCSI code. The
'status', 'msg', 'host' and 'driver' enable access of individual SCSI
status fields in a type-safe fashion.

Change 'int result;' into the following to enable converting one driver at
a time:

	union {
		int result;
		union scsi_status status;
	};

A later patch will remove the outer union and 'int result;'.

Also to enable converting one driver at a time, make scsi_status_is_good(), status_byte(), msg_byte(), host_byte() and driver_byte() accept an int or union scsi_status as argument. A later patch will make this function and
these macros accept the scsi_status union only.

Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Ming Lei <ming.lei@xxxxxxxxxx>
Cc: Hannes Reinecke <hare@xxxxxxxx>
Cc: John Garry <john.garry@xxxxxxxxxx>
Cc: Can Guo <cang@xxxxxxxxxxxxxx>
Cc: James Smart <james.smart@xxxxxxxxxxxx>
Cc: Lee Duncan <lduncan@xxxxxxxx>
Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
---
 drivers/scsi/scsi.c              |  9 +++++++++
 include/linux/bsg-lib.h          |  6 +++++-
 include/scsi/scsi.h              | 24 +++++++++++++++++++-----
 include/scsi/scsi_bsg_iscsi.h    |  6 +++++-
 include/scsi/scsi_cmnd.h         | 14 +++++++++-----
 include/scsi/scsi_eh.h           |  7 ++++++-
 include/scsi/scsi_request.h      |  6 +++++-
 include/scsi/scsi_status.h       | 29 +++++++++++++++++++++++++++++
 include/uapi/scsi/scsi_bsg_fc.h  | 10 ++++++++++
 include/uapi/scsi/scsi_bsg_ufs.h | 11 +++++++++++
 10 files changed, 108 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index e9e2f0e15ac8..4f71f2005be4 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -763,10 +763,19 @@ MODULE_LICENSE("GPL");
 module_param(scsi_logging_level, int, S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(scsi_logging_level, "a bit mask of logging levels");

+#define TEST_STATUS ((union scsi_status){.combined = 0x01020308})
+
 static int __init init_scsi(void)
 {
 	int error;

+	BUILD_BUG_ON(sizeof(union scsi_status) != 4);
+	BUILD_BUG_ON(TEST_STATUS.combined != 0x01020308);
+	BUILD_BUG_ON(driver_byte(TEST_STATUS) != 1);
+	BUILD_BUG_ON(host_byte(TEST_STATUS) != 2);
+	BUILD_BUG_ON(msg_byte(TEST_STATUS) != 3);
+	BUILD_BUG_ON(status_byte(TEST_STATUS) != 4);
+
 	error = scsi_init_procfs();
 	if (error)
 		goto cleanup_queue;
diff --git a/include/linux/bsg-lib.h b/include/linux/bsg-lib.h
index 960988d42f77..f934afc45760 100644
--- a/include/linux/bsg-lib.h
+++ b/include/linux/bsg-lib.h
@@ -11,6 +11,7 @@

 #include <linux/blkdev.h>
 #include <scsi/scsi_request.h>
+#include <scsi/scsi_status.h>

 struct request;
 struct device;
@@ -52,7 +53,10 @@ struct bsg_job {
 	struct bsg_buffer request_payload;
 	struct bsg_buffer reply_payload;

-	int result;
+	union {
+		int		  result; /* do not use in new code */
+		union scsi_status status;
+	};
 	unsigned int reply_payload_rcv_len;

 	/* BIDI support */
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index c9ccb6b45b76..18bb1fb2458f 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -39,7 +39,7 @@ enum scsi_timeouts {
  * This returns true for known good conditions that may be treated as
  * command completed normally
  */
-static inline int scsi_status_is_good(int status)
+static inline bool __scsi_status_is_good(int status)
 {
 	/*
 	 * FIXME: bit0 is listed as reserved in SCSI-2, but is
@@ -56,6 +56,20 @@ static inline int scsi_status_is_good(int status)
 		(status == SAM_STAT_COMMAND_TERMINATED));
 }

+/*
+ * If the 'status' argument has type int, unsigned int or union scsi_status, + * return the combined SCSI status. If the 'status' argument has another type, + * trigger a compiler error by passing a struct to a context where an integer
+ * is expected.
+ */
+#define scsi_status_to_int(status)			\
+	__builtin_choose_expr(sizeof(status) == 4,	\
+			      *(int32_t *)&(status),	\
+			      (union scsi_status){})
+
+#define scsi_status_is_good(status)				\
+	__scsi_status_is_good(scsi_status_to_int(status))
+

 /*
  * standard mode-select header prepended to all mode-select commands
@@ -134,10 +148,10 @@ enum scsi_disposition {
  *      driver_byte = set by mid-level.
  */
 #define status_byte(result) ((enum sam_status_divided_by_two)	\
-			     (((result) >> 1) & 0x7f))
-#define msg_byte(result)    (((result) >> 8) & 0xff)
-#define host_byte(result)   (((result) >> 16) & 0xff)
-#define driver_byte(result) (((result) >> 24) & 0xff)
+			     ((scsi_status_to_int((result)) >> 1) & 0x7f))
+#define msg_byte(result) ((scsi_status_to_int((result)) >> 8) & 0xff) +#define host_byte(result) ((scsi_status_to_int((result)) >> 16) & 0xff) +#define driver_byte(result) ((scsi_status_to_int((result)) >> 24) & 0xff)

 #define sense_class(sense)  (((sense) >> 4) & 0x7)
 #define sense_error(sense)  ((sense) & 0xf)
diff --git a/include/scsi/scsi_bsg_iscsi.h b/include/scsi/scsi_bsg_iscsi.h
index 6b8128005af8..d18e7e061927 100644
--- a/include/scsi/scsi_bsg_iscsi.h
+++ b/include/scsi/scsi_bsg_iscsi.h
@@ -13,6 +13,7 @@
  */

 #include <scsi/scsi.h>
+#include <scsi/scsi_status.h>

 /*
  * iSCSI Transport SGIO v4 BSG Message Support
@@ -82,7 +83,10 @@ struct iscsi_bsg_reply {
 	 * msg and status fields. The per-msgcode reply structure
 	 * will contain valid data.
 	 */
-	uint32_t result;
+	union {
+		uint32_t	  result; /* do not use in new code */
+		union scsi_status status;
+	};

 	/* If there was reply_payload, how much was recevied ? */
 	uint32_t reply_payload_rcv_len;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 202106e7c814..539be97b0a7d 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -140,7 +140,11 @@ struct scsi_cmnd {
 					 * obtained by scsi_malloc is guaranteed
 					 * to be at an address < 16Mb). */

-	int result;		/* Status code from lower level driver */
+	/* Status code from lower level driver */
+	union {
+		int		  result; /* do not use in new code. */
+		union scsi_status status;
+	};
 	int flags;		/* Command flags */
 	unsigned long state;	/* Command completion state */

@@ -317,23 +321,23 @@ static inline struct scsi_data_buffer
*scsi_prot(struct scsi_cmnd *cmd)
 static inline void set_status_byte(struct scsi_cmnd *cmd,
 				   enum sam_status status)
 {
-	cmd->result = (cmd->result & 0xffffff00) | status;
+	cmd->status.b.status = status;
 }

static inline void set_msg_byte(struct scsi_cmnd *cmd, enum msg_byte status)
 {
-	cmd->result = (cmd->result & 0xffff00ff) | (status << 8);
+	cmd->status.b.msg = status;
 }

 static inline void set_host_byte(struct scsi_cmnd *cmd, enum
host_status status)
 {
-	cmd->result = (cmd->result & 0xff00ffff) | (status << 16);
+	cmd->status.b.host = status;
 }

 static inline void set_driver_byte(struct scsi_cmnd *cmd,
 				   enum driver_status status)
 {
-	cmd->result = (cmd->result & 0x00ffffff) | (status << 24);
+	cmd->status.b.driver = status;
 }

 static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 468094254b3c..dcd66bb9a1ba 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -6,6 +6,8 @@

 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_common.h>
+#include <scsi/scsi_status.h>
+
 struct scsi_device;
 struct Scsi_Host;

@@ -31,7 +33,10 @@ extern int scsi_ioctl_reset(struct scsi_device *,
int __user *);

 struct scsi_eh_save {
 	/* saved state */
-	int result;
+	union {
+		int		  result; /* do not use in new code */
+		union scsi_status status;
+	};
 	unsigned int resid_len;
 	int eh_eflags;
 	enum dma_data_direction data_direction;
diff --git a/include/scsi/scsi_request.h b/include/scsi/scsi_request.h
index b06f28c74908..83f5549cc74c 100644
--- a/include/scsi/scsi_request.h
+++ b/include/scsi/scsi_request.h
@@ -3,6 +3,7 @@
 #define _SCSI_SCSI_REQUEST_H

 #include <linux/blk-mq.h>
+#include <scsi/scsi_status.h>

 #define BLK_MAX_CDB	16

@@ -10,7 +11,10 @@ struct scsi_request {
 	unsigned char	__cmd[BLK_MAX_CDB];
 	unsigned char	*cmd;
 	unsigned short	cmd_len;
-	int		result;
+	union {
+		int		  result; /* do not use in new code */
+		union scsi_status status;
+	};
 	unsigned int	sense_len;
 	unsigned int	resid_len;	/* residual count */
 	int		retries;
diff --git a/include/scsi/scsi_status.h b/include/scsi/scsi_status.h
index da2ba825f981..120f5a43d2ed 100644
--- a/include/scsi/scsi_status.h
+++ b/include/scsi/scsi_status.h
@@ -3,6 +3,7 @@

 #include <linux/types.h>
 #include <linux/compiler_attributes.h>
+#include <asm/byteorder.h>
 #include <scsi/scsi_proto.h>

 /*
@@ -88,4 +89,32 @@ enum driver_status {
 	DRIVER_SENSE	= 0x08,
 } __packed;

+/**
+ * SCSI status passed by LLDs to the midlayer.
+ * @combined: One of the following:
+ * - A (driver, host, msg, status) quadruplet encoded as a 32-bit integer.
+ *	- A negative Unix error code.
+ * - In the IDE code, a positive value that represents an error code, an
+ *	  error counter or a bitfield.
+ * @b: SCSI status code.
+ */
+union scsi_status {
+	int32_t combined;
+	struct {
+#if defined(__BIG_ENDIAN)
+		enum driver_status driver;
+		enum host_status host;
+		enum msg_byte msg;
+		enum sam_status status;
+#elif defined(__LITTLE_ENDIAN)
+		enum sam_status status;
+		enum msg_byte msg;
+		enum host_status host;
+		enum driver_status driver;
+#else
+#error Endianness?
+#endif
+	} b;
+};
+
 #endif /* _SCSI_SCSI_STATUS_H */
diff --git a/include/uapi/scsi/scsi_bsg_fc.h b/include/uapi/scsi/scsi_bsg_fc.h
index 3ae65e93235c..419db719fe8e 100644
--- a/include/uapi/scsi/scsi_bsg_fc.h
+++ b/include/uapi/scsi/scsi_bsg_fc.h
@@ -9,6 +9,9 @@
 #define SCSI_BSG_FC_H

 #include <linux/types.h>
+#ifdef __KERNEL__
+#include <scsi/scsi_status.h>
+#endif

 /*
  * This file intended to be included by both kernel and user space
@@ -291,7 +294,14 @@ struct fc_bsg_reply {
 	 *    msg and status fields. The per-msgcode reply structure
 	 *    will contain valid data.
 	 */
+#ifdef __KERNEL__
+	union {
+		__u32		  result; /* do not use in new kernel code */
+		union scsi_status status;
+	};
+#else
 	__u32 result;
+#endif

 	/* If there was reply_payload, how much was recevied ? */
 	__u32 reply_payload_rcv_len;
diff --git a/include/uapi/scsi/scsi_bsg_ufs.h b/include/uapi/scsi/scsi_bsg_ufs.h
index d55f2176dfd4..3dfe5a5a0842 100644
--- a/include/uapi/scsi/scsi_bsg_ufs.h
+++ b/include/uapi/scsi/scsi_bsg_ufs.h
@@ -9,6 +9,10 @@
 #define SCSI_BSG_UFS_H

 #include <linux/types.h>
+#ifdef __KERNEL__
+#include <scsi/scsi_status.h>
+#endif
+
 /*
  * This file intended to be included by both kernel and user space
  */
@@ -95,7 +99,14 @@ struct ufs_bsg_reply {
 	 * msg and status fields. The per-msgcode reply structure
 	 * will contain valid data.
 	 */
+#ifdef __KERNEL__
+	union {
+		__u32		  result; /* do not use in new kernel code */
+		union scsi_status status;
+	};
+#else
 	__u32 result;
+#endif

 	/* If there was reply_payload, how much was received? */
 	__u32 reply_payload_rcv_len;

Reviewed-by: Can Guo <cang@xxxxxxxxxxxxxx>



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux