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

[Stefan Hajnoczi]

On Wed, Nov 08, 2017 at 05:32:23PM +0300, Pavel Butsykin wrote:
> 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:
> > 
> > <snip>
> > 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:
> > </snip>
> 
> Yes, that's better. Thank you.

I have updated the commit description and sent an updated pull request
for QEMU 2.11-rc1.

Stefan

signature.asc
Description: PGP signature