On Wed, 2018-08-08 at 16:50 +0800, Ming Lei wrote:
> On Tue, Aug 07, 2018 at 03:51:30PM -0700, Bart Van Assche wrote:
> > Instead of allowing requests that are not power management requests
> > to enter the queue in runtime suspended status (RPM_SUSPENDED), make
> > the blk_get_request() caller block. This change fixes a starvation
>
> Looks not see the related change which blocks blk_get_request() in
> this patchset.
I was referring to setting pm-only mode for suspended devices. Since
blk_queue_enter() is called before a request is allocated that is sufficient
to make request allocation block.
> > diff --git a/block/blk-pm.c b/block/blk-pm.c
> > index bf8532da952d..d6b65cef9764 100644
> > --- a/block/blk-pm.c
> > +++ b/block/blk-pm.c
> > @@ -86,14 +86,39 @@ int blk_pre_runtime_suspend(struct request_queue *q)
> > if (!q->dev)
> > return ret;
> >
> > + WARN_ON_ONCE(q->rpm_status != RPM_ACTIVE);
> > +
> > + blk_set_pm_only(q);
> > + /*
> > + * This function only gets called if the most recent
> > + * pm_request_resume() call occurred at least autosuspend_delay_ms
> > + * ago. Since blk_queue_enter() is called by the request allocation
> > + * code before pm_request_resume(), if q_usage_counter indicates that
> > + * no requests are in flight it is safe to suspend the device.
> > + */
> > + ret = -EBUSY;
> > + if (!percpu_ref_is_in_use(&q->q_usage_counter)) {
> > + /*
> > + * Switch to preempt-only mode before calling
> > + * synchronize_rcu() such that later blk_queue_enter() calls
> > + * see the preempt-only state. See also
> > + * http://lwn.net/Articles/573497/.
> > + */
> > + synchronize_rcu();
> > + if (!percpu_ref_is_in_use(&q->q_usage_counter))
> > + ret = 0;
> > + }
> > +
>
> In blk_queue_enter(), V5 starts to allow all RQF_PREEMPT requests
> to enter queue even though pm_only is set. That means any scsi_execute()
> may issue a new command to host just after the above percpu_ref_is_in_use()
> returns 0, meantime the suspend is in-progress.
>
> This case is illegal given RQF_PM is the only kind of request which can be
> issued to hardware during suspend.
Right, that's something that needs to be addressed.
Bart.
