Re: [PATCH v6 08/12] block, scsi: Introduce blk_pm_runtime_exit()

["jianchao.wang" <jianchao.w.wang@oracle.com>]

Hi Bart

On 08/10/2018 03:41 AM, Bart Van Assche wrote:
> +void blk_pm_runtime_exit(struct request_queue *q)
> +{
> +	if (!q->dev)
> +		return;
> +
> +	pm_runtime_get_sync(q->dev);
> +	q->dev = NULL;
> +}
> +EXPORT_SYMBOL(blk_pm_runtime_exit);
> +
>  /**
>   * blk_pre_runtime_suspend - Pre runtime suspend check
>   * @q: the queue of the device
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 69ab459abb98..5537762dfcfd 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3420,12 +3420,11 @@ static int sd_probe(struct device *dev)
>   **/
>  static int sd_remove(struct device *dev)
>  {
> -	struct scsi_disk *sdkp;
> -	dev_t devt;
> +	struct scsi_disk *sdkp = dev_get_drvdata(dev);
> +	struct scsi_device *sdp = sdkp->device;
> +	dev_t devt = disk_devt(sdkp->disk);
>  
> -	sdkp = dev_get_drvdata(dev);
> -	devt = disk_devt(sdkp->disk);
> -	scsi_autopm_get_device(sdkp->device);
> +	blk_pm_runtime_exit(sdp->request_queue)


Based on __scsi_device_remove, sd_remove will be invoked before blk_cleanup_queue.
So it should be not safe that we set the q->dev to NULL here.

We have following operations in followed patch:
+static inline void blk_pm_mark_last_busy(struct request *rq)
+{
+	if (rq->q->dev && !(rq->rq_flags & RQF_PM))
+		pm_runtime_mark_last_busy(rq->q->dev);
+}

Thanks
Jianchao