[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: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant |
Date: |
Thu, 20 Jun 2013 10:16:43 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 |
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.
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.
Paolo