[Top][All Lists]

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

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

From: liu ping fan
Subject: Re: [Qemu-devel] [PATCH] QEMUBH: make AioContext's bh re-entrant
Date: Sun, 16 Jun 2013 18:00:46 +0800

On Sat, Jun 15, 2013 at 11:44 PM, Paolo Bonzini <address@hidden> 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.
Thanks and regards,

> 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.

reply via email to

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