diff mbox series

[v3,3/4] scsi: st: scsi_device: Modify st.c to use the new scsi_error counters

Message ID 20250120194925.44432-4-Kai.Makisara@kolumbus.fi (mailing list archive)
State New
Headers show
Series scsi: st: scsi_error: More reset patches | expand

Commit Message

Kai Mäkisara Jan. 20, 2025, 7:49 p.m. UTC
Compare the stored values of por_ctr and new_media_ctr against
the values in the device struct. In case of mismatch, the
Unit Attention corresponding to the counter has happened.
This is a safeguard against another ULD catching the
Unit Attention sense data.

Macros scsi_get_ua_new_media_ctr and scsi_get_ua_por_ctr are added to
read the current values of the counters.

Signed-off-by: Kai Mäkisara <Kai.Makisara@kolumbus.fi>
---
 drivers/scsi/st.c          | 30 +++++++++++++++++++++++++-----
 drivers/scsi/st.h          |  4 ++++
 include/scsi/scsi_device.h |  6 ++++++
 3 files changed, 35 insertions(+), 5 deletions(-)

Comments

John Meneghini Jan. 30, 2025, 6:18 p.m. UTC | #1
Reviewed-by: John Meneghini <jmeneghi@redhat.com>
Tested-by: John Meneghini <jmeneghi@redhat.com>

Testing of this patch with my current tests show no regressions.

I'll work on a multi-ULD test to improve those tests.

/John

On 1/20/25 2:49 PM, Kai Mäkisara wrote:
> Compare the stored values of por_ctr and new_media_ctr against
> the values in the device struct. In case of mismatch, the
> Unit Attention corresponding to the counter has happened.
> This is a safeguard against another ULD catching the
> Unit Attention sense data.
> 
> Macros scsi_get_ua_new_media_ctr and scsi_get_ua_por_ctr are added to
> read the current values of the counters.
> 
> Signed-off-by: Kai Mäkisara <Kai.Makisara@kolumbus.fi>
> ---
>   drivers/scsi/st.c          | 30 +++++++++++++++++++++++++-----
>   drivers/scsi/st.h          |  4 ++++
>   include/scsi/scsi_device.h |  6 ++++++
>   3 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> index 0fc9afe60862..6ec1cfeb92ff 100644
> --- a/drivers/scsi/st.c
> +++ b/drivers/scsi/st.c
> @@ -163,9 +163,11 @@ static const char *st_formats[] = {
>   
>   static int debugging = DEBUG;
>   
> +/* Setting these non-zero may risk recognizing resets */
>   #define MAX_RETRIES 0
>   #define MAX_WRITE_RETRIES 0
>   #define MAX_READY_RETRIES 0
> +
>   #define NO_TAPE  NOT_READY
>   
>   #define ST_TIMEOUT (900 * HZ)
> @@ -357,10 +359,18 @@ static int st_chk_result(struct scsi_tape *STp, struct st_request * SRpnt)
>   {
>   	int result = SRpnt->result;
>   	u8 scode;
> +	unsigned int ctr;
>   	DEB(const char *stp;)
>   	char *name = STp->name;
>   	struct st_cmdstatus *cmdstatp;
>   
> +	ctr = scsi_get_ua_por_ctr(STp->device);
> +	if (ctr != STp->por_ctr) {
> +		STp->por_ctr = ctr;
> +		STp->pos_unknown = 1; /* ASC => power on / reset */
> +		st_printk(KERN_WARNING, STp, "Power on/reset recognized.");
> +	}
> +
>   	if (!result)
>   		return 0;
>   
> @@ -413,10 +423,11 @@ static int st_chk_result(struct scsi_tape *STp, struct st_request * SRpnt)
>   	if (cmdstatp->have_sense &&
>   	    cmdstatp->sense_hdr.asc == 0 && cmdstatp->sense_hdr.ascq == 0x17)
>   		STp->cleaning_req = 1; /* ASC and ASCQ => cleaning requested */
> -	if (cmdstatp->have_sense && scode == UNIT_ATTENTION && cmdstatp->sense_hdr.asc == 0x29)
> +	if (cmdstatp->have_sense && scode == UNIT_ATTENTION &&
> +		cmdstatp->sense_hdr.asc == 0x29 && !STp->pos_unknown) {
>   		STp->pos_unknown = 1; /* ASC => power on / reset */
> -
> -	STp->pos_unknown |= STp->device->was_reset;
> +		st_printk(KERN_WARNING, STp, "Power on/reset recognized.");
> +	}
>   
>   	if (cmdstatp->have_sense &&
>   	    scode == RECOVERED_ERROR
> @@ -968,6 +979,7 @@ static int test_ready(struct scsi_tape *STp, int do_wait)
>   {
>   	int attentions, waits, max_wait, scode;
>   	int retval = CHKRES_READY, new_session = 0;
> +	unsigned int ctr;
>   	unsigned char cmd[MAX_COMMAND_SIZE];
>   	struct st_request *SRpnt = NULL;
>   	struct st_cmdstatus *cmdstatp = &STp->buffer->cmdstat;
> @@ -1024,6 +1036,13 @@ static int test_ready(struct scsi_tape *STp, int do_wait)
>   			}
>   		}
>   
> +		ctr = scsi_get_ua_new_media_ctr(STp->device);
> +		if (ctr != STp->new_media_ctr) {
> +			STp->new_media_ctr = ctr;
> +			new_session = 1;
> +			DEBC_printk(STp, "New tape session.");
> +		}
> +
>   		retval = (STp->buffer)->syscall_result;
>   		if (!retval)
>   			retval = new_session ? CHKRES_NEW_SESSION : CHKRES_READY;
> @@ -3639,8 +3658,6 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
>   				goto out;
>   			}
>   			reset_state(STp); /* Clears pos_unknown */
> -			/* remove this when the midlevel properly clears was_reset */
> -			STp->device->was_reset = 0;
>   
>   			/* Fix the device settings after reset, ignore errors */
>   			if (mtc.mt_op == MTREW || mtc.mt_op == MTSEEK ||
> @@ -4402,6 +4419,9 @@ static int st_probe(struct device *dev)
>   		goto out_idr_remove;
>   	}
>   
> +	tpnt->new_media_ctr = scsi_get_ua_new_media_ctr(SDp);
> +	tpnt->por_ctr = scsi_get_ua_por_ctr(SDp);
> +
>   	dev_set_drvdata(dev, tpnt);
>   
>   
> diff --git a/drivers/scsi/st.h b/drivers/scsi/st.h
> index 6d31b894ee84..0d7c4b8c2c8a 100644
> --- a/drivers/scsi/st.h
> +++ b/drivers/scsi/st.h
> @@ -179,6 +179,10 @@ struct scsi_tape {
>   	int recover_count;     /* From tape opening */
>   	int recover_reg;       /* From last status call */
>   
> +	/* The saved values of midlevel counters */
> +	unsigned int new_media_ctr;
> +	unsigned int por_ctr;
> +
>   #if DEBUG
>   	unsigned char write_pending;
>   	int nbr_finished;
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index f5c0f07a053a..013018608370 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -687,6 +687,12 @@ static inline int scsi_device_busy(struct scsi_device *sdev)
>   	return sbitmap_weight(&sdev->budget_map);
>   }
>   
> +/* Macros to access the UNIT ATTENTION counters */
> +#define scsi_get_ua_new_media_ctr(sdev) \
> +	((const unsigned int)(sdev->ua_new_media_ctr))
> +#define scsi_get_ua_por_ctr(sdev) \
> +	((const unsigned int)(sdev->ua_por_ctr))
> +
>   #define MODULE_ALIAS_SCSI_DEVICE(type) \
>   	MODULE_ALIAS("scsi:t-" __stringify(type) "*")
>   #define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"
diff mbox series

Patch

diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 0fc9afe60862..6ec1cfeb92ff 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -163,9 +163,11 @@  static const char *st_formats[] = {
 
 static int debugging = DEBUG;
 
+/* Setting these non-zero may risk recognizing resets */
 #define MAX_RETRIES 0
 #define MAX_WRITE_RETRIES 0
 #define MAX_READY_RETRIES 0
+
 #define NO_TAPE  NOT_READY
 
 #define ST_TIMEOUT (900 * HZ)
@@ -357,10 +359,18 @@  static int st_chk_result(struct scsi_tape *STp, struct st_request * SRpnt)
 {
 	int result = SRpnt->result;
 	u8 scode;
+	unsigned int ctr;
 	DEB(const char *stp;)
 	char *name = STp->name;
 	struct st_cmdstatus *cmdstatp;
 
+	ctr = scsi_get_ua_por_ctr(STp->device);
+	if (ctr != STp->por_ctr) {
+		STp->por_ctr = ctr;
+		STp->pos_unknown = 1; /* ASC => power on / reset */
+		st_printk(KERN_WARNING, STp, "Power on/reset recognized.");
+	}
+
 	if (!result)
 		return 0;
 
@@ -413,10 +423,11 @@  static int st_chk_result(struct scsi_tape *STp, struct st_request * SRpnt)
 	if (cmdstatp->have_sense &&
 	    cmdstatp->sense_hdr.asc == 0 && cmdstatp->sense_hdr.ascq == 0x17)
 		STp->cleaning_req = 1; /* ASC and ASCQ => cleaning requested */
-	if (cmdstatp->have_sense && scode == UNIT_ATTENTION && cmdstatp->sense_hdr.asc == 0x29)
+	if (cmdstatp->have_sense && scode == UNIT_ATTENTION &&
+		cmdstatp->sense_hdr.asc == 0x29 && !STp->pos_unknown) {
 		STp->pos_unknown = 1; /* ASC => power on / reset */
-
-	STp->pos_unknown |= STp->device->was_reset;
+		st_printk(KERN_WARNING, STp, "Power on/reset recognized.");
+	}
 
 	if (cmdstatp->have_sense &&
 	    scode == RECOVERED_ERROR
@@ -968,6 +979,7 @@  static int test_ready(struct scsi_tape *STp, int do_wait)
 {
 	int attentions, waits, max_wait, scode;
 	int retval = CHKRES_READY, new_session = 0;
+	unsigned int ctr;
 	unsigned char cmd[MAX_COMMAND_SIZE];
 	struct st_request *SRpnt = NULL;
 	struct st_cmdstatus *cmdstatp = &STp->buffer->cmdstat;
@@ -1024,6 +1036,13 @@  static int test_ready(struct scsi_tape *STp, int do_wait)
 			}
 		}
 
+		ctr = scsi_get_ua_new_media_ctr(STp->device);
+		if (ctr != STp->new_media_ctr) {
+			STp->new_media_ctr = ctr;
+			new_session = 1;
+			DEBC_printk(STp, "New tape session.");
+		}
+
 		retval = (STp->buffer)->syscall_result;
 		if (!retval)
 			retval = new_session ? CHKRES_NEW_SESSION : CHKRES_READY;
@@ -3639,8 +3658,6 @@  static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg)
 				goto out;
 			}
 			reset_state(STp); /* Clears pos_unknown */
-			/* remove this when the midlevel properly clears was_reset */
-			STp->device->was_reset = 0;
 
 			/* Fix the device settings after reset, ignore errors */
 			if (mtc.mt_op == MTREW || mtc.mt_op == MTSEEK ||
@@ -4402,6 +4419,9 @@  static int st_probe(struct device *dev)
 		goto out_idr_remove;
 	}
 
+	tpnt->new_media_ctr = scsi_get_ua_new_media_ctr(SDp);
+	tpnt->por_ctr = scsi_get_ua_por_ctr(SDp);
+
 	dev_set_drvdata(dev, tpnt);
 
 
diff --git a/drivers/scsi/st.h b/drivers/scsi/st.h
index 6d31b894ee84..0d7c4b8c2c8a 100644
--- a/drivers/scsi/st.h
+++ b/drivers/scsi/st.h
@@ -179,6 +179,10 @@  struct scsi_tape {
 	int recover_count;     /* From tape opening */
 	int recover_reg;       /* From last status call */
 
+	/* The saved values of midlevel counters */
+	unsigned int new_media_ctr;
+	unsigned int por_ctr;
+
 #if DEBUG
 	unsigned char write_pending;
 	int nbr_finished;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index f5c0f07a053a..013018608370 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -687,6 +687,12 @@  static inline int scsi_device_busy(struct scsi_device *sdev)
 	return sbitmap_weight(&sdev->budget_map);
 }
 
+/* Macros to access the UNIT ATTENTION counters */
+#define scsi_get_ua_new_media_ctr(sdev) \
+	((const unsigned int)(sdev->ua_new_media_ctr))
+#define scsi_get_ua_por_ctr(sdev) \
+	((const unsigned int)(sdev->ua_por_ctr))
+
 #define MODULE_ALIAS_SCSI_DEVICE(type) \
 	MODULE_ALIAS("scsi:t-" __stringify(type) "*")
 #define SCSI_DEVICE_MODALIAS_FMT "scsi:t-0x%02x"