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 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); > +}
Hello Jianchao, How about moving the blk_pm_runtime_exit() call into blk_cleanup_queue() at a point where it is sure that there are no requests in progress? That should avoid that blk_pm_mark_last_busy() races against blk_pm_runtime_exit(). Thanks, Bart.