Re: [PATCH v5 5/9] block: Change the runtime power management approach (1/2)

[Bart Van Assche <Bart.VanAssche@wdc.com>]

On Wed, 2018-08-08 at 14:43 +0800, jianchao.wang wrote:
>=20
> On 08/08/2018 02:11 PM, jianchao.wang wrote:
> > Hi Bart
> >=20
> > On 08/08/2018 06:51 AM, Bart Van Assche wrote:
> > > @@ -391,6 +393,9 @@ static struct request =
Co-blk_mq_get_request(struct request_queue *q,
> > >  		}
> > >  	}
> > >  	data->hctx->queued++;
> > > +
> > > +	blk_pm_add_request(q, rq);
> > > +
> > >  	return rq;
> > >  }
> >=20
> > The request_queue is in pm_only mode when suspended, wh=
o can reach here to do the resume ?
>=20
> 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_queu=
e_enter while the queue is suspended
> and in pm-only mode. But if no resume, where does the pm request come=
 from ?
>=20
> The blk_pm_add_request should be added to blk_queue=
F8-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_o=
nly more finally
>    3. the task blocked in blk_queue_enter is waked up and pro=
ceed.

Hello Jianchao,

Some but not all blk_queue_enter() calls are related to request all=
ocation 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_devi=
ce() calls in
the sd and sr drivers. However, not all request submission code is protecte=
d 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_=
AKg-() restores
full power mode.

Bart.