[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2465978d-28d3-e30f-248e-87333c789743@acm.org>
Date: Sat, 11 Jul 2020 20:21:17 -0700
From: Bart Van Assche <bvanassche@....org>
To: Stanley Chu <stanley.chu@...iatek.com>, linux-scsi@...r.kernel.org,
martin.petersen@...cle.com, avri.altman@....com,
alim.akhtar@...sung.com, jejb@...ux.ibm.com
Cc: beanhuo@...ron.com, asutoshd@...eaurora.org, cang@...eaurora.org,
matthias.bgg@...il.com, linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
kuohong.wang@...iatek.com, peter.wang@...iatek.com,
chun-hung.wu@...iatek.com, andy.teng@...iatek.com,
chaotian.jing@...iatek.com, cc.chou@...iatek.com
Subject: Re: [RFC PATCH v3] scsi: ufs: Quiesce all scsi devices before
shutdown
On 2020-07-06 06:22, Stanley Chu wrote:
> +static void ufshcd_cleanup_queue(struct scsi_device *sdev, void *data)
> +{
> + if (sdev->request_queue)
> + blk_cleanup_queue(sdev->request_queue);
> +}
No SCSI LLD should ever call blk_cleanup_queue() directly for
sdev->request_queue. Only the SCSI core should call blk_cleanup_queue()
directly for that queue.
> int ufshcd_shutdown(struct ufs_hba *hba)
> {
> int ret = 0;
> + struct scsi_target *starget;
>
> if (!hba->is_powered)
> goto out;
> @@ -8612,7 +8632,25 @@ int ufshcd_shutdown(struct ufs_hba *hba)
> goto out;
> }
>
> + /*
> + * 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.
> + */
> + ufshcd_scsi_for_each_sdev(ufshcd_quiece_sdev);
> +
> ret = ufshcd_suspend(hba, UFS_SHUTDOWN_PM);
> +
> + /* Set queue as dying to not block queueing commands */
> + ufshcd_scsi_for_each_sdev(ufshcd_cleanup_queue);
> out:
> if (ret)
> dev_err(hba->dev, "%s failed, err %d\n", __func__, ret);
>
What is the purpose of ufshcd_shutdown()? Why does this function exist?
How about removing the calls to ufshcd_shutdown() and invoking power down
code from inside sd_suspend_common() instead?
Thanks,
Bart.
Powered by blists - more mailing lists