[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_can

From: Pavel Butsykin
Subject: Re: [Qemu-devel] [PATCH v3] util/async: use atomic_mb_set in qemu_bh_cancel
Date: Wed, 8 Nov 2017 17:32:23 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

On 08.11.2017 17:24, Sergio Lopez wrote:
On Wed, Nov 8, 2017 at 3:15 PM, Paolo Bonzini <address@hidden> wrote:
On 08/11/2017 15:10, Sergio Lopez wrote:
I'm not quite sure that the pre-fetched is involved in this issue,
because pre-fetch reading a certain addresses should be invalidated by
write on another core to the same addresses. In our case write
req->state = THREAD_DONE should invalidate read req->state == THREAD_DONE.
I am inclined to think that there is a memory-reordering read with
write. It's a very real case for x86 and I don't see the reasons which
can prevent it:

Yes, you're right. This is actually a memory reordering issue. I'm
going to rewrite that paragraph.

Well, memory reordering _is_ caused by speculative prefetching, delayed
cache invalidation (store buffers), and so on.

But it's probably better indeed to replace "pre-fetched" with
"outdated".  Whoever commits the patch can do the substitution (I can too).

Alternatively, if we want to explicitly mention the memory barrier, we
can replace the third paragraph with something like this:

This was considered to be safe, as the completion function restarts the
loop just after the call to qemu_bh_cancel. But, as this loop lacks a HW
memory barrier, the read of req->state may actually happen _before_ the
call, seeing it still as THREAD_QUEUED, and ending the completion
function without having processed a pending TPE linked at pool->head:

Yes, that's better. Thank you.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]