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 <stefa...@redhat.com>
> > ---
> > 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);
> >
