On Wed, 2018-08-08 at 14:43 +0800, jianchao.wang wrote:
> 
> On 08/08/2018 02:11 PM, jianchao.wang wrote:
> > Hi Bart
> > 
> > On 08/08/2018 06:51 AM, Bart Van Assche wrote:
> > > @@ -391,6 +393,9 @@ static struct request *blk_mq_get_request(struct 
> > > request_queue *q,
> > >           }
> > >   }
> > >   data->hctx->queued++;
> > > +
> > > + blk_pm_add_request(q, rq);
> > > +
> > >   return rq;
> > >  }
> > 
> > The request_queue is in pm_only mode when suspended, who can reach here to 
> > do the resume ?
> 
> I mean, in the original blk-legacy runtime pm implementation, any new IO 
> could trigger the resume,
> after your patch set, only the pm request could pass the blk_queue_enter 
> while the queue is suspended
> and in pm-only mode. But if no resume, where does the pm request come from ?
> 
> The blk_pm_add_request should be added to blk_queue_enter.
> It looks like as following:
>    1. when an normal io reaches blk_queue_enter, if queue is in suspended 
> mode, it invoke blk_pm_add_request
>       to trigger the resume, then wait here for the pm_only mode to be 
> cleared.
>    2. the runtime pm core does the resume work and clear the pm_only more 
> finally
>    3. the task blocked in blk_queue_enter is waked up and proceed.

Hello Jianchao,

Some but not all blk_queue_enter() calls are related to request allocation so
I'm not sure that that call should be added into blk_queue_enter(). The reason
my tests passed is probably because of the scsi_autopm_get_device() calls in
the sd and sr drivers. However, not all request submission code is protected by
these calls. I will have a closer look at how to preserve the behavior that
queueing a new request that is not protected by scsi_autopm_get_*() restores
full power mode.

Bart.

Reply via email to