From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37632) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TbTIl-0006Jl-6v for qemu-devel@nongnu.org; Thu, 22 Nov 2012 04:42:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TbTIf-0005i8-9s for qemu-devel@nongnu.org; Thu, 22 Nov 2012 04:42:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:28815) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TbTIf-0005hp-2u for qemu-devel@nongnu.org; Thu, 22 Nov 2012 04:42:25 -0500 Date: Thu, 22 Nov 2012 11:45:07 +0200 From: "Michael S. Tsirkin" Message-ID: <20121122094507.GA24617@redhat.com> References: <1353522781-12721-1-git-send-email-stefanha@redhat.com> <1353522781-12721-10-git-send-email-stefanha@redhat.com> <50ADF195.4030204@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <50ADF195.4030204@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 09/12] iov: add iov_get_ptr() to reference vector data List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , Anthony Liguori , dkoch@cloudswitch.com, qemu-devel@nongnu.org, Michael Roth , Blue Swirl , khoa@us.ibm.com, Stefan Hajnoczi , Asias He On Thu, Nov 22, 2012 at 10:34:13AM +0100, Paolo Bonzini wrote: > Il 21/11/2012 19:32, Stefan Hajnoczi ha scritto: > > The iov_get_ptr() data returns a pointer to contiguous data within a > > vector. This allows the caller to manipulate data inside the vector > > without copying in/out using iov_from_buf()/iov_to_buf() when we know > > that data is contiguous within an iovec element. > > This works for you because you have a single byte to write. It would > not work for the SG_IO inhdr, which would need iov_to_buf(). > > What about the following alternative API: > > void *iov_get_ptr(struct iovec *iov, unsigned int iov_cnt, > ssize_t offset, size_t *bytes); > > which would place the number of valid bytes (i.e. the length of the > remainder of the iovec entry) in *bytes? > > Also, I think that offset == iov_size(iov, iov_cnt) should be > acceptable, and it would be the only case in which *bytes == 0. > > Otherwise looks good. > > Paolo All this looks suspiciously like premature optimization to me. Do we have data to show avoiding header copy is a win? The caller would have to handle the case where the header is not contigious, which seems just too complex - in practice the code seems to simply fail in this case. Why not just copy the header using iov_from_buf()/iov_to_buf() and be done with it? > > Signed-off-by: Stefan Hajnoczi > > --- > > iov.c | 25 +++++++++++++++++++++++++ > > iov.h | 11 +++++++++++ > > 2 files changed, 36 insertions(+) > > > > diff --git a/iov.c b/iov.c > > index 6eed089..bc78c34 100644 > > --- a/iov.c > > +++ b/iov.c > > @@ -395,3 +395,28 @@ size_t iov_discard(struct iovec **iov, unsigned int *iov_cnt, ssize_t bytes) > > } > > return total; > > } > > + > > +void *iov_get_ptr(struct iovec *iov, unsigned int iov_cnt, > > + ssize_t offset, size_t bytes) > > +{ > > + if (offset < 0) { > > + offset += iov_size(iov, iov_cnt); > > + if (offset < 0) { > > + return NULL; /* offset before beginning of vector */ > > + } > > + } > > + > > + while (iov_cnt > 0) { > > + if (offset < iov->iov_len) { > > + if (bytes > iov->iov_len - offset) { > > + return NULL; /* would span iovec elements */ > > + } > > + return iov->iov_base + offset; > > + } > > + > > + offset -= iov->iov_len; > > + iov_cnt--; > > + iov++; > > + } > > + return NULL; /* offset beyond end of vector */ > > +} > > diff --git a/iov.h b/iov.h > > index d6d1fa6..674dd51 100644 > > --- a/iov.h > > +++ b/iov.h > > @@ -108,3 +108,14 @@ unsigned iov_copy(struct iovec *dst_iov, unsigned int dst_iov_cnt, > > * smaller than requested if the vector is too small. > > */ > > size_t iov_discard(struct iovec **iov, unsigned int *iov_cnt, ssize_t bytes); > > + > > +/* > > + * Get a pointer into a vector at offset if the given number of bytes is > > + * contiguous and not split across iovec elements. NULL is returned if > > + * memory would span iovec elements or exceed the length of the vector. > > + * > > + * The offset is ssize_t so that an offset from the end of the vector can > > + * be specified with a negative number. > > + */ > > +void *iov_get_ptr(struct iovec *iov, unsigned int iov_cnt, ssize_t offset, > > + size_t bytes); > >