Re: [RFC PATCH 3/3] block: add queue idle timer

[Jeff Moyer]

Lin Ming <ming.m.lin@xxxxxxxxx> writes:

> On Tue, 2012-05-15 at 15:19 -0400, Jeff Moyer wrote:
>> Lin Ming <ming.m.lin@xxxxxxxxx> writes:
>> 
>> > Add an idle timer that is set to some suitable timeout and would be
>> > added when the queue first goes empty. If nothing has happened during
>> > the timeout interval, then the queue is suspended.
>> >
>> > Queueing a new request could check the state and resume queue if it is
>> > supended.
>> >
>> 
>> [snip]
>> 
>> > @@ -1129,6 +1141,13 @@ 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)) {
>> > +		if (!(--q->nr_pending))
>> > +			/* Hard code to 20secs, will move to sysfs */
>> > +			mod_timer(&q->idle, jiffies + 20*HZ);
>> > +	}
>> > +
>> 
>> I'm pretty sure Jens wanted to avoid doing a mod_timer, here, given that
>> the queue can transition between empty and non-empty fairly rapidly for
>> dependent I/O.
>
> I'll remove this idle timer and use runtime pm core's timer.

This issues isn't which timer to use, it's when to modify it.  Since the
queue can cycle between empty and non-empty very quickly, you should try
to avoid calling mod_timer for every non-empty to empty transition.
Jens had described one way to do this in the thread you referenced in
your 0/3 email.

Cheers,
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/