On Tue, 2017-09-12 at 10:29 +0800, Ming Lei wrote:
> On Fri, Sep 08, 2017 at 04:52:25PM -0700, Bart Van Assche wrote:
> > @@ -1350,6 +1368,16 @@ static struct request *get_request(struct request_queue *q, unsigned int op,
> > lockdep_assert_held(q->queue_lock);
> > WARN_ON_ONCE(q->mq_ops);
> >
> > + WARN_ON_ONCE((op & REQ_PM) && blk_pm_suspended(q));
> > +
> > + /*
> > + * Wait if the request queue is suspended or in the process of
> > + * suspending/resuming and the request being allocated will not be
> > + * used for power management purposes.
> > + */
> > + if (!(op & REQ_PM) && !blk_wait_until_active(q, !(op & REQ_NOWAIT)))
> > + return ERR_PTR(-EAGAIN);
> > +
>
> Firstly it is not enough to prevent new allocation only, because
> requests pool may have been used up and all the allocated requests
> just stay in block queue when SCSI device is put into quiesce.
> So you may cause new I/O hang and wait forever here. That is why
> I take freeze to do that because freezing queue can prevent new
> allocation and drain in-queue requests both.
Sorry but I disagree. If all tags are allocated and it is attempted to
suspend a queue then this patch not only will prevent allocation of new
non-PM requests but it will also let these previously submitted non-PM
requests finish. See also the blk_peek_request() changes in patch 4/5.
Once a previously submitted request finished allocation of the PM request
will succeed.
> Secondly, all RQF_PREEMPT(not PM) is blocked here too, that may
> cause regression since we usually need/allow RQF_PREEMPT to run
> during SCSI quiesce.
Sorry but I disagree with this comment too. Please keep in mind that my patch
only affects the SCSI quiesce status if that status has been requested by the
power management subsystem. After the power management subsystem has
transitioned a SCSI queue into the quiesce state that queue has reached the
RPM_SUSPENDED status. No new requests must be submitted against a suspended
queue. It is easy to see in the legacy block layer that only PM requests and
no other RQF_PREEMPT requests are processed once the queue has reached the
suspended status:
/*
* Don't process normal requests when queue is suspended
* or in the process of suspending/resuming
*/
static struct request *blk_pm_peek_request(struct request_queue *q,
struct request *rq)
{
if (q->dev && (q->rpm_status == RPM_SUSPENDED ||
(q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM))))
return NULL;
else
return rq;
}
Bart.
