答复: [PATCH v5 1/2] scsi: sd: mark disk dead on surprise host removal to avoid I/O hang

[胡连勤]

Hi Christoph,

> >  	unsigned short_inquiry:1;
> >
> > +	/* Device is physically gone; skip sync in sd_remove() */
> > +	unsigned surprise_removal:1;
> 
> Placing this right in the middle of static fields is a bit weird.
> 
> Maybe we should get started with an unsigned long and atomic bitops
> for something like that actually is known to set it at any point in
> runtime and thus most suspect to corruption non-atomic bitfields?

Thanks for the review. You're right on both points.

I plan to change it as follows:

 drivers/scsi/sd.c        | 4 ++++
 include/scsi/scsi_host.h | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a1b21ea14e54..075a50c08404 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -4272,6 +4272,10 @@ static void sd_remove(struct scsi_device *sdp)
 
 	scsi_autopm_get_device(sdkp->device);
 
+	/* Device is gone; avoid sync_filesystem() hang in del_gendisk(). */
+	if (test_bit(SHOST_SURPRISE_REMOVAL, &sdkp->device->host->flags))
+		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..66aa9ae607de 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -757,6 +757,13 @@ struct Scsi_Host {
 	/* Delay for runtime autosuspend */
 	int rpm_autosuspend_delay;
 
+	/*
+	 * Host flags for runtime state that may be set concurrently.
+	 * Use atomic bitops (set_bit/test_bit) to access.
+	 */
+	unsigned long flags;
+#define SHOST_SURPRISE_REMOVAL	0
+
 	/*
 	 * We should ensure that this is aligned, both for better performance
 	 * and also because some compilers (m68k) don't automatically force

 drivers/usb/storage/usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 064c7fc8e368..3a5a0cac2b78 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -930,6 +930,9 @@ static void quiesce_and_remove_host(struct us_data *us)
 	if (us->pusb_dev->state == USB_STATE_NOTATTACHED) {
 		set_bit(US_FLIDX_DISCONNECTING, &us->dflags);
 		wake_up(&us->delay_wait);
+
+		/* Device is physically gone, cannot handle I/O */
+		set_bit(SHOST_SURPRISE_REMOVAL, &host->flags);
 	}


The flags field is placed after rpm_autosuspend_delay, away from the
static bitfields. set_bit()/test_bit() ensure atomic access when the
flag is set at disconnect time and read in sd_remove().

Regards,
Lianqin