| Message ID | 20241115162003.3908-2-Kai.Makisara@kolumbus.fi (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: st: More reset patches | expand |
> On 15. Nov 2024, at 18.20, Kai Mäkisara <Kai.Makisara@kolumbus.fi> wrote: > > Don't use the was_reset flag set by scsi error handling. It is enough to > recognize device resets based in the UNIT ATTENTION sense data. (The > retry counts are zero and either REQ_OP_DRV_OUT or REC_OP_DRV_IN are > used to prevent retries by midlevel.) > Please hold this patch until the problem below has been solved. The following came into my mind when I looked at John's debugging data he sent to linux-scsi today. I still think that UNIT ATTENTIONs (UAs) reach the high level device without problems. The problem is that the device attached to the target first issuing a SCSI command after reset gets the UA. As long as this is st device, there are no problems. But, if it is the sg device attached to the same target, the tape device misses it. The device->was_reset flag stays set in many (most?) cases forever, unless someone resets it. (Scsi_error resets it only if the associated kthread ends up locking the device door.) Because the flag stays set, the st device can notice it even if the sg device gets the UA. Using a flag set by an other layer is ugly (to put it mildly). Even uglier is that st clears the flag when it has noticed that it is set. And there are cases where the device reset does not originate from the same computer. Does anyone have any suggestions?
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c index e8ef27d7ef61..3acaa3561c81 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) @@ -413,10 +415,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 = 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 @@ -3631,9 +3634,7 @@ static long st_ioctl(struct file *file, unsigned int cmd_in, unsigned long arg) retval = (-EIO); goto out; } - reset_state(STp); - /* remove this when the midlevel properly clears was_reset */ - STp->device->was_reset = 0; + reset_state(STp); /* Clears pos_unknown */ } if (mtc.mt_op != MTNOP && mtc.mt_op != MTSETBLK &&
Don't use the was_reset flag set by scsi error handling. It is enough to recognize device resets based in the UNIT ATTENTION sense data. (The retry counts are zero and either REQ_OP_DRV_OUT or REC_OP_DRV_IN are used to prevent retries by midlevel.) Signed-off-by: Kai Mäkisara <Kai.Makisara@kolumbus.fi> --- drivers/scsi/st.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)