[SPAM]Re: [PATCH v7] scsi: ufs: Quiesce all scsi devices before shutdown

[Chaotian Jing]

On Mon, 2020-08-03 at 09:04 -0700, Bart Van Assche wrote:
> On 2020-08-03 03:04, Stanley Chu wrote:
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 307622284239..7cb220b3fde0 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -8640,6 +8640,7 @@ EXPORT_SYMBOL(ufshcd_runtime_idle);
> >  int ufshcd_shutdown(struct ufs_hba *hba)
> >  {
> >  	int ret = 0;
> > +	struct scsi_target *starget;
> >  
> >  	if (!hba->is_powered)
> >  		goto out;
> > @@ -8647,11 +8648,27 @@ int ufshcd_shutdown(struct ufs_hba *hba)
> >  	if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba))
> >  		goto out;
> >  
> > -	if (pm_runtime_suspended(hba->dev)) {
> > -		ret = ufshcd_runtime_resume(hba);
> > -		if (ret)
> > -			goto out;
> > -	}
> > +	/*
> > +	 * Let runtime PM framework manage and prevent concurrent runtime
> > +	 * operations with shutdown flow.
> > +	 */
> > +	pm_runtime_get_sync(hba->dev);
> > +
> > +	/*
> > +	 * Quiesce all SCSI devices to prevent any non-PM requests sending
> > +	 * from block layer during and after shutdown.
> > +	 *
> > +	 * Here we can not use blk_cleanup_queue() since PM requests
> > +	 * (with BLK_MQ_REQ_PREEMPT flag) are still required to be sent
> > +	 * through block layer. Therefore SCSI command queued after the
> > +	 * scsi_target_quiesce() call returned will block until
> > +	 * blk_cleanup_queue() is called.
> > +	 *
> > +	 * Besides, scsi_target_"un"quiesce (e.g., scsi_target_resume) can
> > +	 * be ignored since shutdown is one-way flow.
> > +	 */
> > +	list_for_each_entry(starget, &hba->host->__targets, siblings)
> > +		scsi_target_quiesce(starget);
> >  
> >  	ret = ufshcd_suspend(hba, UFS_SHUTDOWN_PM);
> >  out:
> 
> This seems wrong to me. Since ufshcd_shutdown() shuts down the link I think
> it should call scsi_remove_device() instead of scsi_target_quiesce().
> 
> Thanks,
> 
> Bart.
> 
> Hi Bart & Can & Stanley,

I have a question about this:
thread A is running the shutdown flow, but thread B is still access
UFS(sda/sdb/sdc..), is it expected ? after the sd_shutdown() completed,
if thread B still has access to UFS(sda/sdb/sdc...), it will make the
sd_shutdown() make no sense because the sd_resume() will send ssu to
start sda/sdb/sdc.

so can we avoid this and ensure that there is no request to sda after
sda's shutdown() is completed ?

so that is it possible to modify the sd_shutdown() ? take "sda" for
example: after sync cache && ssu to stop sda, do blk_cleanup_queue()
then it will ensure no runtime resume of sda and no more new requests to
sda.

then, for UFSHCI host driver, its shutdown() no need and should not
handle the sda/sdb/sdc's queue and device status, because these
devices(sda/sdb/sdc) has already complete its shutdown.
just like part of Can's comment, UFSHCI's shutdown() should only handle
hba->sdev_ufs_device.
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek