| Message ID | 5059738E.5070004@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 2012-09-19 09:26, Paolo Bonzini wrote: > Il 18/09/2012 22:37, Anthony Liguori ha scritto: >> Unfortunately, there's a lot of Windows code in qemu-timer.c and main-loop.c >> right now otherwise the refactoring would be trivial. I'll leave that for >> another day. >> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Jan Kiszka <jan.kiszka@siemens.com> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> --- >> Please note, this is lightly tested. Since this is such a fundamental change, >> I'd like to do some performance analysis before committing but wanted to share >> early. > > Looks good. I think Peter Portante tested something similar, and found no big > difference between the two. But it's a good thing and, in my opinion, for > non-timerfd OSes we should simply adjust the select() timeout and not bother > with signals. What would be the advantage of timerfd over select? On Linux, both use hrtimers (and low slack for RT processes). I'm starting to like the select/WaitForMultipleObjects pattern as it would allow to consolidate over basically two versions of timers and simplify the code. Jan > > I'm not sure if the same can be done for Windows, but I think it's possible as long > as you keep the timeBeginPeriod/timeEndPeriod calls. As a start, Stefan, can you > check if the win32 timer works for you with the calls added? Like this: > > diff --git a/qemu-timer.c b/qemu-timer.c > index c7a1551..721c769 100644 > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -673,6 +673,10 @@ static int win32_start_timer(struct qemu_alarm_timer *t) > HANDLE hTimer; > BOOLEAN success; > > + timeGetDevCaps(&mm_tc, sizeof(mm_tc)); > + > + timeBeginPeriod(mm_tc.wPeriodMin); > + > /* If you call ChangeTimerQueueTimer on a one-shot timer (its period > is zero) that has already expired, the timer is not updated. Since > creating a new timer is relatively expensive, set a bogus one-hour > @@ -688,6 +692,7 @@ static int win32_start_timer(struct qemu_alarm_timer *t) > if (!success) { > fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n", > GetLastError()); > + timeEndPeriod(mm_tc.wPeriodMin); > return -1; > } > > @@ -702,6 +707,7 @@ static void win32_stop_timer(struct qemu_alarm_timer *t) > if (hTimer) { > DeleteTimerQueueTimer(NULL, hTimer, NULL); > } > + timeEndPeriod(mm_tc.wPeriodMin); > } > > static void win32_rearm_timer(struct qemu_alarm_timer *t, > > Paolo >
Il 19/09/2012 09:44, Jan Kiszka ha scritto: >> > Looks good. I think Peter Portante tested something similar, and found no big >> > difference between the two. But it's a good thing and, in my opinion, for >> > non-timerfd OSes we should simply adjust the select() timeout and not bother >> > with signals. > What would be the advantage of timerfd over select? On Linux, both use > hrtimers (and low slack for RT processes). I'm starting to like the > select/WaitForMultipleObjects pattern as it would allow to consolidate > over basically two versions of timers and simplify the code. Oh, I didn't know this. Even better. Paolo
On Wed, Sep 19, 2012 at 3:44 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2012-09-19 09:26, Paolo Bonzini wrote: > > Il 18/09/2012 22:37, Anthony Liguori ha scritto: > >> Unfortunately, there's a lot of Windows code in qemu-timer.c and > main-loop.c > >> right now otherwise the refactoring would be trivial. I'll leave that > for > >> another day. > >> > >> Cc: Paolo Bonzini <pbonzini@redhat.com> > >> Cc: Jan Kiszka <jan.kiszka@siemens.com> > >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > >> --- > >> Please note, this is lightly tested. Since this is such a fundamental > change, > >> I'd like to do some performance analysis before committing but wanted > to share > >> early. > > > > Looks good. I think Peter Portante tested something similar, and found > no big > > difference between the two. But it's a good thing and, in my opinion, > for > > non-timerfd OSes we should simply adjust the select() timeout and not > bother > > with signals. > > What would be the advantage of timerfd over select? On Linux, both use > hrtimers (and low slack for RT processes). I am not sure the comparison is timerfd v. select, but timerfd v signal based timer (setitimer). The timerfd path allows you to integrate with select/poll/epoll loops, where as signal based timers make that more difficult. One can do the same thing with signalfd, but only for one signal, where as you can setup multiple timers at the expense of file descriptors. Additionally, FWIW, select() has a resolution capped by its use of struct timeval, which is microseconds, where timerfd_settime allows for nanosecond resolution. > I'm starting to like the > select/WaitForMultipleObjects pattern as it would allow to consolidate > over basically two versions of timers and simplify the code. > With timerfd, signalfd and eventfd, Linux seems to have provided all the coverage needed to make that happen. > > Jan > > > > > I'm not sure if the same can be done for Windows, but I think it's > possible as long > > as you keep the timeBeginPeriod/timeEndPeriod calls. As a start, > Stefan, can you > > check if the win32 timer works for you with the calls added? Like this: > > > > diff --git a/qemu-timer.c b/qemu-timer.c > > index c7a1551..721c769 100644 > > --- a/qemu-timer.c > > +++ b/qemu-timer.c > > @@ -673,6 +673,10 @@ static int win32_start_timer(struct > qemu_alarm_timer *t) > > HANDLE hTimer; > > BOOLEAN success; > > > > + timeGetDevCaps(&mm_tc, sizeof(mm_tc)); > > + > > + timeBeginPeriod(mm_tc.wPeriodMin); > > + > > /* If you call ChangeTimerQueueTimer on a one-shot timer (its period > > is zero) that has already expired, the timer is not updated. > Since > > creating a new timer is relatively expensive, set a bogus > one-hour > > @@ -688,6 +692,7 @@ static int win32_start_timer(struct qemu_alarm_timer > *t) > > if (!success) { > > fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n", > > GetLastError()); > > + timeEndPeriod(mm_tc.wPeriodMin); > > return -1; > > } > > > > @@ -702,6 +707,7 @@ static void win32_stop_timer(struct qemu_alarm_timer > *t) > > if (hTimer) { > > DeleteTimerQueueTimer(NULL, hTimer, NULL); > > } > > + timeEndPeriod(mm_tc.wPeriodMin); > > } > > > > static void win32_rearm_timer(struct qemu_alarm_timer *t, > > > > Paolo > > > > -- > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > Corporate Competence Center Embedded Linux > >
Please turn of HTML in you mailer. It's very hard to parse your reply. On 2012-09-19 16:15, Peter Portante wrote: > On Wed, Sep 19, 2012 at 3:44 AM, Jan Kiszka <jan.kiszka@siemens.com<mailto:jan.kiszka@siemens.com>> wrote: > On 2012-09-19 09:26, Paolo Bonzini wrote: >> Il 18/09/2012 22:37, Anthony Liguori ha scritto: >>> Unfortunately, there's a lot of Windows code in qemu-timer.c and main-loop.c >>> right now otherwise the refactoring would be trivial. I'll leave that for >>> another day. >>> >>> Cc: Paolo Bonzini <pbonzini@redhat.com<mailto:pbonzini@redhat.com>> >>> Cc: Jan Kiszka <jan.kiszka@siemens.com<mailto:jan.kiszka@siemens.com>> >>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com<mailto:aliguori@us.ibm.com>> >>> --- >>> Please note, this is lightly tested. Since this is such a fundamental change, >>> I'd like to do some performance analysis before committing but wanted to share >>> early. >> >> Looks good. I think Peter Portante tested something similar, and found no big >> difference between the two. But it's a good thing and, in my opinion, for >> non-timerfd OSes we should simply adjust the select() timeout and not bother >> with signals. > > What would be the advantage of timerfd over select? On Linux, both use > hrtimers (and low slack for RT processes). > > I am not sure the comparison is timerfd v. select, but timerfd v signal based timer (setitimer). The timerfd path allows you to integrate with select/poll/epoll loops, where as signal based timers make that more difficult. One can do the same thing with signalfd, but only for one signal, where as you can setup multiple timers at the expense of file descriptors. > > Additionally, FWIW, select() has a resolution capped by its use of struct timeval, which is microseconds, where timerfd_settime allows for nanosecond resolution. < 1µs resolution is pointless, even on RT-hardened kernels with fast hardware underneath and when running natively. > > I'm starting to like the > select/WaitForMultipleObjects pattern as it would allow to consolidate > over basically two versions of timers and simplify the code. > > With timerfd, signalfd and eventfd, Linux seems to have provided all the coverage needed to make that happen. The advantage is that timers based on select/poll timeouts will allow to unify a lot of code for _all_ host platforms, i.e. even Windows. We still need to evaluate the precise impact and look for potentially missed limitations (aka: someone has to write patches and test them). But if there are no relevant ones, it should be the better architecture. That said, a timerfd based solution for Linux may be an intermediate step of the select-based work takes longer. Jan
Am 19.09.2012 09:26, schrieb Paolo Bonzini: > Il 18/09/2012 22:37, Anthony Liguori ha scritto: >> Unfortunately, there's a lot of Windows code in qemu-timer.c and main-loop.c >> right now otherwise the refactoring would be trivial. I'll leave that for >> another day. >> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Jan Kiszka <jan.kiszka@siemens.com> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> --- >> Please note, this is lightly tested. Since this is such a fundamental change, >> I'd like to do some performance analysis before committing but wanted to share >> early. > Looks good. I think Peter Portante tested something similar, and found no big > difference between the two. But it's a good thing and, in my opinion, for > non-timerfd OSes we should simply adjust the select() timeout and not bother > with signals. > > I'm not sure if the same can be done for Windows, but I think it's possible as long > as you keep the timeBeginPeriod/timeEndPeriod calls. As a start, Stefan, can you > check if the win32 timer works for you with the calls added? Like this: > > diff --git a/qemu-timer.c b/qemu-timer.c > index c7a1551..721c769 100644 > --- a/qemu-timer.c > +++ b/qemu-timer.c > @@ -673,6 +673,10 @@ static int win32_start_timer(struct qemu_alarm_timer *t) > HANDLE hTimer; > BOOLEAN success; > > + timeGetDevCaps(&mm_tc, sizeof(mm_tc)); > + > + timeBeginPeriod(mm_tc.wPeriodMin); > + > /* If you call ChangeTimerQueueTimer on a one-shot timer (its period > is zero) that has already expired, the timer is not updated. Since > creating a new timer is relatively expensive, set a bogus one-hour > @@ -688,6 +692,7 @@ static int win32_start_timer(struct qemu_alarm_timer *t) > if (!success) { > fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n", > GetLastError()); > + timeEndPeriod(mm_tc.wPeriodMin); > return -1; > } > > @@ -702,6 +707,7 @@ static void win32_stop_timer(struct qemu_alarm_timer *t) > if (hTimer) { > DeleteTimerQueueTimer(NULL, hTimer, NULL); > } > + timeEndPeriod(mm_tc.wPeriodMin); > } > > static void win32_rearm_timer(struct qemu_alarm_timer *t, > > Paolo The win32 timer still works when these modifications were applied. What are they good for? Stefan
Il 19/09/2012 18:04, Stefan Weil ha scritto: > The win32 timer still works when these modifications were applied. > What are they good for? IIUC the Win32 did _not_ work (except on Wine) without these. Note I'm talking about "-clock win32". So these provide a hint that the problem with the Win32 timer is not in the bowels of Windows, but only that it does not adjust the Windows scheduler quantum. Wine does not need it because the Linux scheduler is not as lame. So if we just eliminated timers and used the g_poll timeout (what was proposed upthread), Windows would work provided timeBeginPeriod/timeEndPeriod are called correctly. Paolo
Am 19.09.2012 18:12, schrieb Paolo Bonzini: > Il 19/09/2012 18:04, Stefan Weil ha scritto: >> The win32 timer still works when these modifications were applied. >> What are they good for? > IIUC the Win32 did _not_ work (except on Wine) without these. Note I'm > talking about "-clock win32". > > So these provide a hint that the problem with the Win32 timer is not in > the bowels of Windows, but only that it does not adjust the Windows > scheduler quantum. Wine does not need it because the Linux scheduler is > not as lame. > > So if we just eliminated timers and used the g_poll timeout (what was > proposed upthread), Windows would work provided > timeBeginPeriod/timeEndPeriod are called correctly. > > Paolo I tested with -clock win32 (not the mm timer), and it worked on Win7 (64 bit QEMU) with and without the modification. Maybe I can run some more tests on 32 bit Windows (also with g_poll), but don't expect results soon. Stefan
On 09/19/2012 10:44 AM, Jan Kiszka wrote: > On 2012-09-19 09:26, Paolo Bonzini wrote: >> Il 18/09/2012 22:37, Anthony Liguori ha scritto: >>> Unfortunately, there's a lot of Windows code in qemu-timer.c and main-loop.c >>> right now otherwise the refactoring would be trivial. I'll leave that for >>> another day. >>> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: Jan Kiszka <jan.kiszka@siemens.com> >>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >>> --- >>> Please note, this is lightly tested. Since this is such a fundamental change, >>> I'd like to do some performance analysis before committing but wanted to share >>> early. >> >> Looks good. I think Peter Portante tested something similar, and found no big >> difference between the two. But it's a good thing and, in my opinion, for >> non-timerfd OSes we should simply adjust the select() timeout and not bother >> with signals. > > What would be the advantage of timerfd over select? On Linux, both use > hrtimers (and low slack for RT processes). I'm starting to like the > select/WaitForMultipleObjects pattern as it would allow to consolidate > over basically two versions of timers and simplify the code. An advantage is that if you have a lot of fd events but fewer timer events, then you do not need to rearm the timer needlessly. It just waits in the background. I doubt whether that advantage amounts to anything in practice.
Jan Kiszka <jan.kiszka <at> siemens.com> writes: > What would be the advantage of timerfd over select? On Linux, both use > hrtimers (and low slack for RT processes). I'm starting to like the > select/WaitForMultipleObjects pattern as it would allow to consolidate > over basically two versions of timers and simplify the code. Why not use CreateWaitableTimer on Windows, then, to implement the same pattern? -a
Jan Kiszka <jan.kiszka@siemens.com> writes: > On 2012-09-19 09:26, Paolo Bonzini wrote: >> Il 18/09/2012 22:37, Anthony Liguori ha scritto: >>> Unfortunately, there's a lot of Windows code in qemu-timer.c and main-loop.c >>> right now otherwise the refactoring would be trivial. I'll leave that for >>> another day. >>> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: Jan Kiszka <jan.kiszka@siemens.com> >>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >>> --- >>> Please note, this is lightly tested. Since this is such a fundamental change, >>> I'd like to do some performance analysis before committing but wanted to share >>> early. >> >> Looks good. I think Peter Portante tested something similar, and found no big >> difference between the two. But it's a good thing and, in my opinion, for >> non-timerfd OSes we should simply adjust the select() timeout and not bother >> with signals. > > What would be the advantage of timerfd over select? On Linux, both use > hrtimers (and low slack for RT processes). I'm starting to like the > select/WaitForMultipleObjects pattern as it would allow to consolidate > over basically two versions of timers and simplify the code. I don't think it's an either or. Ideally we get to a complete glib main loop and our timer implementation could fall back to use the timeout if timerfd wasn't available. Practically speaking, timerfd allows selecting the clock to use which select() doesn't and it also supports absolute events instead of relative events. Not sure either matters all that much, but no doubt it's better than using signals. Regarding microsecond granularity, note that while select() provides microsecond granualrity, poll() only provides millisecond granularity. I think there are a lot of good reasons to evaluate moving to poll() (glib uses poll by default). With poll(), we definitely need timerfd. Regards, Anthony Liguori > > Jan > >> >> I'm not sure if the same can be done for Windows, but I think it's possible as long >> as you keep the timeBeginPeriod/timeEndPeriod calls. As a start, Stefan, can you >> check if the win32 timer works for you with the calls added? Like this: >> >> diff --git a/qemu-timer.c b/qemu-timer.c >> index c7a1551..721c769 100644 >> --- a/qemu-timer.c >> +++ b/qemu-timer.c >> @@ -673,6 +673,10 @@ static int win32_start_timer(struct qemu_alarm_timer *t) >> HANDLE hTimer; >> BOOLEAN success; >> >> + timeGetDevCaps(&mm_tc, sizeof(mm_tc)); >> + >> + timeBeginPeriod(mm_tc.wPeriodMin); >> + >> /* If you call ChangeTimerQueueTimer on a one-shot timer (its period >> is zero) that has already expired, the timer is not updated. Since >> creating a new timer is relatively expensive, set a bogus one-hour >> @@ -688,6 +692,7 @@ static int win32_start_timer(struct qemu_alarm_timer *t) >> if (!success) { >> fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n", >> GetLastError()); >> + timeEndPeriod(mm_tc.wPeriodMin); >> return -1; >> } >> >> @@ -702,6 +707,7 @@ static void win32_stop_timer(struct qemu_alarm_timer *t) >> if (hTimer) { >> DeleteTimerQueueTimer(NULL, hTimer, NULL); >> } >> + timeEndPeriod(mm_tc.wPeriodMin); >> } >> >> static void win32_rearm_timer(struct qemu_alarm_timer *t, >> >> Paolo >> > > -- > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > Corporate Competence Center Embedded Linux
On Wed, Sep 19, 2012 at 10:27 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > Please turn of HTML in you mailer. It's very hard to parse your reply. > > On 2012-09-19 16:15, Peter Portante wrote: >> On Wed, Sep 19, 2012 at 3:44 AM, Jan Kiszka <jan.kiszka@siemens.com<mailto:jan.kiszka@siemens.com>> wrote: >> On 2012-09-19 09:26, Paolo Bonzini wrote: >>> Il 18/09/2012 22:37, Anthony Liguori ha scritto: >>>> Unfortunately, there's a lot of Windows code in qemu-timer.c and main-loop.c >>>> right now otherwise the refactoring would be trivial. I'll leave that for >>>> another day. >>>> >>>> Cc: Paolo Bonzini <pbonzini@redhat.com<mailto:pbonzini@redhat.com>> >>>> Cc: Jan Kiszka <jan.kiszka@siemens.com<mailto:jan.kiszka@siemens.com>> >>>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com<mailto:aliguori@us.ibm.com>> >>>> --- >>>> Please note, this is lightly tested. Since this is such a fundamental change, >>>> I'd like to do some performance analysis before committing but wanted to share >>>> early. >>> >>> Looks good. I think Peter Portante tested something similar, and found no big >>> difference between the two. But it's a good thing and, in my opinion, for >>> non-timerfd OSes we should simply adjust the select() timeout and not bother >>> with signals. >> >> What would be the advantage of timerfd over select? On Linux, both use >> hrtimers (and low slack for RT processes). >> >> I am not sure the comparison is timerfd v. select, but timerfd v signal based timer (setitimer). The timerfd path allows you to integrate with select/poll/epoll loops, where as signal based timers make that more difficult. One can do the same thing with signalfd, but only for one signal, where as you can setup multiple timers at the expense of file descriptors. >> >> Additionally, FWIW, select() has a resolution capped by its use of struct timeval, which is microseconds, where timerfd_settime allows for nanosecond resolution. > > < 1us resolution is pointless, even on RT-hardened kernels with fast > hardware underneath and when running natively. Perhaps. Under Linux, we have been unable to demonstrate sub-50us resolution for timeouts outside of RT scheduling classes. We have been able to get around 1us resolution using SCHED_RR or SCHED_FIFO. However, if native clocks return timer values at nanosecond resolution, why would you not want to maintain those units to avoid rounding errors or simple logic errors during conversions? Not sure how pointless all the work has been to create software APIs and hardware interfaces that use nanosecond resolution. >> >> I'm starting to like the >> select/WaitForMultipleObjects pattern as it would allow to consolidate >> over basically two versions of timers and simplify the code. >> >> With timerfd, signalfd and eventfd, Linux seems to have provided all the coverage needed to make that happen. > > The advantage is that timers based on select/poll timeouts will allow to > unify a lot of code for _all_ host platforms, i.e. even Windows. We > still need to evaluate the precise impact and look for potentially > missed limitations (aka: someone has to write patches and test them). > But if there are no relevant ones, it should be the better architecture. > > That said, a timerfd based solution for Linux may be an intermediate > step of the select-based work takes longer. > > Jan > > -- > Siemens AG, Corporate Technology, CT RTC ITP SDP-DE > Corporate Competence Center Embedded Linux
diff --git a/qemu-timer.c b/qemu-timer.c index c7a1551..721c769 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -673,6 +673,10 @@ static int win32_start_timer(struct qemu_alarm_timer *t) HANDLE hTimer; BOOLEAN success; + timeGetDevCaps(&mm_tc, sizeof(mm_tc)); + + timeBeginPeriod(mm_tc.wPeriodMin); + /* If you call ChangeTimerQueueTimer on a one-shot timer (its period is zero) that has already expired, the timer is not updated. Since creating a new timer is relatively expensive, set a bogus one-hour @@ -688,6 +692,7 @@ static int win32_start_timer(struct qemu_alarm_timer *t) if (!success) { fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n", GetLastError()); + timeEndPeriod(mm_tc.wPeriodMin); return -1; } @@ -702,6 +707,7 @@ static void win32_stop_timer(struct qemu_alarm_timer *t) if (hTimer) { DeleteTimerQueueTimer(NULL, hTimer, NULL); } + timeEndPeriod(mm_tc.wPeriodMin); } static void win32_rearm_timer(struct qemu_alarm_timer *t,