[Qemu-devel] [PATCH] qemu:virtio-9p: Send iounit to client for read/write operations

[Qemu-devel] [PATCH] qemu:virtio-9p: Send iounit to client for read/write operations

[M. Mohan Kumar]

Compute iounit based on the host filesystem block size and pass it to
client with open/create response. Also return iounit as statfs's f_bsize
for optimal block size transfers.


Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
Reviewd-by: Sripathi Kodi <sripathik@in.ibm.com>
---
 hw/virtio-9p.c |   69 +++++++++++++++++++++++++++++++++++++++++++++----------
 hw/virtio-9p.h |    7 +++++
 2 files changed, 63 insertions(+), 13 deletions(-)

This patch depends on the patch
http://lists.nongnu.org/archive/html/qemu-devel/2010-05/msg02706.html
 
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 851004d..c60dcc8 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -258,6 +258,11 @@ static int v9fs_do_fsync(V9fsState *s, int fd)
     return s->ops->fsync(&s->ctx, fd);
 }
 
+static int v9fs_do_statfs(V9fsState *s, V9fsString *path, struct statfs *stbuf)
+{
+    return s->ops->statfs(&s->ctx, path->data, stbuf);
+}
+
 static void v9fs_string_init(V9fsString *str)
 {
     str->data = NULL;
@@ -1067,11 +1072,10 @@ static void v9fs_fix_path(V9fsString *dst, V9fsString *src, int len)
 
 static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
 {
-    int32_t msize;
     V9fsString version;
     size_t offset = 7;
 
-    pdu_unmarshal(pdu, offset, "ds", &msize, &version);
+    pdu_unmarshal(pdu, offset, "ds", &s->msize, &version);
 
     if (!strcmp(version.data, "9P2000.u")) {
         s->proto_version = V9FS_PROTO_2000U;
@@ -1081,7 +1085,7 @@ static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
         v9fs_string_sprintf(&version, "unknown");
     }
 
-    offset += pdu_marshal(pdu, offset, "ds", msize, &version);
+    offset += pdu_marshal(pdu, offset, "ds", s->msize, &version);
     complete_pdu(s, pdu, offset);
 
     v9fs_string_free(&version);
@@ -1515,6 +1519,26 @@ out:
     v9fs_walk_complete(s, vs, err);
 }
 
+static int32_t get_iounit(V9fsState *s, V9fsString *name)
+{
+    struct statfs stbuf;
+    int32_t iounit = 0;
+
+    /*
+     * iounit should be multiples of f_bsize (host filesystem block size
+     * and as well as less than (client msize - P9_IOHDRSZ))
+     */
+    if (!v9fs_do_statfs(s, name, &stbuf)) {
+        iounit = stbuf.f_bsize;
+        iounit *= (s->msize - P9_IOHDRSZ)/stbuf.f_bsize;
+    }
+
+    if (!iounit) {
+        iounit = s->msize - P9_IOHDRSZ;
+    }
+    return iounit;
+}
+
 static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err)
 {
     if (vs->fidp->dir == NULL) {
@@ -1532,12 +1556,15 @@ out:
 
 static void v9fs_open_post_open(V9fsState *s, V9fsOpenState *vs, int err)
 {
+    int32_t iounit;
+
     if (vs->fidp->fd == -1) {
         err = -errno;
         goto out;
     }
 
-    vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, 0);
+    iounit = get_iounit(s, &vs->fidp->path);
+    vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, iounit);
     err = vs->offset;
 out:
     complete_pdu(s, vs->pdu, err);
@@ -2011,11 +2038,16 @@ out:
 
 static void v9fs_post_create(V9fsState *s, V9fsCreateState *vs, int err)
 {
+    int32_t iounit;
+
+    iounit = get_iounit(s, &vs->fidp->path);
+
     if (err == 0) {
         v9fs_string_copy(&vs->fidp->path, &vs->fullname);
         stat_to_qid(&vs->stbuf, &vs->qid);
 
-        vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, 0);
+        vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid,
+                iounit);
 
         err = vs->offset;
     }
@@ -2654,23 +2686,34 @@ out:
     qemu_free(vs);
 }
 
-static int v9fs_do_statfs(V9fsState *s, V9fsString *path, struct statfs *stbuf)
-{
-    return s->ops->statfs(&s->ctx, path->data, stbuf);
-}
-
 static void v9fs_statfs_post_statfs(V9fsState *s, V9fsStatfsState *vs, int err)
 {
+    int32_t bsize_factor;
+
     if (err) {
         err = -errno;
         goto out;
     }
 
+    /*
+     * compute bsize factor based on host file system block size
+     * and client msize
+     */
+    bsize_factor = (s->msize - P9_IOHDRSZ)/vs->stbuf.f_bsize;
+    if (!bsize_factor) {
+        bsize_factor = 1;
+    }
     vs->v9statfs.f_type = vs->stbuf.f_type;
     vs->v9statfs.f_bsize = vs->stbuf.f_bsize;
-    vs->v9statfs.f_blocks = vs->stbuf.f_blocks;
-    vs->v9statfs.f_bfree = vs->stbuf.f_bfree;
-    vs->v9statfs.f_bavail = vs->stbuf.f_bavail;
+    vs->v9statfs.f_bsize *= bsize_factor;
+    /*
+     * f_bsize is adjusted(multiplied) by bsize factor, so we need to
+     * adjust(divide) the number of blocks, free blocks and available
+     * blocks by bsize factor
+     */
+    vs->v9statfs.f_blocks = vs->stbuf.f_blocks/bsize_factor;
+    vs->v9statfs.f_bfree = vs->stbuf.f_bfree/bsize_factor;
+    vs->v9statfs.f_bavail = vs->stbuf.f_bavail/bsize_factor;
     vs->v9statfs.f_files = vs->stbuf.f_files;
     vs->v9statfs.f_ffree = vs->stbuf.f_ffree;
     vs->v9statfs.fsid_val = (unsigned int) vs->stbuf.f_fsid.__val[0] |
diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h
index 3819be5..92bbeb0 100644
--- a/hw/virtio-9p.h
+++ b/hw/virtio-9p.h
@@ -84,6 +84,12 @@ enum p9_proto_version {
 #define P9_NOFID    (u32)(~0)
 #define P9_MAXWELEM 16
 
+/*
+ * ample room for Twrite/Rread header
+ * size[4] Tread/Twrite tag[2] fid[4] offset[8] count[4]
+ */
+#define P9_IOHDRSZ 24
+
 typedef struct V9fsPDU V9fsPDU;
 
 struct V9fsPDU
@@ -168,6 +174,7 @@ typedef struct V9fsState
     uint8_t *tag;
     size_t config_size;
     enum p9_proto_version proto_version;
+    int32_t msize;
 } V9fsState;
 
 typedef struct V9fsCreateState {
-- 
1.6.6.1

[Qemu-devel] [PATCH] 9p: Make use of iounit for read/write

[M. Mohan Kumar]

Change the v9fs_file_readn function to limit the maximum transfer size
based on the iounit or msize.

Also remove the redundant check for limiting the transfer size in
v9fs_file_write. This check is done by p9_client_write.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 fs/9p/vfs_file.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index 25b300e..ae0147c 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -139,7 +139,7 @@ ssize_t
 v9fs_file_readn(struct file *filp, char *data, char __user *udata, u32 count,
 	       u64 offset)
 {
-	int n, total;
+	int n, total, size;
 	struct p9_fid *fid = filp->private_data;
 
 	P9_DPRINTK(P9_DEBUG_VFS, "fid %d offset %llu count %d\n", fid->fid,
@@ -147,6 +147,7 @@ v9fs_file_readn(struct file *filp, char *data, char __user *udata, u32 count,
 
 	n = 0;
 	total = 0;
+	size = fid->iounit ? fid->iounit : fid->clnt->msize - P9_IOHDRSZ;
 	do {
 		n = p9_client_read(fid, data, udata, offset, count);
 		if (n <= 0)
@@ -160,7 +161,7 @@ v9fs_file_readn(struct file *filp, char *data, char __user *udata, u32 count,
 		offset += n;
 		count -= n;
 		total += n;
-	} while (count > 0 && n == (fid->clnt->msize - P9_IOHDRSZ));
+	} while (count > 0 && n == size);
 
 	if (n < 0)
 		total = n;
@@ -183,11 +184,13 @@ v9fs_file_read(struct file *filp, char __user *udata, size_t count,
 {
 	int ret;
 	struct p9_fid *fid;
+	size_t size;
 
 	P9_DPRINTK(P9_DEBUG_VFS, "count %zu offset %lld\n", count, *offset);
 	fid = filp->private_data;
 
-	if (count > (fid->clnt->msize - P9_IOHDRSZ))
+	size = fid->iounit ? fid->iounit : fid->clnt->msize - P9_IOHDRSZ;
+	if (count > size)
 		ret = v9fs_file_readn(filp, NULL, udata, count, *offset);
 	else
 		ret = p9_client_read(fid, NULL, udata, *offset, count);
@@ -224,9 +227,7 @@ v9fs_file_write(struct file *filp, const char __user * data,
 	fid = filp->private_data;
 	clnt = fid->clnt;
 
-	rsize = fid->iounit;
-	if (!rsize || rsize > clnt->msize-P9_IOHDRSZ)
-		rsize = clnt->msize - P9_IOHDRSZ;
+	rsize = fid->iounit ? fid->iounit : clnt->msize - P9_IOHDRSZ;
 
 	do {
 		if (count < rsize)
-- 
1.6.6.1

[Qemu-devel] Re: [V9fs-developer] [PATCH] qemu:virtio-9p: Send iounit to client for read/write operations

[Venkateswararao Jujjuri (JV)]

M. Mohan Kumar wrote:
> Compute iounit based on the host filesystem block size and pass it to
> client with open/create response. Also return iounit as statfs's f_bsize
> for optimal block size transfers.
> 
> 
> Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
> Reviewd-by: Sripathi Kodi <sripathik@in.ibm.com>
> ---
>  hw/virtio-9p.c |   69 +++++++++++++++++++++++++++++++++++++++++++++----------
>  hw/virtio-9p.h |    7 +++++
>  2 files changed, 63 insertions(+), 13 deletions(-)
> 
> This patch depends on the patch
> http://lists.nongnu.org/archive/html/qemu-devel/2010-05/msg02706.html
> 
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 851004d..c60dcc8 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -258,6 +258,11 @@ static int v9fs_do_fsync(V9fsState *s, int fd)
>      return s->ops->fsync(&s->ctx, fd);
>  }
> 
> +static int v9fs_do_statfs(V9fsState *s, V9fsString *path, struct statfs *stbuf)
> +{
> +    return s->ops->statfs(&s->ctx, path->data, stbuf);
> +}
> +
>  static void v9fs_string_init(V9fsString *str)
>  {
>      str->data = NULL;
> @@ -1067,11 +1072,10 @@ static void v9fs_fix_path(V9fsString *dst, V9fsString *src, int len)
> 
>  static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
>  {
> -    int32_t msize;
>      V9fsString version;
>      size_t offset = 7;
> 
> -    pdu_unmarshal(pdu, offset, "ds", &msize, &version);
> +    pdu_unmarshal(pdu, offset, "ds", &s->msize, &version);
> 
>      if (!strcmp(version.data, "9P2000.u")) {
>          s->proto_version = V9FS_PROTO_2000U;
> @@ -1081,7 +1085,7 @@ static void v9fs_version(V9fsState *s, V9fsPDU *pdu)
>          v9fs_string_sprintf(&version, "unknown");
>      }
> 
> -    offset += pdu_marshal(pdu, offset, "ds", msize, &version);
> +    offset += pdu_marshal(pdu, offset, "ds", s->msize, &version);
>      complete_pdu(s, pdu, offset);
> 
>      v9fs_string_free(&version);
> @@ -1515,6 +1519,26 @@ out:
>      v9fs_walk_complete(s, vs, err);
>  }
> 
> +static int32_t get_iounit(V9fsState *s, V9fsString *name)
> +{
> +    struct statfs stbuf;
> +    int32_t iounit = 0;
> +
> +    /*
> +     * iounit should be multiples of f_bsize (host filesystem block size
> +     * and as well as less than (client msize - P9_IOHDRSZ))
> +     */
> +    if (!v9fs_do_statfs(s, name, &stbuf)) {
> +        iounit = stbuf.f_bsize;
> +        iounit *= (s->msize - P9_IOHDRSZ)/stbuf.f_bsize;
> +    }
> +
> +    if (!iounit) {
> +        iounit = s->msize - P9_IOHDRSZ;
> +    }
> +    return iounit;
> +}
> +
>  static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err)
>  {
>      if (vs->fidp->dir == NULL) {
> @@ -1532,12 +1556,15 @@ out:
> 
>  static void v9fs_open_post_open(V9fsState *s, V9fsOpenState *vs, int err)
>  {
> +    int32_t iounit;
> +
>      if (vs->fidp->fd == -1) {
>          err = -errno;
>          goto out;
>      }
> 
> -    vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, 0);
> +    iounit = get_iounit(s, &vs->fidp->path);
> +    vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, iounit);
>      err = vs->offset;
>  out:
>      complete_pdu(s, vs->pdu, err);
> @@ -2011,11 +2038,16 @@ out:
> 
>  static void v9fs_post_create(V9fsState *s, V9fsCreateState *vs, int err)
>  {
> +    int32_t iounit;
> +
> +    iounit = get_iounit(s, &vs->fidp->path);

Now get_iounit() does a statfs() system call..hence I think we need to introduce a
state/post function  here.

- JV

> +
>      if (err == 0) {
>          v9fs_string_copy(&vs->fidp->path, &vs->fullname);
>          stat_to_qid(&vs->stbuf, &vs->qid);
> 
> -        vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid, 0);
> +        vs->offset += pdu_marshal(vs->pdu, vs->offset, "Qd", &vs->qid,
> +                iounit);
> 
>          err = vs->offset;
>      }
> @@ -2654,23 +2686,34 @@ out:
>      qemu_free(vs);
>  }
> 
> -static int v9fs_do_statfs(V9fsState *s, V9fsString *path, struct statfs *stbuf)
> -{
> -    return s->ops->statfs(&s->ctx, path->data, stbuf);
> -}
> -
>  static void v9fs_statfs_post_statfs(V9fsState *s, V9fsStatfsState *vs, int err)
>  {
> +    int32_t bsize_factor;
> +
>      if (err) {
>          err = -errno;
>          goto out;
>      }
> 
> +    /*
> +     * compute bsize factor based on host file system block size
> +     * and client msize
> +     */
> +    bsize_factor = (s->msize - P9_IOHDRSZ)/vs->stbuf.f_bsize;
> +    if (!bsize_factor) {
> +        bsize_factor = 1;
> +    }
>      vs->v9statfs.f_type = vs->stbuf.f_type;
>      vs->v9statfs.f_bsize = vs->stbuf.f_bsize;
> -    vs->v9statfs.f_blocks = vs->stbuf.f_blocks;
> -    vs->v9statfs.f_bfree = vs->stbuf.f_bfree;
> -    vs->v9statfs.f_bavail = vs->stbuf.f_bavail;
> +    vs->v9statfs.f_bsize *= bsize_factor;
> +    /*
> +     * f_bsize is adjusted(multiplied) by bsize factor, so we need to
> +     * adjust(divide) the number of blocks, free blocks and available
> +     * blocks by bsize factor
> +     */
> +    vs->v9statfs.f_blocks = vs->stbuf.f_blocks/bsize_factor;
> +    vs->v9statfs.f_bfree = vs->stbuf.f_bfree/bsize_factor;
> +    vs->v9statfs.f_bavail = vs->stbuf.f_bavail/bsize_factor;
>      vs->v9statfs.f_files = vs->stbuf.f_files;
>      vs->v9statfs.f_ffree = vs->stbuf.f_ffree;
>      vs->v9statfs.fsid_val = (unsigned int) vs->stbuf.f_fsid.__val[0] |
> diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h
> index 3819be5..92bbeb0 100644
> --- a/hw/virtio-9p.h
> +++ b/hw/virtio-9p.h
> @@ -84,6 +84,12 @@ enum p9_proto_version {
>  #define P9_NOFID    (u32)(~0)
>  #define P9_MAXWELEM 16
> 
> +/*
> + * ample room for Twrite/Rread header
> + * size[4] Tread/Twrite tag[2] fid[4] offset[8] count[4]
> + */
> +#define P9_IOHDRSZ 24
> +
>  typedef struct V9fsPDU V9fsPDU;
> 
>  struct V9fsPDU
> @@ -168,6 +174,7 @@ typedef struct V9fsState
>      uint8_t *tag;
>      size_t config_size;
>      enum p9_proto_version proto_version;
> +    int32_t msize;
>  } V9fsState;
> 
>  typedef struct V9fsCreateState {