From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] [PATCH] Refactor AIO to allow multiple AIO implementations Date: Tue, 16 Sep 2008 14:03:05 -0500 Message-ID: <48D002E9.3070400@us.ibm.com> References: <1221582247-8886-1-git-send-email-aliguori@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, Ryan Harper , kvm@vger.kernel.org To: Blue Swirl Return-path: Received: from e36.co.us.ibm.com ([32.97.110.154]:46707 "EHLO e36.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756595AbYIPTE1 (ORCPT ); Tue, 16 Sep 2008 15:04:27 -0400 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by e36.co.us.ibm.com (8.13.8/8.13.8) with ESMTP id m8GJ4P2Q011809 for ; Tue, 16 Sep 2008 15:04:25 -0400 Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id m8GJ4HtQ082870 for ; Tue, 16 Sep 2008 13:04:21 -0600 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m8GJ41es007937 for ; Tue, 16 Sep 2008 13:04:05 -0600 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: Blue Swirl wrote: > On 9/16/08, Anthony Liguori 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? >