[PULL 1/2] 9pfs: log warning if msize <= 8192

[PULL 1/2] 9pfs: log warning if msize <= 8192

[Christian Schoenebeck]

It is essential to choose a reasonable high value for 'msize' to avoid
severely degraded file I/O performance. This parameter can only be
chosen on client/guest side, and a Linux client defaults to an 'msize'
of only 8192 if the user did not explicitly specify a value for 'msize',
which results in very poor file I/O performance.

Unfortunately many users are not aware that they should specify an
appropriate value for 'msize' to avoid severe performance issues, so
log a performance warning (with a QEMU wiki link explaining this issue
in detail) on host side in that case to make it more clear.

Currently a client cannot automatically pick a reasonable value for
'msize', because a good value for 'msize' depends on the file I/O
potential of the underlying storage on host side, i.e. a feature
invisible to the client, and even then a user would still need to trade
off between performance profit and additional RAM costs, i.e. with
growing 'msize' (RAM occupation), performance still increases, but
performance delta will shrink continuously.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <e6fc84845c95816ad5baecb0abd6bfefdcf7ec9f.1599144062.git.qemu_oss@crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 7bb994bbf2..99b6f24fd6 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1353,6 +1353,15 @@ static void coroutine_fn v9fs_version(void *opaque)
         goto out;
     }
 
+    /* 8192 is the default msize of Linux clients */
+    if (s->msize <= 8192) {
+        warn_report_once(
+            "9p: degraded performance: a reasonable high msize should be "
+            "chosen on client/guest side (chosen msize is <= 8192). See "
+            "https://wiki.qemu.org/Documentation/9psetup#msize for details."
+        );
+    }
+
 marshal:
     err = pdu_marshal(pdu, offset, "ds", s->msize, &version);
     if (err < 0) {
-- 
2.20.1

[PULL 2/2] 9pfs: disable msize warning for synth driver

[Christian Schoenebeck]

Previous patch introduced a performance warning being logged on host
side if client connected with an 'msize' <= 8192. Disable this
performance warning for the synth driver to prevent that warning from
being printed whenever the 9pfs (qtest) test cases are running.

Introduce a new export flag V9FS_NO_PERF_WARN for that purpose, which
might also be used to disable such warnings from the CLI in future.

We could have also prevented the warning by simply raising P9_MAX_SIZE
in virtio-9p-test.c to any value larger than 8192, however in the
context of test cases it makes sense running for edge cases, which
includes the lowest 'msize' value supported by the server which is
4096, hence we want to preserve an msize of 4096 for the test client.

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <E1kEyDy-0006nN-5A@lizzy.crudebyte.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 fsdev/file-op-9p.h | 4 ++++
 hw/9pfs/9p-synth.c | 2 ++
 hw/9pfs/9p.c       | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index f2f7772c86..d51cec2f3b 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -64,6 +64,10 @@ typedef struct ExtendedOps {
  */
 #define V9FS_REMAP_INODES           0x00000200
 #define V9FS_FORBID_MULTIDEVS       0x00000400
+/*
+ * Disables certain performance warnings from being logged on host side.
+ */
+#define V9FS_NO_PERF_WARN           0x00000800
 
 #define V9FS_SEC_MASK               0x0000003C
 
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index 7eb210ffa8..cec8c0eefc 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -541,6 +541,8 @@ static int synth_init(FsContext *ctx, Error **errp)
     QLIST_INIT(&synth_root.child);
     qemu_mutex_init(&synth_mutex);
 
+    ctx->export_flags |= V9FS_NO_PERF_WARN;
+
     /* Add "." and ".." entries for root */
     v9fs_add_dir_node(&synth_root, synth_root.attr->mode,
                       "..", synth_root.attr, synth_root.attr->inode);
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 99b6f24fd6..741d222c3f 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1354,7 +1354,7 @@ static void coroutine_fn v9fs_version(void *opaque)
     }
 
     /* 8192 is the default msize of Linux clients */
-    if (s->msize <= 8192) {
+    if (s->msize <= 8192 && !(s->ctx.export_flags & V9FS_NO_PERF_WARN)) {
         warn_report_once(
             "9p: degraded performance: a reasonable high msize should be "
             "chosen on client/guest side (chosen msize is <= 8192). See "
-- 
2.20.1

[PULL 0/2] 9p queue

[Christian Schoenebeck]

The following changes since commit 2d2c73d0e3d504a61f868e46e6abd5643f38091b:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200914-1' into staging (2020-09-14 16:03:08 +0100)

are available in the Git repository at:

  https://github.com/cschoenebeck/qemu.git tags/pull-9p-20200915

for you to fetch changes up to c418f935ac6aed9aa1dc0ceb4b19044a9b7b7968:

  9pfs: disable msize warning for synth driver (2020-09-15 12:12:03 +0200)

----------------------------------------------------------------
The intention of the following two patches is making users aware about
the negative file I/O performance impact when using a very low value
for 9P client parameter 'msize', which especially is the case if no
'msize' parameter was supplied by the user with a 9P Linux client at all.

All it does is logging a performance warning on host side (once) in
that case. By setting 'msize' on client side to any value larger than
8192 the performance warning will disappear.

See https://wiki.qemu.org/Documentation/9psetup#msize for details.

----------------------------------------------------------------
Christian Schoenebeck (2):
      9pfs: log warning if msize <= 8192
      9pfs: disable msize warning for synth driver

 fsdev/file-op-9p.h | 4 ++++
 hw/9pfs/9p-synth.c | 2 ++
 hw/9pfs/9p.c       | 9 +++++++++
 3 files changed, 15 insertions(+)

Re: [PULL 0/2] 9p queue

[Peter Maydell]

On Wed, 16 Sep 2020 at 10:20, Christian Schoenebeck
<qemu_oss@crudebyte.com> wrote:
>
> The following changes since commit 2d2c73d0e3d504a61f868e46e6abd5643f38091b:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200914-1' into staging (2020-09-14 16:03:08 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/cschoenebeck/qemu.git tags/pull-9p-20200915
>
> for you to fetch changes up to c418f935ac6aed9aa1dc0ceb4b19044a9b7b7968:
>
>   9pfs: disable msize warning for synth driver (2020-09-15 12:12:03 +0200)
>
> ----------------------------------------------------------------
> The intention of the following two patches is making users aware about
> the negative file I/O performance impact when using a very low value
> for 9P client parameter 'msize', which especially is the case if no
> 'msize' parameter was supplied by the user with a 9P Linux client at all.
>
> All it does is logging a performance warning on host side (once) in
> that case. By setting 'msize' on client side to any value larger than
> 8192 the performance warning will disappear.
>
> See https://wiki.qemu.org/Documentation/9psetup#msize for details.
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM