RE: [PATCH v1 1/1] ufs: core: set SDEV_OFFLINE when ufs shutdown.

Seunghwan Baek
2024-09-24T02:17:19Z

> On 8/29/24 2:39 AM, Seunghwan Baek wrote:
> > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > index a6f818cdef0e..4ac1492787c2 100644
> > --- a/drivers/ufs/core/ufshcd.c
> > +++ b/drivers/ufs/core/ufshcd.c
> > @@ -10215,7 +10215,9 @@ static void ufshcd_wl_shutdown(struct device
> *dev)
> > shost_for_each_device(sdev, hba->host) {
> > if (sdev == hba->ufs_device_wlun)
> > continue;
> > - scsi_device_quiesce(sdev);
> > + mutex_lock(&sdev->state_mutex);
> > + scsi_device_set_state(sdev, SDEV_OFFLINE);
> > + mutex_unlock(&sdev->state_mutex);
> > }
> > __ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
>
> Why to keep one scsi_device_quiesce() call and convert the other call?
> Please consider something like this change:
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index
> e808350c6774..914770dff18f 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -10134,11 +10134,10 @@ 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->ufs_device_wlun)
> - continue;
> - scsi_device_quiesce(sdev);
> + mutex_lock(&sdev->state_mutex);
> + scsi_device_set_state(sdev, SDEV_OFFLINE);
> + mutex_unlock(&sdev->state_mutex);
> }
> __ufshcd_wl_suspend(hba, UFS_SHUTDOWN_PM);
>
> Thanks,
>
> Bart.

That's because SSU (Start Stop Unit) command must be sent during shutdown process.
If SDEV_OFFLINE is set for wlun, SSU command cannot be sent because it is rejected
by the scsi layer. Therefore, we consider to set SDEV_QUIESCE for wlun, and set
SDEV_OFFLINE for other lus.

static int ufshcd_execute_start_stop(struct scsi_device *sdev,
enum ufs_dev_pwr_mode pwr_mode,
struct scsi_sense_hdr *sshdr)
{
const unsigned char cdb[6] = { START_STOP, 0, 0, 0, pwr_mode << 4, 0 };
const struct scsi_exec_args args = {
.sshdr = sshdr,
.req_flags = BLK_MQ_REQ_PM, <<< set REQ_PM flag
.scmd_flags = SCMD_FAIL_IF_RECOVERING,
};

return scsi_execute_cmd(sdev, cdb, REQ_OP_DRV_IN, /*buffer=*/NULL,
/*bufflen=*/0, /*timeout=*/10 * HZ, /*retries=*/0,
&args);
}

static blk_status_t
scsi_device_state_check(struct scsi_device *sdev, struct request *req)
{
case SDEV_OFFLINE:
case SDEV_TRANSPORT_OFFLINE: <<< Refuse all commands
/*
* If the device is offline we refuse to process any
* commands. The device must be brought online
* before trying any recovery commands.
*/
if (!sdev->offline_already) {
sdev->offline_already = true;
sdev_printk(KERN_ERR, sdev,
"rejecting I/O to offline device\n");
}
return BLK_STS_IOERR;
case SDEV_QUIESCE: <<< Refuse all commands except REQ_PM flag
/*
* If the device is blocked we only accept power management
* commands.
*/
if (req && WARN_ON_ONCE(!(req->rq_flags & RQF_PM)))
return BLK_STS_RESOURCE;
return BLK_STS_OK;