[PATCH v4] scsi: sd: mark disk dead on surprise host removal to avoid I/O hang

[PATCH v4] scsi: sd: mark disk dead on surprise host removal to avoid I/O hang

[胡连勤]

When a USB mass storage device with a mounted filesystem (e.g. exFAT)
enters runtime suspend (autosuspend), physical removal triggers
scsi_remove_host() which invokes sd_remove() -> del_gendisk() ->
blk_report_disk_dead().  This triggers sync_filesystem() to write
back dirty data, submits bio and blocks in __bio_queue_enter()
waiting for I/O completion that can never happen since the device
is already gone, leading to a hung task.

del_gendisk() unconditionally calls blk_report_disk_dead(disk, false)
when GD_DEAD is not yet set, which always attempts to sync the
filesystem.  For surprise removal where the device cannot handle I/O,
this sync is futile and hangs forever.

Add scsi_host_in_cancel() helper and call blk_mark_disk_dead() in
sd_remove() when the SCSI host is in SHOST_CANCEL or
SHOST_CANCEL_RECOVERY state (set by scsi_remove_host() before
removing devices), so that del_gendisk() sees GD_DEAD already set
and skips the sync.  For orderly removal (e.g. echo 1 >
/sys/.../delete), the host remains SHOST_RUNNING and sync proceeds
normally.  This matches the NVMe behavior where
nvme_mark_namespaces_dead() is called only on surprise removal
(NVME_CTRL_DEAD).

Call trace on USB storage surprise removal:
INFO: task "kworker/1:0":31 blocked for more than 147 seconds.
Call trace:
 __switch_to+0x198/0x380
 __schedule+0x548/0xfbc
 schedule+0x4c/0x118
 __bio_queue_enter+0xb8/0x174
 blk_mq_submit_bio+0x640/0x7e4
 __submit_bio+0x1a4/0x314
 __submit_bio_noacct_mq+0x38/0x8c
 submit_bio_noacct+0x678/0x750
 submit_bio+0xa8/0x1c8
 submit_bh_wbc+0x148/0x1b4
 __sync_dirty_buffer+0x120/0x1f8
 exfat_sync_fs+0xa4/0xe0
 sync_filesystem+0xa8/0xdc
 fs_bdev_mark_dead+0x30/0x88
 bdev_mark_dead+0x54/0xc4
 blk_report_disk_dead+0x8c/0xd4
 del_gendisk+0xa0/0x2f8
 sd_remove+0x30/0x60
 device_release_driver_internal+0x1c4/0x2bc
 device_release_driver+0x18/0x28
 bus_remove_device+0x158/0x170
 device_del+0x1c8/0x320
 __scsi_remove_device+0x9c/0x184
 scsi_forget_host+0x50/0x70
 scsi_remove_host+0x88/0x18c
 usb_stor_disconnect+0x68/0xf4
 usb_unbind_interface+0x13c/0x340
 device_release_driver_internal+0x1c4/0x2bc
 device_release_driver+0x18/0x28
 bus_remove_device+0x158/0x170
 device_del+0x1c8/0x320
 usb_disable_device+0x84/0x190
 usb_disconnect+0xe8/0x338
 hub_event+0xbd8/0x19ac
 process_scheduled_works+0x200/0x9d8
 worker_thread+0x154/0x3b0

Fixes: 6f8191fdf41d ("block: simplify disk shutdown")
Cc: stable@vger.kernel.org
Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
---

Changes in v4:
  - Repost: v3 did not receive any review feedback.
  - The block layer counterpart (7e9a46004b47 'block: set QUEUE_FLAG_DYING
   unconditionally in blk_mark_disk_dead()') has been merged, but it alone
   is not sufficient as the queue is not frozen during autosuspend.  This
   SCSI patch is still needed to mark the disk dead before del_gendisk().
  - Link to v3: https://lore.kernel.org/all/TYUPR06MB62177178F9305DF5C1A7D046D2A92@TYUPR06MB6217.apcprd06.prod.outlook.com/

Changes in v3:
 - Repost after merge window closure as requested by maintainer.
 - Use scsi_get_host_state() to match scsi_host_in_recovery() pattern.
 - Fix Link to v1 URL.
 - Link to v2: https://lore.kernel.org/all/PUZPR06MB622453C37D2BABC5E71B195DD2DC2@PUZPR06MB6224.apcprd06.prod.outlook.com/

Changes in v2:
 - Split into a two-patch series for independent review by block and SCSI maintainers.
 - This is the SCSI part extracted from v1.
 - Link to v1: https://lore.kernel.org/all/TYUPR06MB62172E412054140753D5E782D2C82@TYUPR06MB6217.apcprd06.prod.outlook.com/

 drivers/scsi/sd.c        | 10 ++++++++++
 include/scsi/scsi_host.h |  8 ++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a1b21ea14e54..f68fd0eac3d6 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -4272,6 +4272,16 @@ static void sd_remove(struct scsi_device *sdp)
 
 	scsi_autopm_get_device(sdkp->device);
 
+	/*
+	 * If the host is being torn down (e.g. USB surprise removal),
+	 * the device cannot handle I/O anymore.
+	 * Mark the disk dead before del_gendisk() so that new I/O is
+	 * rejected immediately and sync_filesystem() in del_gendisk()
+	 * is skipped, avoiding a hang waiting for I/O that can never
+	 * complete.
+	 */
+	if (scsi_host_in_cancel(sdkp->device->host))
+		blk_mark_disk_dead(sdkp->disk);
 	device_del(&sdkp->disk_dev);
 	del_gendisk(sdkp->disk);
 	if (!sdkp->suspended)
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 16243ec376cd..d35c3ccb24b0 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -804,6 +804,14 @@ static inline int scsi_host_in_recovery(struct Scsi_Host *shost)
 		shost->tmf_in_progress;
 }
 
+static inline bool scsi_host_in_cancel(struct Scsi_Host *shost)
+{
+	enum scsi_host_state state = scsi_get_host_state(shost);
+
+	return state == SHOST_CANCEL ||
+		state == SHOST_CANCEL_RECOVERY;
+}
+
 extern int scsi_queue_work(struct Scsi_Host *, struct work_struct *);
 extern void scsi_flush_work(struct Scsi_Host *);
 
-- 
2.48.1

Re: [PATCH v4] scsi: sd: mark disk dead on surprise host removal to avoid I/O hang

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: Data loss on orderly host teardown due to misidentified surprise removal
--

commit 9a3b48997b6a95bc55f9d45d1fe71786ff63438c
Author:  <hulianqin@vivo.com>

scsi: sd: mark disk dead on surprise host removal to avoid I/O hang

This commit adds a scsi_host_in_cancel() check in sd_remove() to mark the
disk dead before del_gendisk(). This is intended to skip sync_filesystem()
during surprise removals where the device can no longer handle I/O.

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b0d2542fd3172..00cf408513ce4 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -4270,6 +4270,16 @@ static void sd_remove(struct scsi_device *sdp)
>  
>  	scsi_autopm_get_device(sdkp->device);
>  
> +	/*
> +	 * If the host is being torn down (e.g. USB surprise removal),
> +	 * the device cannot handle I/O anymore.
> +	 * Mark the disk dead before del_gendisk() so that new I/O is
> +	 * rejected immediately and sync_filesystem() in del_gendisk()
> +	 * is skipped, avoiding a hang waiting for I/O that can never
> +	 * complete.
> +	 */
> +	if (scsi_host_in_cancel(sdkp->device->host))
> +		blk_mark_disk_dead(sdkp->disk);

[Severity: High]
Will this logic conflate all host removals with surprise removals?

The patch aims to skip sync_filesystem() only during surprise removals by
checking if the host is in SHOST_CANCEL. However, scsi_remove_host() in
drivers/scsi/hosts.c unconditionally transitions the host state to
SHOST_CANCEL for all host teardowns:

drivers/scsi/hosts.c:scsi_remove_host() {
    ...
    if (scsi_host_set_state(shost, SHOST_CANCEL))
    ...
}

Because SHOST_CANCEL is the standard teardown state for all removals,
including orderly ones like rmmod usb-storage or sysfs driver unbinding,
blk_mark_disk_dead() will set GD_DEAD for every host removal.

When del_gendisk() executes, it will bypass its own
blk_report_disk_dead(disk, false) because GD_DEAD is already set.

Could this skip sync_filesystem() entirely, causing all pending dirty page
cache data to be discarded instead of written back during an orderly unbind
or unload, resulting in data loss?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/TYUPR06MB6217570B9AFA4B5980FCCED0D2932@TYUPR06MB6217.apcprd06.prod.outlook.com?part=1