Messages in this thread | | | Date | Wed, 17 Aug 2022 06:52:14 -0700 | Subject | Re: [PATCH] scsi: core: Fix block I/O error of USB card reader during resume | From | Bart Van Assche <> |
| |
On 8/17/22 01:34, Michael Wu wrote: > When accessing storage device via an USB card reader, a block I/O error > occurs during resume: > > PM: suspend exit > sd 0: 0:0:0: [sda] tag#0 UNKNOWN(0x2003) Result: hostbyte=0x00 driverbyte > =0x08 > sd 0: 0:0:0: [sda] tag#0 Sense Key : 0x6 [current] > sd 0: 0:0:0: [sda] tag#0 ASC=0x28 ASCQ=0x0 > sd 0: 0:0:0: [sda] tag#0 CDB: opcode=0x28 28 00 00 17 ce e1 00 00 f0 00 > blk_update_request: I/O error, dev sda, sector 1560289 op 0x0:(READ) flags > 0x84700 phys_seg 19 prio class 0 > sd 0: 0:0:0: [sda] tag#0 device offline or changed > > Fix it by changing the action in scsi_io_completion_action() from > ACTION_FAIL to ACTION_RETRY by adding the condition `cmd->device-> > lockable`. > > Signed-off-by: Michael Wu <michael@allwinnertech.com> > --- > drivers/scsi/scsi_lib.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index 4dbd29ab1dcc..4bc480721947 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -704,7 +704,8 @@ static void scsi_io_completion_action(struct scsi_cmnd *cmd, int result) > } else if (sense_valid && sense_current) { > switch (sshdr.sense_key) { > case UNIT_ATTENTION: > - if (cmd->device->removable) { > + if (cmd->device->removable && > + cmd->device->lockable) { > /* Detected disc change. Set a bit > * and quietly refuse further access. > */
To me the above doesn't look like a good way to address this. I don't see why a device being lockable should control whether or not a unit attention results in a retry? Shouldn't the decision taken by scsi_io_completion_action() depend on the ASC and ASCQ codes rather than on whether a device is removable and/or lockable?
BTW, the code modified by the above patch is old. This is what I found in the 2002 version of scsi_lib.c:
if ((SCpnt->sense_buffer[0] & 0x7f) == 0x70 && (SCpnt->sense_buffer[2] & 0xf) == UNIT_ATTENTION) { if (SCpnt->device->removable) { /* detected disc change. set a bit and quietly refuse * further access. */ SCpnt->device->changed = 1; SCpnt = scsi_end_request(SCpnt, 0, this_count); return; } else { /* * Must have been a power glitch, or a * bus reset. Could not have been a * media change, so we just retry the * request and see what happens. */ scsi_queue_next_request(q, SCpnt); return; }
Bart.
Thanks,
Bart.
| |