From 1c528cd1cf380ef92d5e4aee0f6697ab17b6fac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Tue, 13 Oct 2020 18:14:41 +0200 Subject: [PATCH 1/2] packaging: Apply virtiofs performance related fixes to 5.x MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vivek Goyal found out that using "shared" thread pool, instead of "exclusive" results in better performance. Knowning that and with the plan to have virtio-fs as the default fs for the 2.0, let's bring this patch in for both 5.0 and 5.1. Fixes: #944 Signed-off-by: Fabiano Fidêncio --- ...rtiofsd-Used-glib-shared-thread-pool.patch | 60 +++++++++++++++++++ ...rtiofsd-Used-glib-shared-thread-pool.patch | 60 +++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100644 tools/packaging/qemu/patches/5.0.x/0003-virtiofsd-Used-glib-shared-thread-pool.patch create mode 100644 tools/packaging/qemu/patches/5.1.x/0006-virtiofsd-Used-glib-shared-thread-pool.patch diff --git a/tools/packaging/qemu/patches/5.0.x/0003-virtiofsd-Used-glib-shared-thread-pool.patch b/tools/packaging/qemu/patches/5.0.x/0003-virtiofsd-Used-glib-shared-thread-pool.patch new file mode 100644 index 0000000000..2e8113b8e7 --- /dev/null +++ b/tools/packaging/qemu/patches/5.0.x/0003-virtiofsd-Used-glib-shared-thread-pool.patch @@ -0,0 +1,60 @@ +From 04d325e86f79bd61f8fd50d45ff795aca0dd3404 Mon Sep 17 00:00:00 2001 +From: Vivek Goyal +Date: Mon, 21 Sep 2020 17:32:16 -0400 +Subject: [PATCH] virtiofsd: Used glib "shared" thread pool + +glib offers thread pools and it seems to support "exclusive" and "shared" +thread pools. + +https://developer.gnome.org/glib/stable/glib-Thread-Pools.html#g-thread-pool-new + +Currently we use "exlusive" thread pools but its performance seems to be +poor. I tried using "shared" thread pools and performance seems much +better. I posted performance results here. + +https://www.redhat.com/archives/virtio-fs/2020-September/msg00080.html + +So lets switch to shared thread pools. We can think of making it optional +once somebody can show in what cases exclusive thread pools offer better +results. For now, my simple performance tests across the board see +better results with shared thread pools. + +Signed-off-by: Vivek Goyal +Message-Id: <20200921213216.GE13362@redhat.com> +Reviewed-by: Stefan Hajnoczi +Signed-off-by: Dr. David Alan Gilbert + With seccomp fix from Miklos +--- + tools/virtiofsd/fuse_virtio.c | 2 +- + tools/virtiofsd/seccomp.c | 2 ++ + 2 files changed, 3 insertions(+), 1 deletion(-) + +diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c +index 9e5537506c..d5c8e98253 100644 +--- a/tools/virtiofsd/fuse_virtio.c ++++ b/tools/virtiofsd/fuse_virtio.c +@@ -588,7 +588,7 @@ static void *fv_queue_thread(void *opaque) + struct fuse_session *se = qi->virtio_dev->se; + GThreadPool *pool; + +- pool = g_thread_pool_new(fv_queue_worker, qi, se->thread_pool_size, TRUE, ++ pool = g_thread_pool_new(fv_queue_worker, qi, se->thread_pool_size, FALSE, + NULL); + if (!pool) { + fuse_log(FUSE_LOG_ERR, "%s: g_thread_pool_new failed\n", __func__); +diff --git a/tools/virtiofsd/seccomp.c b/tools/virtiofsd/seccomp.c +index 19fee60011..eb9af8265f 100644 +--- a/tools/virtiofsd/seccomp.c ++++ b/tools/virtiofsd/seccomp.c +@@ -93,6 +93,8 @@ static const int syscall_whitelist[] = { + SCMP_SYS(rt_sigaction), + SCMP_SYS(rt_sigprocmask), + SCMP_SYS(rt_sigreturn), ++ SCMP_SYS(sched_getattr), ++ SCMP_SYS(sched_setattr), + SCMP_SYS(sendmsg), + SCMP_SYS(setresgid), + SCMP_SYS(setresuid), +-- +2.28.0 + diff --git a/tools/packaging/qemu/patches/5.1.x/0006-virtiofsd-Used-glib-shared-thread-pool.patch b/tools/packaging/qemu/patches/5.1.x/0006-virtiofsd-Used-glib-shared-thread-pool.patch new file mode 100644 index 0000000000..2e8113b8e7 --- /dev/null +++ b/tools/packaging/qemu/patches/5.1.x/0006-virtiofsd-Used-glib-shared-thread-pool.patch @@ -0,0 +1,60 @@ +From 04d325e86f79bd61f8fd50d45ff795aca0dd3404 Mon Sep 17 00:00:00 2001 +From: Vivek Goyal +Date: Mon, 21 Sep 2020 17:32:16 -0400 +Subject: [PATCH] virtiofsd: Used glib "shared" thread pool + +glib offers thread pools and it seems to support "exclusive" and "shared" +thread pools. + +https://developer.gnome.org/glib/stable/glib-Thread-Pools.html#g-thread-pool-new + +Currently we use "exlusive" thread pools but its performance seems to be +poor. I tried using "shared" thread pools and performance seems much +better. I posted performance results here. + +https://www.redhat.com/archives/virtio-fs/2020-September/msg00080.html + +So lets switch to shared thread pools. We can think of making it optional +once somebody can show in what cases exclusive thread pools offer better +results. For now, my simple performance tests across the board see +better results with shared thread pools. + +Signed-off-by: Vivek Goyal +Message-Id: <20200921213216.GE13362@redhat.com> +Reviewed-by: Stefan Hajnoczi +Signed-off-by: Dr. David Alan Gilbert + With seccomp fix from Miklos +--- + tools/virtiofsd/fuse_virtio.c | 2 +- + tools/virtiofsd/seccomp.c | 2 ++ + 2 files changed, 3 insertions(+), 1 deletion(-) + +diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c +index 9e5537506c..d5c8e98253 100644 +--- a/tools/virtiofsd/fuse_virtio.c ++++ b/tools/virtiofsd/fuse_virtio.c +@@ -588,7 +588,7 @@ static void *fv_queue_thread(void *opaque) + struct fuse_session *se = qi->virtio_dev->se; + GThreadPool *pool; + +- pool = g_thread_pool_new(fv_queue_worker, qi, se->thread_pool_size, TRUE, ++ pool = g_thread_pool_new(fv_queue_worker, qi, se->thread_pool_size, FALSE, + NULL); + if (!pool) { + fuse_log(FUSE_LOG_ERR, "%s: g_thread_pool_new failed\n", __func__); +diff --git a/tools/virtiofsd/seccomp.c b/tools/virtiofsd/seccomp.c +index 19fee60011..eb9af8265f 100644 +--- a/tools/virtiofsd/seccomp.c ++++ b/tools/virtiofsd/seccomp.c +@@ -93,6 +93,8 @@ static const int syscall_whitelist[] = { + SCMP_SYS(rt_sigaction), + SCMP_SYS(rt_sigprocmask), + SCMP_SYS(rt_sigreturn), ++ SCMP_SYS(sched_getattr), ++ SCMP_SYS(sched_setattr), + SCMP_SYS(sendmsg), + SCMP_SYS(setresgid), + SCMP_SYS(setresuid), +-- +2.28.0 + From 1a9515a998875b623686cc6fb347491523f871bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= Date: Tue, 13 Oct 2020 18:19:40 +0200 Subject: [PATCH 2/2] runtime: Pass `--thread-pool-size=1` to virtiofsd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dave Gilbert brough up that passing --thread-pool-size=1 to virtiofsd may result in a performance improvement especially when using `cache=none`. While our current default is `cache=auto`, Dave mentioned that he seems no harm in having it set and he also mentiond that it may use a lot less stack space on aarch/arm. Fixes: #943 Signed-off-by: Fabiano Fidêncio --- src/runtime/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/Makefile b/src/runtime/Makefile index 2a6730d4ef..352280b393 100644 --- a/src/runtime/Makefile +++ b/src/runtime/Makefile @@ -181,7 +181,7 @@ DEFVIRTIOFSCACHE ?= auto # # see `virtiofsd -h` for possible options. # Make sure you quote args. -DEFVIRTIOFSEXTRAARGS ?= [] +DEFVIRTIOFSEXTRAARGS ?= [\"--thread-pool-size=1\"] DEFENABLEIOTHREADS := false DEFENABLEMEMPREALLOC := false DEFENABLEHUGEPAGES := false