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