Re: [Qemu-devel] [PATCH] QEMUBH: make AioContext's bh re-entrant

[Paolo Bonzini <pbonzini@redhat.com>]

Il 15/06/2013 07:16, Paolo Bonzini ha scritto:
> ... I'm not sure that this works yet.  I see two problems:
> ctx->walking_bh needs to be accessed atomic, and while you are doing the
> deletions somebody could come up and start a read.  Havoc.

Hmm, are you just trying to protect aio_bh_poll from aio_bh_new, while
still having only one thread that can do aio_bh_poll (multiple producer,
single consumer)?

In this case what you have should work and this patch is almost ready
for commit.  In fact it's actually important to have it for the
dataplane stuff that Stefan is doing, I think.

But _please document what you are doing_!  It's not the first time that
I tell you this: locking policy must be _thoroughly_ documented, and
_not_ figured out by the reviewers.  Without this, I'm not going to give
my Reviewed-by.

Regarding barriers, there is another write barrier you need to add
before anything that sets bh->scheduled.  This makes sure any writes
that are needed by the callback are done before the locations are read
in the aio_bh_poll thread.  Similarly, you want a matching read barrier
before calling the callback.

Please pick up the atomics patch from my branch so that you have the
smp_read_barrier_depends() primitive; and resubmit with documentation
about the locking policy in the commit message _and_ the header files.
Also please add a comment before each barrier saying what is ordered
against what.

Thanks,

Paolo

> It's not an easy problem, because even with RCU the bottom half callback
> should run _outside_ the RCU critical section.  I suggest that you hunt
> the kernel for something that is trying to do the same.