On Thu, Jul 28, 2011 at 10:42 PM, Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote: > On Thu, Jul 28, 2011 at 12:24:48PM +0800, Zhi Yong Wu wrote: >> On Wed, Jul 27, 2011 at 11:49 PM, Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote: >> > On Wed, Jul 27, 2011 at 06:17:15PM +0800, Zhi Yong Wu wrote: >> >> >> + wait_time = 1; >> >> >> + } >> >> >> + >> >> >> + wait_time = wait_time + (slice_time - elapsed_time); >> >> >> + if (wait) { >> >> >> + *wait = wait_time * BLOCK_IO_SLICE_TIME * 10 + 1; >> >> >> + } >> >> > >> >> > The guest can keep submitting requests where "wait_time = 1" above, >> >> > and the timer will be rearmed continuously in the future. >> > >> > This is wrong. >> > >> >> Can't you >> >> > simply arm the timer to the next slice start? _Some_ data must be >> >> > transfered by then, anyway (and nothing can be transfered earlier than >> >> > that). >> > >> > This is valid. >> > >> >> Sorry, i have got what you mean. Can you elaborate in more detail? >> > >> > Sorry, the bug i mentioned about timer being rearmed does not exist. >> > >> > But arming the timer for the last request as its done is confusing/unnecessary. >> > >> > The timer processes queued requests, but the timer is armed accordingly >> > to the last queued request in the slice. For example, if a request is >> > submitted 1ms before the slice ends, the timer is armed 100ms + >> > (slice_time - elapsed_time) in the future. >> >> If the timer is simply amred to the next slice start, this timer will >> be a periodic timer, either the I/O rate can not be throttled under >> the limits, or the enqueued request can be delayed to handled, this >> will lower I/O rate seriously than the limits. > > Yes, periodic but disarmed when there is no queueing. I don't understand > your point about low I/O rate. We hope that at the same time the runtime I/O rate is throttled under this limits, it can be very close to the limits. If the timer is simpily armed to the next slice, the enqueued request could be delayed a bit long to be handled, this could make current I/O rate lowerer largely than the limits. So this could seriously hurt the I/O performance. > >> Maybe the slice time should be variable with current I/O rate. What do >> you think of it? > > Not sure if its necessary. The slice should be small to avoid excessive > work on timer context, but the advantage of increasing the slice is > very small if any. BTW, 10ms seems a better default than 100ms. Thanks for your comments, if you're interested, pls take a look at the code V3. It has added the codes for variable slice time. > > -- Regards, Zhi Yong Wu -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html