Skip to content

Commit 785538b

Browse files
bvanasschemartinkpetersen
authored andcommitted
scsi: sd: Revert "Rework asynchronous resume support"
Although commit 88f1669 ("scsi: sd: Rework asynchronous resume support") eliminates a delay for some ATA disks after resume, it causes resume of ATA disks to fail on other setups. See also: * "Resume process hangs for 5-6 seconds starting sometime in 5.16" (https://bugzilla.kernel.org/show_bug.cgi?id=215880). * Geert's regression report (https://lore.kernel.org/linux-scsi/[email protected]/). This is what I understand about this issue: * During resume, ata_port_pm_resume() starts the SCSI error handler. This changes the SCSI host state into SHOST_RECOVERY and causes scsi_queue_rq() to return BLK_STS_RESOURCE. * sd_resume() calls sd_start_stop_device() for ATA devices. That function in turn calls sd_submit_start() which tries to submit a START STOP UNIT command. That command can only be submitted after the SCSI error handler has changed the SCSI host state back to SHOST_RUNNING. * The SCSI error handler runs on its own thread and calls schedule_work(&(ap->scsi_rescan_task)). That causes ata_scsi_dev_rescan() to be called from the context of a kernel workqueue. That call hangs in blk_mq_get_tag(). I'm not sure why - maybe because all available tags have been allocated by sd_submit_start() calls (this is a guess). Link: https://lore.kernel.org/r/[email protected] Fixes: 88f1669 ("scsi: sd: Rework asynchronous resume support") Cc: Damien Le Moal <[email protected]> Cc: Hannes Reinecke <[email protected]> Cc: Geert Uytterhoeven <[email protected]> Cc: [email protected] Reported-by: Geert Uytterhoeven <[email protected]> Reported-by: [email protected] Reported-and-tested-by: Vlastimil Babka <[email protected]> Tested-by: John Garry <[email protected]> Tested-by: Hans de Goede <[email protected]> Signed-off-by: Bart Van Assche <[email protected]> Signed-off-by: Martin K. Petersen <[email protected]>
1 parent fac8e55 commit 785538b

File tree

2 files changed

+18
-71
lines changed

2 files changed

+18
-71
lines changed

drivers/scsi/sd.c

Lines changed: 18 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ static void sd_config_discard(struct scsi_disk *, unsigned int);
103103
static void sd_config_write_same(struct scsi_disk *);
104104
static int sd_revalidate_disk(struct gendisk *);
105105
static void sd_unlock_native_capacity(struct gendisk *disk);
106-
static void sd_start_done_work(struct work_struct *work);
107106
static int sd_probe(struct device *);
108107
static int sd_remove(struct device *);
109108
static void sd_shutdown(struct device *);
@@ -3471,7 +3470,6 @@ static int sd_probe(struct device *dev)
34713470
sdkp->max_retries = SD_MAX_RETRIES;
34723471
atomic_set(&sdkp->openers, 0);
34733472
atomic_set(&sdkp->device->ioerr_cnt, 0);
3474-
INIT_WORK(&sdkp->start_done_work, sd_start_done_work);
34753473

34763474
if (!sdp->request_queue->rq_timeout) {
34773475
if (sdp->type != TYPE_MOD)
@@ -3594,69 +3592,12 @@ static void scsi_disk_release(struct device *dev)
35943592
kfree(sdkp);
35953593
}
35963594

3597-
/* Process sense data after a START command finished. */
3598-
static void sd_start_done_work(struct work_struct *work)
3599-
{
3600-
struct scsi_disk *sdkp = container_of(work, typeof(*sdkp),
3601-
start_done_work);
3602-
struct scsi_sense_hdr sshdr;
3603-
int res = sdkp->start_result;
3604-
3605-
if (res == 0)
3606-
return;
3607-
3608-
sd_print_result(sdkp, "Start/Stop Unit failed", res);
3609-
3610-
if (res < 0)
3611-
return;
3612-
3613-
if (scsi_normalize_sense(sdkp->start_sense_buffer,
3614-
sdkp->start_sense_len, &sshdr))
3615-
sd_print_sense_hdr(sdkp, &sshdr);
3616-
}
3617-
3618-
/* A START command finished. May be called from interrupt context. */
3619-
static void sd_start_done(struct request *req, blk_status_t status)
3620-
{
3621-
const struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
3622-
struct scsi_disk *sdkp = scsi_disk(req->q->disk);
3623-
3624-
sdkp->start_result = scmd->result;
3625-
WARN_ON_ONCE(scmd->sense_len > SCSI_SENSE_BUFFERSIZE);
3626-
sdkp->start_sense_len = scmd->sense_len;
3627-
memcpy(sdkp->start_sense_buffer, scmd->sense_buffer,
3628-
ARRAY_SIZE(sdkp->start_sense_buffer));
3629-
WARN_ON_ONCE(!schedule_work(&sdkp->start_done_work));
3630-
}
3631-
3632-
/* Submit a START command asynchronously. */
3633-
static int sd_submit_start(struct scsi_disk *sdkp, u8 cmd[], u8 cmd_len)
3634-
{
3635-
struct scsi_device *sdev = sdkp->device;
3636-
struct request_queue *q = sdev->request_queue;
3637-
struct request *req;
3638-
struct scsi_cmnd *scmd;
3639-
3640-
req = scsi_alloc_request(q, REQ_OP_DRV_IN, BLK_MQ_REQ_PM);
3641-
if (IS_ERR(req))
3642-
return PTR_ERR(req);
3643-
3644-
scmd = blk_mq_rq_to_pdu(req);
3645-
scmd->cmd_len = cmd_len;
3646-
memcpy(scmd->cmnd, cmd, cmd_len);
3647-
scmd->allowed = sdkp->max_retries;
3648-
req->timeout = SD_TIMEOUT;
3649-
req->rq_flags |= RQF_PM | RQF_QUIET;
3650-
req->end_io = sd_start_done;
3651-
blk_execute_rq_nowait(req, /*at_head=*/true);
3652-
3653-
return 0;
3654-
}
3655-
36563595
static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
36573596
{
36583597
unsigned char cmd[6] = { START_STOP }; /* START_VALID */
3598+
struct scsi_sense_hdr sshdr;
36593599
struct scsi_device *sdp = sdkp->device;
3600+
int res;
36603601

36613602
if (start)
36623603
cmd[4] |= 1; /* START */
@@ -3667,10 +3608,23 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
36673608
if (!scsi_device_online(sdp))
36683609
return -ENODEV;
36693610

3670-
/* Wait until processing of sense data has finished. */
3671-
flush_work(&sdkp->start_done_work);
3611+
res = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
3612+
SD_TIMEOUT, sdkp->max_retries, 0, RQF_PM, NULL);
3613+
if (res) {
3614+
sd_print_result(sdkp, "Start/Stop Unit failed", res);
3615+
if (res > 0 && scsi_sense_valid(&sshdr)) {
3616+
sd_print_sense_hdr(sdkp, &sshdr);
3617+
/* 0x3a is medium not present */
3618+
if (sshdr.asc == 0x3a)
3619+
res = 0;
3620+
}
3621+
}
36723622

3673-
return sd_submit_start(sdkp, cmd, sizeof(cmd));
3623+
/* SCSI error codes must not go to the generic layer */
3624+
if (res)
3625+
return -EIO;
3626+
3627+
return 0;
36743628
}
36753629

36763630
/*
@@ -3697,8 +3651,6 @@ static void sd_shutdown(struct device *dev)
36973651
sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
36983652
sd_start_stop_device(sdkp, 0);
36993653
}
3700-
3701-
flush_work(&sdkp->start_done_work);
37023654
}
37033655

37043656
static int sd_suspend_common(struct device *dev, bool ignore_stop_errors)

drivers/scsi/sd.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,11 +150,6 @@ struct scsi_disk {
150150
unsigned urswrz : 1;
151151
unsigned security : 1;
152152
unsigned ignore_medium_access_errors : 1;
153-
154-
int start_result;
155-
u32 start_sense_len;
156-
u8 start_sense_buffer[SCSI_SENSE_BUFFERSIZE];
157-
struct work_struct start_done_work;
158153
};
159154
#define to_scsi_disk(obj) container_of(obj, struct scsi_disk, disk_dev)
160155

0 commit comments

Comments
 (0)