[13/29] scsi: ufs: Remove the LUN quiescing code from ufshcd_wl_shutdown()

Quiescing LUNs falls outside the scope of a shutdown callback. The shutdown
callback is called from inside the reboot() system call and the reboot()
system call is called after user space has stopped accessing block devices.
Hence this patch that removes the quiescing calls from
ufshcd_wl_shutdown(). This patch makes shutdown faster since multiple
synchronize_rcu() calls are removed.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/ufs/ufshcd.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

On 01/04/2022 1.34, Bart Van Assche wrote:
> Quiescing LUNs falls outside the scope of a shutdown callback. The shutdown
> callback is called from inside the reboot() system call and the reboot()
> system call is called after user space has stopped accessing block devices.
> Hence this patch that removes the quiescing calls from
> ufshcd_wl_shutdown(). This patch makes shutdown faster since multiple
> synchronize_rcu() calls are removed.

AFAIK there is nothing stopping shutdown being called during intense UFS I/O.
What happens then?

> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/ufs/ufshcd.c | 10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index a48362165672..ae08c7964f2d 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -9212,9 +9212,7 @@ static int ufshcd_wl_resume(struct device *dev)
>  static void ufshcd_wl_shutdown(struct device *dev)
>  {
>  	struct scsi_device *sdev = to_scsi_device(dev);
> -	struct ufs_hba *hba;
> -
> -	hba = shost_priv(sdev->host);
> +	struct ufs_hba *hba = shost_priv(sdev->host);
>  
>  	down(&hba->host_sem);
>  	hba->shutting_down = true;
> @@ -9222,12 +9220,6 @@ static void ufshcd_wl_shutdown(struct device *dev)
>  
>  	/* Turn on everything while shutting down */
>  	ufshcd_rpm_get_sync(hba);
> -	scsi_device_quiesce(sdev);
> -	shost_for_each_device(sdev, hba->host) {
> -		if (sdev == hba->sdev_ufs_device)
> -			continue;
> -		scsi_device_quiesce(sdev);
> -	}
>  	__ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
>  }
>

On 3/31/22 23:20, Adrian Hunter wrote:
> On 01/04/2022 1.34, Bart Van Assche wrote:
>> Quiescing LUNs falls outside the scope of a shutdown callback. The shutdown
>> callback is called from inside the reboot() system call and the reboot()
>> system call is called after user space has stopped accessing block devices.
>> Hence this patch that removes the quiescing calls from
>> ufshcd_wl_shutdown(). This patch makes shutdown faster since multiple
>> synchronize_rcu() calls are removed.
> 
> AFAIK there is nothing stopping shutdown being called during intense UFS I/O.
> What happens then?

Hmm ... how could this happen? Am I perhaps misunderstanding something 
about the Linux shutdown sequence?

The UFS driver is the only driver I know that tries to stop I/O from 
inside its shutdown callback. I'm not aware of any other Linux kernel 
driver that tries to pause I/O from inside its shutdown callback.

Thanks,

Bart.

On 01/04/2022 16.52, Bart Van Assche wrote:
> On 3/31/22 23:20, Adrian Hunter wrote:
>> On 01/04/2022 1.34, Bart Van Assche wrote:
>>> Quiescing LUNs falls outside the scope of a shutdown callback. The shutdown
>>> callback is called from inside the reboot() system call and the reboot()
>>> system call is called after user space has stopped accessing block devices.
>>> Hence this patch that removes the quiescing calls from
>>> ufshcd_wl_shutdown(). This patch makes shutdown faster since multiple
>>> synchronize_rcu() calls are removed.
>>
>> AFAIK there is nothing stopping shutdown being called during intense UFS I/O.
>> What happens then?
> 
> Hmm ... how could this happen? Am I perhaps misunderstanding something about the Linux shutdown sequence?
> 
> The UFS driver is the only driver I know that tries to stop I/O from inside its shutdown callback. I'm not aware of any other Linux kernel driver that tries to pause I/O from inside its shutdown callback.

We are putting the UFS device into powerdown mode.  I am not sure what will happen if the device is processing requests at the same time.

On 4/1/22 07:35, Adrian Hunter wrote:
> On 01/04/2022 16.52, Bart Van Assche wrote:
>> On 3/31/22 23:20, Adrian Hunter wrote:
>>> On 01/04/2022 1.34, Bart Van Assche wrote:
>>>> Quiescing LUNs falls outside the scope of a shutdown callback.
>>>> The shutdown callback is called from inside the reboot() system
>>>> call and the reboot() system call is called after user space
>>>> has stopped accessing block devices. Hence this patch that
>>>> removes the quiescing calls from ufshcd_wl_shutdown(). This
>>>> patch makes shutdown faster since multiple synchronize_rcu()
>>>> calls are removed.
>>> 
>>> AFAIK there is nothing stopping shutdown being called during
>>> intense UFS I/O. What happens then?
>> 
>> Hmm ... how could this happen? Am I perhaps misunderstanding
>> something about the Linux shutdown sequence?
>> 
>> The UFS driver is the only driver I know that tries to stop I/O
>> from inside its shutdown callback. I'm not aware of any other Linux
>> kernel driver that tries to pause I/O from inside its shutdown
>> callback.
> 
> We are putting the UFS device into powerdown mode.  I am not sure
> what will happen if the device is processing requests at the same
> time.

I will remove this patch from this patch series such that it does not
block adoption of the other patches in this patch series.

Thanks,

Bart.