From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Aneesh Kumar K.V" Subject: Re: [PATCH] tools/kvm/9p: Add encode/decode routines for protocol data Date: Tue, 28 Jun 2011 12:09:24 +0530 Message-ID: <87d3hypihv.fsf@linux.vnet.ibm.com> References: <1308651608-13534-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1308923238.8038.5.camel@lappy> <87wrgbrzi1.fsf@linux.vnet.ibm.com> <1308958008.10806.6.camel@lappy> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: penberg@kernel.org, kvm@vger.kernel.org To: Sasha Levin Return-path: Received: from e6.ny.us.ibm.com ([32.97.182.146]:47955 "EHLO e6.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756720Ab1F1Gjo (ORCPT ); Tue, 28 Jun 2011 02:39:44 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by e6.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p5S6FPlI001022 for ; Tue, 28 Jun 2011 02:15:25 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p5S6dVmd1642520 for ; Tue, 28 Jun 2011 02:39:32 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p5S6dVLj021809 for ; Tue, 28 Jun 2011 03:39:31 -0300 In-Reply-To: <1308958008.10806.6.camel@lappy> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, 24 Jun 2011 19:26:48 -0400, Sasha Levin wrote: > On Fri, 2011-06-24 at 21:30 +0530, Aneesh Kumar K.V wrote: > > On Fri, 24 Jun 2011 09:47:18 -0400, Sasha Levin wrote: > > > On Tue, 2011-06-21 at 15:50 +0530, Aneesh Kumar K.V wrote: > > > > +static void virtio_p9_pdu_read(struct p9_pdu *pdu, void *data, size_t size) > > > > +{ > > > > + size_t len; > > > > + int i, copied = 0; > > > > + u16 iov_cnt = pdu->out_iov_cnt; > > > > + size_t offset = pdu->read_offset; > > > > + struct iovec *iov = pdu->out_iov; > > > > + > > > > + for (i = 0; i < iov_cnt && size; i++) { > > > > + if (offset >= iov[i].iov_len) { > > > > + offset -= iov[i].iov_len; > > > > + continue; > > > > + } else { > > > > + len = MIN(iov[i].iov_len - offset, size); > > > > + memcpy(data, iov[i].iov_base + offset, len); > > > > + size -= len; > > > > + data += len; > > > > + offset = 0; > > > > + copied += len; > > > > + } > > > > + } > > > > + pdu->read_offset += copied; > > > > +} > > > > + > > > > +static void virtio_p9_pdu_write(struct p9_pdu *pdu, > > > > + const void *data, size_t size) > > > > +{ > > > > + size_t len; > > > > + int i, copied = 0; > > > > + u16 iov_cnt = pdu->in_iov_cnt; > > > > + size_t offset = pdu->write_offset; > > > > + struct iovec *iov = pdu->in_iov; > > > > + > > > > + for (i = 0; i < iov_cnt && size; i++) { > > > > + if (offset >= iov[i].iov_len) { > > > > + offset -= iov[i].iov_len; > > > > + continue; > > > > + } else { > > > > + len = MIN(iov[i].iov_len - offset, size); > > > > + memcpy(iov[i].iov_base + offset, data, len); > > > > + size -= len; > > > > + data += len; > > > > + offset = 0; > > > > + copied += len; > > > > + } > > > > + } > > > > + pdu->write_offset += copied; > > > > +} > > > > + > > > > > > Just wondering here, can we use pipes (read/write/vmsplice) instead of a > > > scatter-gather list? > > > > Can you elaborate on this. The function is to encode/decode data out of > > protocol buffer. > > I was just thinking about storing the data in a pipe instead of a > scatter-gather list. We are building that scatter-gather list from vring. I am not clear how to push that to pipe ? ie we have iovec in vring and how do i store that to a pipe ? > > This would turn virtio_p9_pdu_read() into a simple wrapper for read() > from a pipe and virtio_p9_pdu_write() into a wrapper for write(). > > Something like this: > > void virtio_9p__init(struct kvm *kvm, const char *root, const char > *tag_name) > { > [...] > pipe(p9_pipe); > [...] > } > > static void virtio_p9_pdu_read(struct p9_pdu *pdu, void *data, size_t > size) > { > read(p9_pipe[0], data, size); > } > > static void virtio_p9_pdu_write(struct p9_pdu *pdu, const void *data, > size_t size) > { > write(p9_pipe[1], data, size); > } > But why ? what we currently have is a simple memcpy, why replace that with syscall read/write ? -aneesh