From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1LMfOp-0005sw-9s
	for qemu-devel@nongnu.org; Tue, 13 Jan 2009 04:17:27 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1LMfOn-0005rc-IS
	for qemu-devel@nongnu.org; Tue, 13 Jan 2009 04:17:26 -0500
Received: from [199.232.76.173] (port=37869 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1LMfOn-0005rU-5V
	for qemu-devel@nongnu.org; Tue, 13 Jan 2009 04:17:25 -0500
Received: from gecko.sbs.de ([194.138.37.40]:15073)
	by monty-python.gnu.org with esmtps
	(TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60)
	(envelope-from <jan.kiszka@siemens.com>) id 1LMfOm-0000CS-ND
	for qemu-devel@nongnu.org; Tue, 13 Jan 2009 04:17:25 -0500
Message-ID: <496C5C1E.1070900@siemens.com>
Date: Tue, 13 Jan 2009 10:17:18 +0100
From: Jan Kiszka <jan.kiszka@siemens.com>
MIME-Version: 1.0
References: <494B95CC.8070008@siemens.com>
In-Reply-To: <494B95CC.8070008@siemens.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH] Fix race in POSIX AIO emulation
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: qemu-devel@nongnu.org

Jan Kiszka wrote:
> When we cancel an AIO request that is already being processed by
> aio_thread, qemu_paio_cancel should return QEMU_PAIO_NOTCANCELED as long
> as aio_thread isn't done with this request. But as the latter currently
> updates aiocb->ret after every block of the request, we may report
> QEMU_PAIO_ALLDONE too early.
> 
> Futhermore, in case some zero-length request should have been queued,
> aiocb->ret is never set to != -EINPROGRESS and callers like
> raw_aio_cancel could get stuck in an endless loop.
> 
> Fix those issues by updating aiocb->ret _after_ the request has been
> fully processed. This also simplifies the locking.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
>  posix-aio-compat.c |    9 ++-------
>  1 files changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/posix-aio-compat.c b/posix-aio-compat.c
> index 92ec234..c919e3b 100644
> --- a/posix-aio-compat.c
> +++ b/posix-aio-compat.c
> @@ -81,21 +81,16 @@ static void *aio_thread(void *unused)
>              if (len == -1 && errno == EINTR)
>                  continue;
>              else if (len == -1) {
> -                pthread_mutex_lock(&lock);
> -                aiocb->ret = -errno;
> -                pthread_mutex_unlock(&lock);
> +                offset = -errno;
>                  break;
>              } else if (len == 0)
>                  break;
>  
>              offset += len;
> -
> -            pthread_mutex_lock(&lock);
> -            aiocb->ret = offset;
> -            pthread_mutex_unlock(&lock);
>          }
>  
>          pthread_mutex_lock(&lock);
> +        aiocb->ret = offset;
>          idle_threads++;
>          pthread_mutex_unlock(&lock);
>  
> 

Problem still exists, patch still applies - but no feedback yet.
Forgotten under the Christmas tree?

Jan

-- 
Siemens AG, Corporate Technology, CT SE 26
Corporate Competence Center Embedded Linux