[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAF1ivSbukHBZJsguAcUVDbQotyvE5iAfccgfv+-bGw1v6Z3Dvw@mail.gmail.com>
Date: Wed, 23 May 2012 16:23:18 +0800
From: Lin Ming <ming.m.lin@...el.com>
To: Alan Stern <stern@...land.harvard.edu>
Cc: Jens Axboe <axboe@...nel.dk>, linux-kernel@...r.kernel.org,
linux-pm@...r.kernel.org, linux-scsi@...r.kernel.org
Subject: Re: [RFC PATCH v3 3/4] block: implement runtime pm strategy
On Wed, May 23, 2012 at 12:14 AM, Alan Stern <stern@...land.harvard.edu> wrote:
> On Tue, 22 May 2012, Lin Ming wrote:
>
>> When a request is added:
>> If device is suspended or is suspending and the request is not a
>> PM request, resume the device.
>>
>> When a request finishes:
>> Call pm_runtime_mark_last_busy().
>>
>> When pick a request:
>> If device is not active, then only PM request is allowed to go.
>> Return NULL for other request.
>
> You've got the right idea. There are a few things that need fixing, in
> this patch and in 4/4.
>
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -1130,6 +1130,10 @@ void __blk_put_request(struct request_queue *q, struct request *req)
>> if (unlikely(--req->ref_count))
>> return;
>>
>> + /* PM request is not accounted */
>> + if (!(req->cmd_flags & REQ_PM) && !(--q->nr_pending) && q->dev)
>
> I don't see that it makes any difference. You might as well count PM
> requests along with the others, here and elsewhere.
Need to think about this.
>
>> + pm_runtime_mark_last_busy(q->dev);
>> +
>> elv_completed_request(q, req);
>>
>> /* this is a bio leak */
>> @@ -1918,6 +1922,12 @@ struct request *blk_peek_request(struct request_queue *q)
>> int ret;
>>
>> while ((rq = __elv_next_request(q)) != NULL) {
>> + /* Only PM request is allowed to go if the queue is suspended */
>> + if (q->rpm_status != RPM_ACTIVE && !(rq->cmd_flags & REQ_PM)) {
>> + rq = NULL;
>> + break;
>> + }
>
> Not even PM requests should be allowed to go if the status is
> RPM_SUSPENDED.
PM requests are used to wake up the device.
If they are not allowed to go, then how to wake up the device?
>
> Is this the only interface by which a client driver can get a request
> from the queue?
I searched __elv_next_request and it's only called in blk_peek_request.
Jens,
Could you confirm this?
Thanks,
Lin Ming
>
> Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists