Message ID | 1399538830-4443-1-git-send-email-s.vorobiov@samsung.com |
---|---|
State | New |
Headers | show |
On Thu, May 08, 2014 at 12:47:10PM +0400, Stanislav Vorobiov wrote: > From: Sangho Park <sangho1206.park@samsung.com> > > g_poll has a problem on Windows when using > timeouts < 10ms, in glib/gpoll.c: > > /* If not, and we have a significant timeout, poll again with > * timeout then. Note that this will return indication for only > * one event, or only for messages. We ignore timeouts less than > * ten milliseconds as they are mostly pointless on Windows, the > * MsgWaitForMultipleObjectsEx() call will timeout right away > * anyway. > */ > if (retval == 0 && (timeout == INFINITE || timeout >= 10)) > retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); > > so whenever g_poll is called with timeout < 10ms it does > a quick poll instead of wait, this causes significant performance > degradation of QEMU, thus we should use WaitForMultipleObjectsEx > directly > > Signed-off-by: Stanislav Vorobiov <s.vorobiov@samsung.com> > --- > include/glib-compat.h | 9 +++- > util/oslib-win32.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 120 insertions(+), 1 deletion(-) Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
Am 09.05.2014 20:57, schrieb Stefan Hajnoczi: > On Thu, May 08, 2014 at 12:47:10PM +0400, Stanislav Vorobiov wrote: >> From: Sangho Park <sangho1206.park@samsung.com> >> >> g_poll has a problem on Windows when using >> timeouts < 10ms, in glib/gpoll.c: >> >> /* If not, and we have a significant timeout, poll again with >> * timeout then. Note that this will return indication for only >> * one event, or only for messages. We ignore timeouts less than >> * ten milliseconds as they are mostly pointless on Windows, the >> * MsgWaitForMultipleObjectsEx() call will timeout right away >> * anyway. >> */ >> if (retval == 0 && (timeout == INFINITE || timeout >= 10)) >> retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); >> >> so whenever g_poll is called with timeout < 10ms it does >> a quick poll instead of wait, this causes significant performance >> degradation of QEMU, thus we should use WaitForMultipleObjectsEx >> directly >> >> Signed-off-by: Stanislav Vorobiov <s.vorobiov@samsung.com> >> --- >> include/glib-compat.h | 9 +++- >> util/oslib-win32.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 120 insertions(+), 1 deletion(-) > > Thanks, applied to my block tree: > https://github.com/stefanha/qemu/commits/block > > Stefan > Hi, the patch breaks hard disk i/o: a 64 bit executable built with MinGW-w64 will no longer boot a ReactOS image (BIOS fails to detect a bootable disk). Booting a Linux CDROM seems to work and is indeed much faster. Regards Stefan
Hi, Could you please provide URL to download that image so that I could reproduce this problem ? Also, qemu command line would be nice. btw, I won't be able to look into this during next week, so some help from someone else would be nice. I'll try to take a look later today however... On 05/16/2014 09:00 AM, Stefan Weil wrote: > Am 09.05.2014 20:57, schrieb Stefan Hajnoczi: >> On Thu, May 08, 2014 at 12:47:10PM +0400, Stanislav Vorobiov wrote: >>> From: Sangho Park <sangho1206.park@samsung.com> >>> >>> g_poll has a problem on Windows when using >>> timeouts < 10ms, in glib/gpoll.c: >>> >>> /* If not, and we have a significant timeout, poll again with >>> * timeout then. Note that this will return indication for only >>> * one event, or only for messages. We ignore timeouts less than >>> * ten milliseconds as they are mostly pointless on Windows, the >>> * MsgWaitForMultipleObjectsEx() call will timeout right away >>> * anyway. >>> */ >>> if (retval == 0 && (timeout == INFINITE || timeout >= 10)) >>> retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); >>> >>> so whenever g_poll is called with timeout < 10ms it does >>> a quick poll instead of wait, this causes significant performance >>> degradation of QEMU, thus we should use WaitForMultipleObjectsEx >>> directly >>> >>> Signed-off-by: Stanislav Vorobiov <s.vorobiov@samsung.com> >>> --- >>> include/glib-compat.h | 9 +++- >>> util/oslib-win32.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 120 insertions(+), 1 deletion(-) >> >> Thanks, applied to my block tree: >> https://github.com/stefanha/qemu/commits/block >> >> Stefan >> > > > > Hi, > > the patch breaks hard disk i/o: a 64 bit executable built with MinGW-w64 > will no longer boot a ReactOS image (BIOS fails to detect a bootable > disk). Booting a Linux CDROM seems to work and is indeed much faster. > > Regards > Stefan > >
Am 16.05.2014 08:44, schrieb Stanislav Vorobiov: > Hi, > > Could you please provide URL to download that image so that I > could reproduce this problem ? Also, qemu command line would be nice. > > btw, I won't be able to look into this during next week, so some help > from someone else would be nice. I'll try to take a look later today > however... > There is no need to download an image. Any image will do for this test. The command line also does not matter. I just tried this: i386-softmmu/qemu-system-i386 -L pc-bios vl.c With the current qemu trunk and a 64 bit default build (MinGW-w64 native or cross), the BIOS cannot read the harddisk. When I undefine g_poll in qemu-timer.c, QEMU can read it and the BIOS will report that vl.c is not a bootable disk (which is quite obvious). Stefan
Il 16/05/2014 07:00, Stefan Weil ha scritto: > Hi, > > the patch breaks hard disk i/o: a 64 bit executable built with MinGW-w64 > will no longer boot a ReactOS image (BIOS fails to detect a bootable > disk). Booting a Linux CDROM seems to work and is indeed much faster. What about a 32-bit executable? Paolo
Am 16.05.2014 13:32, schrieb Paolo Bonzini: > Il 16/05/2014 07:00, Stefan Weil ha scritto: >> Hi, >> >> the patch breaks hard disk i/o: a 64 bit executable built with MinGW-w64 >> will no longer boot a ReactOS image (BIOS fails to detect a bootable >> disk). Booting a Linux CDROM seems to work and is indeed much faster. > > What about a 32-bit executable? > > Paolo They show the same effect. When I reported the problem, I had no test result for 32 bit, now I have one. Native builds on Windows are terribly slow and take a lot of time... Summary: Neither 32 nor 64 bit qemu system emulation on Windows hosts can read hard disk images with this patch applied. It's sufficient to test any image and see whether the BIOS detects the hard disk. Stefan
diff --git a/include/glib-compat.h b/include/glib-compat.h index 8d25900..1280fb2 100644 --- a/include/glib-compat.h +++ b/include/glib-compat.h @@ -24,7 +24,14 @@ static inline guint g_timeout_add_seconds(guint interval, GSourceFunc function, } #endif -#if !GLIB_CHECK_VERSION(2, 20, 0) +#ifdef _WIN32 +/* + * g_poll has a problem on Windows when using + * timeouts < 10ms, so use wrapper. + */ +#define g_poll(fds, nfds, timeout) g_poll_fixed(fds, nfds, timeout) +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout); +#elif !GLIB_CHECK_VERSION(2, 20, 0) /* * Glib before 2.20.0 doesn't implement g_poll, so wrap it to compile properly * on older systems. diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 93f7d35..69552f7 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -238,3 +238,115 @@ char *qemu_get_exec_dir(void) { return g_strdup(exec_dir); } + +/* + * g_poll has a problem on Windows when using + * timeouts < 10ms, in glib/gpoll.c: + * + * // If not, and we have a significant timeout, poll again with + * // timeout then. Note that this will return indication for only + * // one event, or only for messages. We ignore timeouts less than + * // ten milliseconds as they are mostly pointless on Windows, the + * // MsgWaitForMultipleObjectsEx() call will timeout right away + * // anyway. + * + * if (retval == 0 && (timeout == INFINITE || timeout >= 10)) + * retval = poll_rest (poll_msgs, handles, nhandles, fds, nfds, timeout); + * + * So whenever g_poll is called with timeout < 10ms it does + * a quick poll instead of wait, this causes significant performance + * degradation of QEMU, thus we should use WaitForMultipleObjectsEx + * directly + */ +gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout) +{ + guint i; + HANDLE handles[MAXIMUM_WAIT_OBJECTS]; + gint nhandles = 0; + int num_completed = 0; + + for (i = 0; i < nfds; i++) { + gint j; + + if (fds[i].fd <= 0) { + continue; + } + + /* don't add same handle several times + */ + for (j = 0; j < nhandles; j++) { + if (handles[j] == (HANDLE)fds[i].fd) { + break; + } + } + + if (j == nhandles) { + if (nhandles == MAXIMUM_WAIT_OBJECTS) { + fprintf(stderr, "Too many handles to wait for!\n"); + break; + } else { + handles[nhandles++] = (HANDLE)fds[i].fd; + } + } + } + + for (i = 0; i < nfds; ++i) { + fds[i].revents = 0; + } + + if (timeout == -1) { + timeout = INFINITE; + } + + if (nhandles == 0) { + if (timeout == INFINITE) { + return -1; + } else { + SleepEx(timeout, TRUE); + return 0; + } + } + + while (1) { + DWORD res; + gint j; + + res = WaitForMultipleObjectsEx(nhandles, handles, FALSE, + timeout, TRUE); + + if (res == WAIT_FAILED) { + for (i = 0; i < nfds; ++i) { + fds[i].revents = 0; + } + + return -1; + } else if ((res == WAIT_TIMEOUT) || (res == WAIT_IO_COMPLETION) || + ((int)res < (int)WAIT_OBJECT_0) || + (res >= (WAIT_OBJECT_0 + nhandles))) { + break; + } + + for (i = 0; i < nfds; ++i) { + if (handles[res - WAIT_OBJECT_0] == (HANDLE)fds[i].fd) { + fds[i].revents = fds[i].events; + } + } + + ++num_completed; + + if (nhandles <= 1) { + break; + } + + /* poll the rest of the handles + */ + for (j = res - WAIT_OBJECT_0 + 1; j < nhandles; j++) { + handles[j - 1] = handles[j]; + } + --nhandles; + + timeout = 0; + } + + return num_completed; +}