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
