qemu-devel
[Top][All Lists]
Advanced

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

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


From: liu ping fan
Subject: Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant
Date: Thu, 20 Jun 2013 17:12:38 +0800

On Thu, Jun 20, 2013 at 4:16 PM, Paolo Bonzini <address@hidden> wrote:
> Il 20/06/2013 09:39, Stefan Hajnoczi ha scritto:
>> qemu_bh_cancel() and qemu_bh_delete() are not modified by this patch.
>>
>> It seems that calling them from a thread is a little risky because there
>> is no guarantee that the BH is no longer invoked after a thread calls
>> these functions.
>>
>> I think that's worth a comment or do you want them to take the lock so
>> they become safe?
>
> Taking the lock wouldn't help.  The invoking loop of aio_bh_poll runs
> lockless.  I think a comment is better.
>
Yes, will document it.
> qemu_bh_cancel is inherently not thread-safe, there's not much you can
> do about it.
>
> qemu_bh_delete is safe as long as you wait for the bottom half to stop
> before deleting the containing object.  Once we have RCU, deletion of
> QOM objects will be RCU-protected.  Hence, a simple way could be to put
> the first part of aio_bh_poll() within rcu_read_lock/unlock.
>
>> The other thing I'm unclear on is the ->idle assignment followed
>> immediately by a ->scheduled assignment.  Without memory barriers
>> aio_bh_poll() isn't guaranteed to get an ordered view of these updates:
>> it may see an idle BH as a regular scheduled BH because ->idle is still
>> 0.
>
> Right.  You need to order ->idle writes before ->scheduled writes, and
> add memory barriers, or alternatively use two bits in ->scheduled so
> that you can assign both atomically.
>
I think just shift the position of smp_rmb/wmb in _schedule and _poll,
we can acheive this (callbacks will not refer to ->idle)

Regards,
Pingfan

> Paolo



reply via email to

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