From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Nw8aL-00059p-GY for qemu-devel@nongnu.org; Mon, 29 Mar 2010 02:36:29 -0400 Received: from [140.186.70.92] (port=49502 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Nw8aJ-000579-Nl for qemu-devel@nongnu.org; Mon, 29 Mar 2010 02:36:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Nw8aI-0007Kd-0Y for qemu-devel@nongnu.org; Mon, 29 Mar 2010 02:36:27 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:39515) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Nw8aH-0007K3-T8 for qemu-devel@nongnu.org; Mon, 29 Mar 2010 02:36:25 -0400 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by e7.ny.us.ibm.com (8.14.3/8.13.1) with ESMTP id o2T6RaD2002410 for ; Mon, 29 Mar 2010 02:27:36 -0400 Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o2T6aMKj142798 for ; Mon, 29 Mar 2010 02:36:22 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id o2T6aLR7024668 for ; Mon, 29 Mar 2010 02:36:22 -0400 Message-ID: <4BB04A59.1040204@linux.vnet.ibm.com> Date: Sun, 28 Mar 2010 23:36:09 -0700 From: jvrao MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH -V3 09/32] virtio-9p: Implement P9_TWRITE/ Thread model in QEMU References: <1269535420-31206-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1269535420-31206-10-git-send-email-aneesh.kumar@linux.vnet.ibm.com> In-Reply-To: <1269535420-31206-10-git-send-email-aneesh.kumar@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Aneesh Kumar K.V" , aliguori@us.ibm.com Cc: ericvh@gmail.com, qemu-devel@nongnu.org Aneesh Kumar K.V wrote: > From: Anthony Liguori We have implemented all the vfs calls in state machine model so that we are prepared for the model where the VCPU thread(s) does the initial work until it needs to block then it submits that work (via a function pointer) to a thread pool. A thread in that thread pool picks up the work, and completes the blocking call, when blocking call returns a callback is invoked in the IO thread. Then the IO thread runs until the next blocking function, and goto start. Basically the VCPU/IO threads does all the non-blocking work, and let the threads in the thread pool work on the blocking calls like mkdir() stat() etc. My question is, why not let the whole work done by the thread in the thread pool? VCPU thread receives the PDU and hands over the entire job to worker thread. When all work is completed, either the worker thread or the IO thread(we can switch back at this point if needed) marks the request as completed in the virtqueue and injects an interrupt to notify the guest. We can still keep the same number of threads in the thread pool. This way, we are not increasing #of threads employed by QEMU...also it makes code lot more easy to read/maintain. I may be missing something..but would like to know more on the advantages of this model. Thanks, JV > > This gets write to file to work > > Signed-off-by: Anthony Liguori > Signed-off-by: Venkateswararao Jujjuri > Signed-off-by: Aneesh Kumar K.V > --- > hw/virtio-9p-local.c | 7 ++++ > hw/virtio-9p.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 102 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c > index d77ecc2..c5d1db3 100644 > --- a/hw/virtio-9p-local.c > +++ b/hw/virtio-9p-local.c > @@ -129,6 +129,12 @@ static off_t local_lseek(void *opaque, int fd, off_t offset, int whence) > return lseek(fd, offset, whence); > } > > +static ssize_t local_writev(void *opaque, int fd, const struct iovec *iov, > + int iovcnt) > +{ > + return writev(fd, iov, iovcnt); > +} > + > static V9fsPosixFileOperations ops = { > .lstat = local_lstat, > .setuid = local_setuid, > @@ -143,6 +149,7 @@ static V9fsPosixFileOperations ops = { > .seekdir = local_seekdir, > .readv = local_readv, > .lseek = local_lseek, > + .writev = local_writev, > }; > > V9fsPosixFileOperations *virtio_9p_init_local(const char *path) > diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c > index 3ac6255..bc26d66 100644 > --- a/hw/virtio-9p.c > +++ b/hw/virtio-9p.c > @@ -168,6 +168,12 @@ static off_t posix_lseek(V9fsState *s, int fd, off_t offset, int whence) > return s->ops->lseek(s->ops->opaque, fd, offset, whence); > } > > +static int posix_writev(V9fsState *s, int fd, const struct iovec *iov, > + int iovcnt) > +{ > + return s->ops->writev(s->ops->opaque, fd, iov, iovcnt); > +} > + > static void v9fs_string_init(V9fsString *str) > { > str->data = NULL; > @@ -1319,10 +1325,97 @@ out: > complete_pdu(s, pdu, err); > } > > +typedef struct V9fsWriteState { > + V9fsPDU *pdu; > + size_t offset; > + int32_t fid; > + int32_t len; > + int32_t count; > + int32_t total; > + int64_t off; > + V9fsFidState *fidp; > + struct iovec iov[128]; /* FIXME: bad, bad, bad */ > + struct iovec *sg; > + int cnt; > +} V9fsWriteState; > + > +static void v9fs_write_post_writev(V9fsState *s, V9fsWriteState *vs, > + ssize_t err) > +{ > + BUG_ON(vs->len < 0); > + vs->total += vs->len; > + vs->sg = adjust_sg(vs->sg, vs->len, &vs->cnt); > + if (vs->total < vs->count && vs->len > 0) { > + do { > + if (0) > + print_sg(vs->sg, vs->cnt); > + vs->len = posix_writev(s, vs->fidp->fd, vs->sg, vs->cnt); > + } while (vs->len == -1 && errno == EINTR); > + v9fs_write_post_writev(s, vs, err); > + } > + vs->offset += pdu_marshal(vs->pdu, vs->offset, "d", vs->total); > + > + err = vs->offset; > + complete_pdu(s, vs->pdu, err); > + qemu_free(vs); > +} > + > +static void v9fs_write_post_lseek(V9fsState *s, V9fsWriteState *vs, ssize_t err) > +{ > + BUG_ON(err == -1); > + > + vs->sg = cap_sg(vs->sg, vs->count, &vs->cnt); > + > + if (vs->total < vs->count) { > + do { > + if (0) > + print_sg(vs->sg, vs->cnt); > + vs->len = posix_writev(s, vs->fidp->fd, vs->sg, vs->cnt); > + } while (vs->len == -1 && errno == EINTR); > + > + v9fs_write_post_writev(s, vs, err); > + return; > + } > + > + complete_pdu(s, vs->pdu, err); > + qemu_free(vs); > +} > + > static void v9fs_write(V9fsState *s, V9fsPDU *pdu) > { > - if (debug_9p_pdu) > - pprint_pdu(pdu); > + V9fsWriteState *vs; > + ssize_t err; > + > + vs = qemu_malloc(sizeof(*vs)); > + > + vs->pdu = pdu; > + vs->offset = 7; > + vs->sg = vs->iov; > + vs->total = 0; > + vs->len = 0; > + > + pdu_unmarshal(vs->pdu, vs->offset, "dqdv", &vs->fid, &vs->off, &vs->count, > + vs->sg, &vs->cnt); > + > + vs->fidp = lookup_fid(s, vs->fid); > + if (vs->fidp == NULL) { > + err = -EINVAL; > + goto out; > + } > + > + if (vs->fidp->fd == -1) { > + err = -EINVAL; > + goto out; > + } > + > + err = posix_lseek(s, vs->fidp->fd, vs->off, SEEK_SET); > + > + v9fs_write_post_lseek(s, vs, err); > + return; > + > +out: > + complete_pdu(s, vs->pdu, err); > + qemu_free(vs); > } > > static void v9fs_create(V9fsState *s, V9fsPDU *pdu)