On Sat, Jun 15, 2013 at 11:44 PM, Paolo Bonzini <pbonz...@redhat.com> wrote: > 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)? > Yes :) > 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. > Ok, will document it. > 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. > Will fix. > 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. > Ok. Thanks and regards, Pingfan
> 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.
