From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58082 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PG77v-0007wM-HE for qemu-devel@nongnu.org; Wed, 10 Nov 2010 04:38:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PG77u-0005Fj-7K for qemu-devel@nongnu.org; Wed, 10 Nov 2010 04:37:59 -0500 Received: from mail-ww0-f53.google.com ([74.125.82.53]:62843) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PG77t-0005FK-V5 for qemu-devel@nongnu.org; Wed, 10 Nov 2010 04:37:58 -0500 Received: by wwb28 with SMTP id 28so4598wwb.10 for ; Wed, 10 Nov 2010 01:37:56 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <4CD9F858.50306@linux.vnet.ibm.com> References: <20101108104542.6769.22583.stgit@localhost6.localdomain6> <20101108104707.6769.69626.stgit@localhost6.localdomain6> <4CD9F858.50306@linux.vnet.ibm.com> Date: Wed, 10 Nov 2010 09:37:56 +0000 Message-ID: Subject: Re: [Qemu-devel] [PATCH 3/3] Add helper functions to enable virtio-9p make use of the threadlets From: Stefan Hajnoczi Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Venkateswararao Jujjuri (JV)" Cc: Arun R Bharadwaj , qemu-devel@nongnu.org On Wed, Nov 10, 2010 at 1:41 AM, Venkateswararao Jujjuri (JV) wrote: > On 11/9/2010 1:06 AM, Stefan Hajnoczi wrote: >> On Mon, Nov 8, 2010 at 10:47 AM, Arun R Bharadwaj >> wrote: >>> + >>> +static struct { >>> + =A0 =A0int rfd; >>> + =A0 =A0int wfd; >>> + =A0 =A0QemuMutex lock; >>> + =A0 =A0QTAILQ_HEAD(, v9fs_post_op) post_op_list; >>> +} v9fs_async_struct; >>> + >>> +static void die2(int err, const char *what) >>> +{ >>> + =A0 =A0fprintf(stderr, "%s failed: %s\n", what, strerror(err)); >>> + =A0 =A0abort(); >>> +} >>> + >>> +static void die(const char *what) >>> +{ >>> + =A0 =A0die2(errno, what); >>> +} >>> + >>> +#define ASYNC_MAX_PROCESS =A0 5 >>> + >>> +/** >>> + * v9fs_process_post_ops: Process any pending v9fs_post_posix_operatio= n >>> + * @arg: Not used. >>> + * >>> + * This function serves as a callback to the iothread to be called int= o whenever >>> + * the v9fs_async_struct.wfd is written into. This thread goes through= the list >>> + * of v9fs_post_posix_operations() and executes them. In the process, = it might >>> + * queue more job on the asynchronous thread pool. >>> + */ >>> +static void v9fs_process_post_ops(void *arg) >>> +{ >>> + =A0 =A0int count =3D 0; >>> + =A0 =A0struct v9fs_post_op *post_op; >>> + =A0 =A0int ret; >>> + =A0 =A0char byte; >>> + >>> + =A0 =A0qemu_mutex_lock(&v9fs_async_struct.lock); >>> + =A0 =A0do { >>> + =A0 =A0 =A0 =A0ret =3D read(v9fs_async_struct.rfd, &byte, sizeof(byte= )); >>> + =A0 =A0} while (ret >=3D 0 && errno !=3D EAGAIN); >> >> } while (ret > 1 || (ret =3D=3D -1 && errno =3D=3D EINTR)); ? > > Not sure why we need to retry if ret > 1; all we need is > > } while (ret =3D=3D -1 && errno =3D=3D EINTR); We've been notified. Now we want to drain the pipe fully in case we were notified multiple times and bytes have been buffered up before we got around to processing pending post_ops. >>> diff --git a/posix-aio-compat.c b/posix-aio-compat.c >>> index 9e02d18..8019be7 100644 >>> --- a/posix-aio-compat.c >>> +++ b/posix-aio-compat.c >>> @@ -260,6 +260,8 @@ static ssize_t handle_aiocb_rw(struct qemu_paiocb *= aiocb) >>> =A0 =A0 return nbytes; >>> =A0} >>> >>> +static PosixAioState */; >>> + >>> =A0static void aio_thread(ThreadletWork *work) >>> =A0{ >>> =A0 =A0 pid_t pid; >>> @@ -290,6 +292,15 @@ static void aio_thread(ThreadletWork *work) >>> =A0 =A0 aiocb->ret =3D ret; >>> =A0 =A0 qemu_mutex_unlock(&aiocb_mutex); >>> >>> + =A0 =A0if (posix_aio_state) { >> >> This test is not needed because posix_aio_state must be non-NULL here. > > I see similar check in the old code also.. The old code was in a signal handler, which means it could be called at an inconvenient moment - before posix_aio_state has been initialized. But now we're guaranteed that posix_aio_state is initialized because we'd never get to aio_thread() otherwise. Stefan