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

[胡连勤]

Hi Bart,

> > Subject: [PATCH] scsi: sd: Mark the disk dead if runtime resume fails in
> > sd_remove()
> >
> > If a SCSI disk is unplugged while runtime suspended, its request queue
> > is still in the pm-only state when sd_remove() is called. The runtime
> > resume attempted by scsi_autopm_get_device() fails because the parent
> > device, e.g. a USB device in the USB_STATE_NOTATTACHED state, cannot be
> > resumed. Hence, sdev_runtime_resume() is not called and the pm-only
> > state is not cleared. Next, del_gendisk() calls
> > blk_report_disk_dead(disk, false), which calls sync_filesystem(). Any
> > bio submitted by the filesystem waits forever in __bio_queue_enter()
> > because the queue will never leave the pm-only state and because
> > GD_DEAD has not yet been set:
> >
> >   __bio_queue_enter+0xb8/0x174
> >   blk_mq_submit_bio+0x640/0x7e4
> >   [ ... ]
> >   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
> >   [ ... ]
> >   scsi_remove_host+0x88/0x18c
> >   usb_stor_disconnect+0x68/0xf4
> >
> > Fix this hang by calling blk_mark_disk_dead() if runtime resume fails.
> > This sets GD_DEAD, tells the filesystem that the disk has been removed
> > without attempting to sync it, and wakes up any bios that are waiting
> > in __bio_queue_enter(). Syncing the filesystem is not possible anyway
> > if runtime resume failed, so no data is lost that could otherwise have
> > been written. If runtime resume succeeds, behavior is not changed. Make
> > this change in sd_remove() instead of in a SCSI LLD such that it works
> > for all SCSI transports.
> >
> > Reported-by: Lianqin Hu <hulianqin@xxxxxxxx>
> > Closes:
> > https://lore.kernel.org/all/TYUPR06MB6217DD31B1B3CD09E431CC12D2922@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> > Fixes: 6f8191fdf41d ("block: simplify disk shutdown")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> > ---
> >   drivers/scsi/sd.c | 10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > index a1b21ea14e54..208b915f25a2 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -4270,7 +4270,15 @@ static void sd_remove(struct scsi_device *sdp)
> >   	struct device *dev = &sdp->sdev_gendev;
> >   	struct scsi_disk *sdkp = dev_get_drvdata(dev);
> >
> > -	scsi_autopm_get_device(sdkp->device);
> > +	/*
> > +	 * If runtime resume fails, e.g. because the device was unplugged
> > +	 * while runtime suspended, the request queue stays in the pm-only
> > +	 * state and any I/O submitted by del_gendisk(), e.g. from
> > +	 * sync_filesystem(), would wait forever in __bio_queue_enter().
> > +	 * Prevent this by marking the disk dead.
> > +	 */
> > +	if (scsi_autopm_get_device(sdkp->device) < 0)
> > +		blk_mark_disk_dead(sdkp->disk);
> >
> >   	device_del(&sdkp->disk_dev);
> >   	del_gendisk(sdkp->disk);
> 

I tested your patch on our kernel (v6.18) with the following scenario:
USB mass storage device with exFAT filesystem, autosuspend enabled,
physical removal during system suspend.

Test result: 1500 iterations, no hung_task observed. The previous
147+ second hang is completely resolved.

The patch works as expected. Thank you for this clean fix.

Tested-by: Lianqin Hu <hulianqin@xxxxxxxx>

Thanks,
Lianqin