diff mbox series

[039/104] virtiofsd: Plumb fuse_bufvec through to do_write_buf

Message ID 20191212163904.159893-40-dgilbert@redhat.com
State New
Headers show
Series virtiofs daemon [all] | expand

Commit Message

Dr. David Alan Gilbert Dec. 12, 2019, 4:37 p.m. UTC
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Let fuse_session_process_buf_int take a fuse_bufvec * instead of a
fuse_buf;  and then through to do_write_buf - where in the best
case it can pass that straight through to op.write_buf without copying
(other than skipping a header).

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tools/virtiofsd/fuse_i.h        |  2 +-
 tools/virtiofsd/fuse_lowlevel.c | 61 ++++++++++++++++++++++-----------
 tools/virtiofsd/fuse_virtio.c   |  3 +-
 3 files changed, 44 insertions(+), 22 deletions(-)

Comments

Masayoshi Mizuma Jan. 17, 2020, 9:01 p.m. UTC | #1
On Thu, Dec 12, 2019 at 04:37:59PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Let fuse_session_process_buf_int take a fuse_bufvec * instead of a
> fuse_buf;  and then through to do_write_buf - where in the best
> case it can pass that straight through to op.write_buf without copying
> (other than skipping a header).
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tools/virtiofsd/fuse_i.h        |  2 +-
>  tools/virtiofsd/fuse_lowlevel.c | 61 ++++++++++++++++++++++-----------
>  tools/virtiofsd/fuse_virtio.c   |  3 +-
>  3 files changed, 44 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
> index cb1ca70ffa..d0679508cd 100644
> --- a/tools/virtiofsd/fuse_i.h
> +++ b/tools/virtiofsd/fuse_i.h
> @@ -119,7 +119,7 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov,
>  void fuse_free_req(fuse_req_t req);
>  
>  void fuse_session_process_buf_int(struct fuse_session *se,
> -                                  const struct fuse_buf *buf,
> +                                  struct fuse_bufvec *bufv,
>                                    struct fuse_chan *ch);
>  
>  
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index bea092b454..7d33bbf539 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -1006,11 +1006,12 @@ static void do_write(fuse_req_t req, fuse_ino_t nodeid, const void *inarg)
>  }
>  
>  static void do_write_buf(fuse_req_t req, fuse_ino_t nodeid, const void *inarg,
> -                         const struct fuse_buf *ibuf)
> +                         struct fuse_bufvec *ibufv)
>  {
>      struct fuse_session *se = req->se;
> -    struct fuse_bufvec bufv = {
> -        .buf[0] = *ibuf,
> +    struct fuse_bufvec *pbufv = ibufv;
> +    struct fuse_bufvec tmpbufv = {
> +        .buf[0] = ibufv->buf[0],
>          .count = 1,
>      };
>      struct fuse_write_in *arg = (struct fuse_write_in *)inarg;
> @@ -1020,22 +1021,31 @@ static void do_write_buf(fuse_req_t req, fuse_ino_t nodeid, const void *inarg,
>      fi.fh = arg->fh;
>      fi.writepage = arg->write_flags & FUSE_WRITE_CACHE;
>  
> -    fi.lock_owner = arg->lock_owner;
> -    fi.flags = arg->flags;
> -    if (!(bufv.buf[0].flags & FUSE_BUF_IS_FD)) {
> -        bufv.buf[0].mem = PARAM(arg);
> -    }
> -
> -    bufv.buf[0].size -=
> -        sizeof(struct fuse_in_header) + sizeof(struct fuse_write_in);
> -    if (bufv.buf[0].size < arg->size) {
> -        fuse_log(FUSE_LOG_ERR, "fuse: do_write_buf: buffer size too small\n");
> -        fuse_reply_err(req, EIO);
> -        return;
> +    if (ibufv->count == 1) {
> +        fi.lock_owner = arg->lock_owner;
> +        fi.flags = arg->flags;
> +        if (!(tmpbufv.buf[0].flags & FUSE_BUF_IS_FD)) {
> +            tmpbufv.buf[0].mem = PARAM(arg);
> +        }
> +        tmpbufv.buf[0].size -=
> +            sizeof(struct fuse_in_header) + sizeof(struct fuse_write_in);
> +        if (tmpbufv.buf[0].size < arg->size) {
> +            fuse_log(FUSE_LOG_ERR,
> +                     "fuse: do_write_buf: buffer size too small\n");
> +            fuse_reply_err(req, EIO);
> +            return;
> +        }
> +        tmpbufv.buf[0].size = arg->size;
> +        pbufv = &tmpbufv;
> +    } else {
> +        /*
> +         *  Input bufv contains the headers in the first element
> +         * and the data in the rest, we need to skip that first element
> +         */
> +        ibufv->buf[0].size = 0;
>      }
> -    bufv.buf[0].size = arg->size;
>  
> -    se->op.write_buf(req, nodeid, &bufv, arg->offset, &fi);
> +    se->op.write_buf(req, nodeid, pbufv, arg->offset, &fi);
>  }
>  
>  static void do_flush(fuse_req_t req, fuse_ino_t nodeid, const void *inarg)
> @@ -2027,13 +2037,24 @@ static const char *opname(enum fuse_opcode opcode)
>  void fuse_session_process_buf(struct fuse_session *se,
>                                const struct fuse_buf *buf)
>  {
> -    fuse_session_process_buf_int(se, buf, NULL);
> +    struct fuse_bufvec bufv = { .buf[0] = *buf, .count = 1 };
> +    fuse_session_process_buf_int(se, &bufv, NULL);
>  }
>  
> +/*
> + * Restriction:
> + *   bufv is normally a single entry buffer, except for a write
> + *   where (if it's in memory) then the bufv may be multiple entries,
> + *   where the first entry contains all headers and subsequent entries
> + *   contain data
> + *   bufv shall not use any offsets etc to make the data anything
> + *   other than contiguous starting from 0.
> + */
>  void fuse_session_process_buf_int(struct fuse_session *se,
> -                                  const struct fuse_buf *buf,
> +                                  struct fuse_bufvec *bufv,
>                                    struct fuse_chan *ch)
>  {
> +    const struct fuse_buf *buf = bufv->buf;
>      struct fuse_in_header *in;
>      const void *inarg;
>      struct fuse_req *req;
> @@ -2111,7 +2132,7 @@ void fuse_session_process_buf_int(struct fuse_session *se,
>  
>      inarg = (void *)&in[1];
>      if (in->opcode == FUSE_WRITE && se->op.write_buf) {
> -        do_write_buf(req, in->nodeid, inarg, buf);
> +        do_write_buf(req, in->nodeid, inarg, bufv);
>      } else {
>          fuse_ll_ops[in->opcode].func(req, in->nodeid, inarg);
>      }
> diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> index fa6e53e7d0..99c877ea2e 100644
> --- a/tools/virtiofsd/fuse_virtio.c
> +++ b/tools/virtiofsd/fuse_virtio.c
> @@ -499,7 +499,8 @@ static void *fv_queue_thread(void *opaque)
>              /* TODO! Endianness of header */
>  
>              /* TODO: Add checks for fuse_session_exited */
> -            fuse_session_process_buf_int(se, &fbuf, &ch);
> +            struct fuse_bufvec bufv = { .buf[0] = fbuf, .count = 1 };
> +            fuse_session_process_buf_int(se, &bufv, &ch);
>  
>              if (!qi->reply_sent) {
>                  fuse_log(FUSE_LOG_DEBUG, "%s: elem %d no reply sent\n",

Looks good to me.

Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
diff mbox series

Patch

diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
index cb1ca70ffa..d0679508cd 100644
--- a/tools/virtiofsd/fuse_i.h
+++ b/tools/virtiofsd/fuse_i.h
@@ -119,7 +119,7 @@  int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov,
 void fuse_free_req(fuse_req_t req);
 
 void fuse_session_process_buf_int(struct fuse_session *se,
-                                  const struct fuse_buf *buf,
+                                  struct fuse_bufvec *bufv,
                                   struct fuse_chan *ch);
 
 
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index bea092b454..7d33bbf539 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -1006,11 +1006,12 @@  static void do_write(fuse_req_t req, fuse_ino_t nodeid, const void *inarg)
 }
 
 static void do_write_buf(fuse_req_t req, fuse_ino_t nodeid, const void *inarg,
-                         const struct fuse_buf *ibuf)
+                         struct fuse_bufvec *ibufv)
 {
     struct fuse_session *se = req->se;
-    struct fuse_bufvec bufv = {
-        .buf[0] = *ibuf,
+    struct fuse_bufvec *pbufv = ibufv;
+    struct fuse_bufvec tmpbufv = {
+        .buf[0] = ibufv->buf[0],
         .count = 1,
     };
     struct fuse_write_in *arg = (struct fuse_write_in *)inarg;
@@ -1020,22 +1021,31 @@  static void do_write_buf(fuse_req_t req, fuse_ino_t nodeid, const void *inarg,
     fi.fh = arg->fh;
     fi.writepage = arg->write_flags & FUSE_WRITE_CACHE;
 
-    fi.lock_owner = arg->lock_owner;
-    fi.flags = arg->flags;
-    if (!(bufv.buf[0].flags & FUSE_BUF_IS_FD)) {
-        bufv.buf[0].mem = PARAM(arg);
-    }
-
-    bufv.buf[0].size -=
-        sizeof(struct fuse_in_header) + sizeof(struct fuse_write_in);
-    if (bufv.buf[0].size < arg->size) {
-        fuse_log(FUSE_LOG_ERR, "fuse: do_write_buf: buffer size too small\n");
-        fuse_reply_err(req, EIO);
-        return;
+    if (ibufv->count == 1) {
+        fi.lock_owner = arg->lock_owner;
+        fi.flags = arg->flags;
+        if (!(tmpbufv.buf[0].flags & FUSE_BUF_IS_FD)) {
+            tmpbufv.buf[0].mem = PARAM(arg);
+        }
+        tmpbufv.buf[0].size -=
+            sizeof(struct fuse_in_header) + sizeof(struct fuse_write_in);
+        if (tmpbufv.buf[0].size < arg->size) {
+            fuse_log(FUSE_LOG_ERR,
+                     "fuse: do_write_buf: buffer size too small\n");
+            fuse_reply_err(req, EIO);
+            return;
+        }
+        tmpbufv.buf[0].size = arg->size;
+        pbufv = &tmpbufv;
+    } else {
+        /*
+         *  Input bufv contains the headers in the first element
+         * and the data in the rest, we need to skip that first element
+         */
+        ibufv->buf[0].size = 0;
     }
-    bufv.buf[0].size = arg->size;
 
-    se->op.write_buf(req, nodeid, &bufv, arg->offset, &fi);
+    se->op.write_buf(req, nodeid, pbufv, arg->offset, &fi);
 }
 
 static void do_flush(fuse_req_t req, fuse_ino_t nodeid, const void *inarg)
@@ -2027,13 +2037,24 @@  static const char *opname(enum fuse_opcode opcode)
 void fuse_session_process_buf(struct fuse_session *se,
                               const struct fuse_buf *buf)
 {
-    fuse_session_process_buf_int(se, buf, NULL);
+    struct fuse_bufvec bufv = { .buf[0] = *buf, .count = 1 };
+    fuse_session_process_buf_int(se, &bufv, NULL);
 }
 
+/*
+ * Restriction:
+ *   bufv is normally a single entry buffer, except for a write
+ *   where (if it's in memory) then the bufv may be multiple entries,
+ *   where the first entry contains all headers and subsequent entries
+ *   contain data
+ *   bufv shall not use any offsets etc to make the data anything
+ *   other than contiguous starting from 0.
+ */
 void fuse_session_process_buf_int(struct fuse_session *se,
-                                  const struct fuse_buf *buf,
+                                  struct fuse_bufvec *bufv,
                                   struct fuse_chan *ch)
 {
+    const struct fuse_buf *buf = bufv->buf;
     struct fuse_in_header *in;
     const void *inarg;
     struct fuse_req *req;
@@ -2111,7 +2132,7 @@  void fuse_session_process_buf_int(struct fuse_session *se,
 
     inarg = (void *)&in[1];
     if (in->opcode == FUSE_WRITE && se->op.write_buf) {
-        do_write_buf(req, in->nodeid, inarg, buf);
+        do_write_buf(req, in->nodeid, inarg, bufv);
     } else {
         fuse_ll_ops[in->opcode].func(req, in->nodeid, inarg);
     }
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index fa6e53e7d0..99c877ea2e 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -499,7 +499,8 @@  static void *fv_queue_thread(void *opaque)
             /* TODO! Endianness of header */
 
             /* TODO: Add checks for fuse_session_exited */
-            fuse_session_process_buf_int(se, &fbuf, &ch);
+            struct fuse_bufvec bufv = { .buf[0] = fbuf, .count = 1 };
+            fuse_session_process_buf_int(se, &bufv, &ch);
 
             if (!qi->reply_sent) {
                 fuse_log(FUSE_LOG_DEBUG, "%s: elem %d no reply sent\n",