[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanentl
From: |
Fabien Chouteau |
Subject: |
Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails |
Date: |
Wed, 24 Jun 2015 11:09:00 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 06/23/2015 07:07 PM, Stefan Weil wrote:
> Am 23.06.2015 um 12:46 schrieb Paolo Bonzini:
>> On 23/06/2015 12:30, Peter Maydell wrote:
>>> On 23 June 2015 at 10:55, Ян Завадовский <address@hidden> wrote:
>>>> On Tue, Jun 23, 2015 at 9:02 AM, Stefan Weil <address@hidden> wrote:
>>>>> We should add an URL to reliable documentation which supports that
>>>>> claim.
>>>> Unfortunately, MSDN says only "SuspendThread suspends the thread. It's
>>>> designed for debuggers. Don't use in applications.":
>>>> https://msdn.microsoft.com/en-us/library/windows/desktop/ms686345(v=vs.85).aspx
>>>> And nothing more useful.
>>>> So when I found this piece of code with Suspend/Resume and failed
>>>> GetContext
>>>> I did some googling.
>>>> And found this article:
>>>> http://blogs.msdn.com/b/oldnewthing/archive/2015/02/05/10591215.aspx
>>> Personally I am happy to treat a Raymond Chen blog post as "reliable
>>> documentation"...
>> Me too. :)
>
> +1
>
> Fabien, I wonder why nobody noticed that the current
> code did not do what it was written for. As far as I see
> the threads were created with the wrong options, so
> GetThreadContext always failed and therefore was only
> executed once, so there was no waiting for thread
> suspension.
>
I'm surprised as well, but we run several hundred thousands of tests
every day (one QEMU instance for each test) and before this fix we had a
few instances freezing for no reason. We identified a possible race
condition on SMP host and the bug disappeared after this fix.
Even if the call was erroneous, adding a call to GetThreadContext
probably gave more time or forced the suspend request to be effective,
it's the only explanation I have right now.
But clearly there was a bug, and the call to GetThreadContext fixed it.
I found other pieces of code that uses this technique but calling
GetThreadContext only once (not in a loop like we did), so maybe it's
enough to call it once and the loop is superfluous...
> Removing the code would have given identical results.
>
Considering we are talking about thread synchronization on Windows and
SMP host, I would not make that assumption :)
> Is that in an indicator that the SuspendThread is not
> needed at all, as it was discussed in the other e-mails
> here?
If we completely change the thread synchronization on Windows, maybe
SuspendeThread is not needed anymore, but with the current scheme (at
least what I know of it), I don't see how we can remove it.
As I said before we must be very careful with this piece of code.
Regards,
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, (continued)
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Ян Завадовский, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Peter Maydell, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Paolo Bonzini, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Daniel P. Berrange, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Paolo Bonzini, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Daniel P. Berrange, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Paolo Bonzini, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Peter Maydell, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Stefan Weil, 2015/06/23
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails,
Fabien Chouteau <=
- Re: [Qemu-devel] [PATCH] thread-win32: fix GetThreadContext() permanently fails, Peter Maydell, 2015/06/24