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

[Bart Van Assche <Bart.VanAssche@wdc.com>]

On Fri, 2018-08-10 at 10:39 +0800, jianchao.wang wrote:
> 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 suspe=
nd 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(st=
ruct 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);
> > =20
> > -	sdkp = dev_get_drvdata(dev);
> > -	devt = disk_devt(sdkp->disk);
> > -	scsi_autopm_get_device(sdkp->device);
> > +	blk_pm_runtime_exit(sdp->request_queue)
>=20
>=20
> Based on __scsi_device_remove, sd_remove will be in=
voked before blk_cleanup_queue.
> So it should be not safe that we set the q->dev to NULL here.
>=20
> We have following operations in followed patch:
> +static inline void blk_pm_mark_last_busy(struct req=
uest *rq)
> +{
> +	if (rq->q->dev && !(rq->rq_flags & R=
QF_PM))
> +		pm_runtime_mark_last_busy(rq->q->dev);=
-
> +}

Hello Jianchao,

How about moving the blk_pm_runtime_exit() call into blk_cl=
eanup_queue() at
a point where it is sure that there are no requests in progress? That shoul=
d
avoid that blk_pm_mark_last_busy() races against blk_pm=
_runtime_exit().

Thanks,

Bart.