From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54173 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzU41-0004eg-2u for qemu-devel@nongnu.org; Tue, 15 Mar 2011 09:13:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PzU3y-0005Gk-OG for qemu-devel@nongnu.org; Tue, 15 Mar 2011 09:13:28 -0400 Received: from e3.ny.us.ibm.com ([32.97.182.143]:52733) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PzU3y-0005GK-Ld for qemu-devel@nongnu.org; Tue, 15 Mar 2011 09:13:26 -0400 Received: from d01dlp01.pok.ibm.com (d01dlp01.pok.ibm.com [9.56.224.56]) by e3.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p2FCr4rJ031659 for ; Tue, 15 Mar 2011 08:53:04 -0400 Received: from d01relay01.pok.ibm.com (d01relay01.pok.ibm.com [9.56.227.233]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id BB74838C803D for ; Tue, 15 Mar 2011 09:13:21 -0400 (EDT) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay01.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p2FDDOm5365708 for ; Tue, 15 Mar 2011 09:13:25 -0400 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p2FDDMu6026860 for ; Tue, 15 Mar 2011 09:13:24 -0400 Message-ID: <4D7F65EF.8080906@us.ibm.com> Date: Tue, 15 Mar 2011 08:13:19 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <20110315103453.GA23922@linux.vnet.ibm.com> <20110315103803.GC23922@linux.vnet.ibm.com> In-Reply-To: <20110315103803.GC23922@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [v1 PATCH 2/3]: Helper routines to use GLib threadpool infrastructure in 9pfs. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: arun@linux.vnet.ibm.com Cc: stefanha@gmail.com, jvrao@linux.vnet.ibm.com, qemu-devel@nongnu.org, aneesh.kumar@linux.vnet.ibm.com On 03/15/2011 05:38 AM, Arun R Bharadwaj wrote: > * Arun R Bharadwaj [2011-03-15 16:04:53]: > > Author: Arun R Bharadwaj > Date: Thu Mar 10 15:11:49 2011 +0530 > > Helper routines to use GLib threadpool infrastructure in 9pfs. > > This patch creates helper routines to make use of the > threadpool infrastructure provided by GLib. This is based > on the prototype patch by Anthony which does a similar thing > for posix-aio-compat.c > > An example use case is provided in the next patch where one > of the syscalls in 9pfs is converted into the threaded model > using these helper routines. > > Signed-off-by: Arun R Bharadwaj > Reviewed-by: Aneesh Kumar K.V Why even bothering signaling for completion with the virtio-9p threadpool? There's no sane guest that's going to poll on virtio-9p completion with interrupts disabled and no timer. Once we enable the I/O thread by default, it won't even be necessary for the paio layer. Regards, Anthony Liguori > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c > index dceefd5..cf61345 100644 > --- a/hw/9pfs/virtio-9p.c > +++ b/hw/9pfs/virtio-9p.c > @@ -18,6 +18,8 @@ > #include "fsdev/qemu-fsdev.h" > #include "virtio-9p-debug.h" > #include "virtio-9p-xattr.h" > +#include "signal.h" > +#include "qemu-thread.h" > > int debug_9p_pdu; > static void v9fs_reclaim_fd(V9fsState *s); > @@ -36,6 +38,89 @@ enum { > Oappend = 0x80, > }; > > +typedef struct V9fsPool { > + GThreadPool *pool; > + GList *requests; > + int rfd; > + int wfd; > +} V9fsPool; > + > +static V9fsPool v9fs_pool; > + > +static void v9fs_qemu_submit_request(V9fsRequest *req) > +{ > + V9fsPool *p =&v9fs_pool; > + > + p->requests = g_list_append(p->requests, req); > + g_thread_pool_push(v9fs_pool.pool, req, NULL); > +} > + > +static void die2(int err, const char *what) > +{ > + fprintf(stderr, "%s failed: %s\n", what, strerror(err)); > + abort(); > +} > + > +static void die(const char *what) > +{ > + die2(errno, what); > +} > + > +static void v9fs_qemu_process_post_ops(void *arg) > +{ > + struct V9fsPool *p =&v9fs_pool; > + struct V9fsPostOp *post_op; > + char byte; > + ssize_t len; > + GList *cur_req, *next_req; > + > + do { > + len = read(p->rfd,&byte, sizeof(byte)); > + } while (len == -1&& errno == EINTR); > + > + for (cur_req = p->requests; cur_req != NULL; cur_req = next_req) { > + V9fsRequest *req = cur_req->data; > + next_req = g_list_next(cur_req); > + > + if (!req->done) { > + continue; > + } > + > + post_op =&req->post_op; > + post_op->func(post_op->arg); > + p->requests = g_list_remove_link(p->requests, cur_req); > + g_list_free(p->requests); > + } > +} > + > +static inline void v9fs_thread_signal(void) > +{ > + struct V9fsPool *p =&v9fs_pool; > + char byte = 0; > + ssize_t ret; > + > + do { > + ret = write(p->wfd,&byte, sizeof(byte)); > + } while (ret == -1&& errno == EINTR); > + > + if (ret< 0&& errno != EAGAIN) { > + die("write() in v9fs"); > + } > + > + if (kill(getpid(), SIGUSR2)) { > + die("kill failed"); > + } > +} > + > +static void v9fs_thread_routine(gpointer data, gpointer user_data) > +{ > + V9fsRequest *req = data; > + > + req->func(req); > + v9fs_thread_signal(); > + req->done = 1; > +} > + > static int omode_to_uflags(int8_t mode) > { > int ret = 0; > @@ -3850,7 +3935,8 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf) > int i, len; > struct stat stat; > FsTypeEntry *fse; > - > + int fds[2]; > + V9fsPool *p =&v9fs_pool; > > s = (V9fsState *)virtio_common_init("virtio-9p", > VIRTIO_ID_9P, > @@ -3939,5 +4025,21 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf) > s->tag_len; > s->vdev.get_config = virtio_9p_get_config; > > + if (qemu_pipe(fds) == -1) { > + fprintf(stderr, "failed to create fd's for virtio-9p\n"); > + exit(1); > + } > + > + p->pool = g_thread_pool_new(v9fs_thread_routine, p, 8, FALSE, NULL); > + p->rfd = fds[0]; > + p->wfd = fds[1]; > + > + fcntl(p->rfd, F_SETFL, O_NONBLOCK); > + fcntl(p->wfd, F_SETFL, O_NONBLOCK); > + > + qemu_set_fd_handler(p->rfd, v9fs_qemu_process_post_ops, NULL, NULL); > + > + (void) v9fs_qemu_submit_request; > + > return&s->vdev; > } > diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h > index 10809ba..e7d2326 100644 > --- a/hw/9pfs/virtio-9p.h > +++ b/hw/9pfs/virtio-9p.h > @@ -124,6 +124,20 @@ struct V9fsPDU > QLIST_ENTRY(V9fsPDU) next; > }; > > +typedef struct V9fsPostOp { > + /* Post Operation routine to execute after executing syscall */ > + void (*func)(void *arg); > + void *arg; > +} V9fsPostOp; > + > +typedef struct V9fsRequest { > + void (*func)(struct V9fsRequest *req); > + > + /* Flag to indicate that request is satisfied, ready for post-processing */ > + int done; > + > + V9fsPostOp post_op; > +} V9fsRequest; > > /* FIXME > * 1) change user needs to set groups and stuff