Blue Swirl wrote:
On 9/16/08, Anthony Liguori <[EMAIL PROTECTED]> wrote:
This patch refactors the AIO layer to allow multiple AIO implementations.  It's
 only possible because of the recent signalfd() patch.

 +/* This is a simple lock used to protect the aio_handlers list.  Specifically,
 + * it's used to ensure that no callbacks are removed while we're walking and
 + * dispatching callbacks.
 + */
 +static int walking_handlers;

Shouldn't this be volatile and/or atomic_t?

No, because this is just used to protect the aio_handlers list within the same thread. Here's the scenario we're protecting:

Walk aio_handlers list =>
  call handler =>
     handler calls qemu_aio_set_fd_handler() to delete aio entry

Since we're walking the list, we can't delete an entry while walking. This is the same problem we have in vl.c with the IOHandler list. Unlike the IOHandler list, we don't walk the aio_handler list often enough to just set a flag on the entry. In the case where qemu_aio_set_fd_handler() is called from something other than a handler callback, we need to be able to directly delete the entry element. This is why we use the "lock", to detect that case.

It's not a real lock, because two threads are never trying to access it at the same time. This is why we don't need volatile, atomic_t, or the real locking primitives.

Regards,

Anthony Liguori

Just wondering, why don't you use real locking operations, for example
those in qemu-lock.h?

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to