On Wed, Sep 27, 2017 at 09:54:09AM +0000, Bart Van Assche wrote:
> On Wed, 2017-09-27 at 13:48 +0800, Ming Lei wrote:
> > @@ -2928,12 +2929,28 @@ scsi_device_quiesce(struct scsi_device *sdev)
> > {
> > int err;
> >
> > + /*
> > + * Simply quiesing SCSI device isn't safe, it is easy
> > + * to use up requests because all these allocated requests
> > + * can't be dispatched when device is put in QIUESCE.
> > + * Then no request can be allocated and we may hang
> > + * somewhere, such as system suspend/resume.
> > + *
> > + * So we set block queue in preempt only first, no new
> > + * normal request can enter queue any more, and all pending
> > + * requests are drained once blk_set_preempt_only()
> > + * returns. Only RQF_PREEMPT is allowed in preempt only mode.
> > + */
> > + blk_set_preempt_only(sdev->request_queue, true);
> > +
> > mutex_lock(&sdev->state_mutex);
> > err = scsi_device_set_state(sdev, SDEV_QUIESCE);
> > mutex_unlock(&sdev->state_mutex);
> >
> > - if (err)
> > + if (err) {
> > + blk_set_preempt_only(sdev->request_queue, false);
> > return err;
> > + }
> >
> > scsi_run_queue(sdev->request_queue);
> > while (atomic_read(&sdev->device_busy)) {
> > @@ -2964,6 +2981,8 @@ void scsi_device_resume(struct scsi_device *sdev)
> > scsi_device_set_state(sdev, SDEV_RUNNING) == 0)
> > scsi_run_queue(sdev->request_queue);
> > mutex_unlock(&sdev->state_mutex);
> > +
> > + blk_set_preempt_only(sdev->request_queue, false);
>
> You should have realized yourself that this code is racy. If a request is
> allocated just before scsi_device_quiesce() is called and dispatched just
> after the device state has been changed into SDEV_QUIESCE then the loop that
That won't happen, any requests allocated before blk_set_preempt_only(true)
will be drained. Any normal requests are prevented from being entering
queue after blk_set_preempt_only(true) returns.
Please look at blk_set_preempt_only():
+void blk_set_preempt_only(struct request_queue *q, bool preempt_only)
+{
+ blk_mq_freeze_queue(q);
+ if (preempt_only)
+ queue_flag_set_unlocked(QUEUE_FLAG_PREEMPT_ONLY, q);
+ else
+ queue_flag_clear_unlocked(QUEUE_FLAG_PREEMPT_ONLY, q);
+ blk_mq_unfreeze_queue(q);
+}
+EXPORT_SYMBOL(blk_set_preempt_only);
blk_set_preempt_only(true) is called before scsi_device_set_state(sdev, SDEV_QUIESCE),
then any requests will be drained by blk_mq_freeze_queue() inside
blk_set_preempt_only(), meantime new normal requests are prevented from
being entering queue.
Once blk_set_preempt_only() returns, only RQF_PREEMPT is allowed to
enter queue.
--
Ming
