fuse-loop/fuse_do_work: Avoid lots of thread creations/destructions

On benchmarking metadata operations with a single threaded bonnie++
and "max_idle_threads" limited to 1, 'top' was showing suspicious
160% cpu usage.
Profiling the system with flame graphs showed that an astonishing
amount of CPU time was spent in thread creation and destruction.

After verifying the code it turned out that fuse_do_work() was
creating a new thread every time all existing idle threads
were already busy. And then just a few lines later after processing
the current request it noticed that it had created too many threads
and destructed the current thread. I.e. there was a thread
creation/destruction ping-pong.

Code is changed to only create new threads if the max number of
threads is not reached.

Furthermore, thread destruction is disabled, as creation/destruction
is expensive in general.

With this change cpu usage of passthrough_hp went from ~160% to
~80% (with different values of max_idle_threads). And bonnie
values got approximately faster by 90%. This is a with single
threaded bonnie++
bonnie++ -x 4 -q -s0  -d <path> -n 30:1:1:10 -r 0

Without this patch, using the default max_idle_threads=10 and just
a single bonnie++ the thread creation/destruction code path is not
triggered.  Just one libfuse and one application thread is just
a corner case - the requirement for the issue was just
n-application-threads >= max_idle_threads.

Signed-off-by: Bernd Schubert <bschubert@ddn.com>

diff --git a/example/passthrough_hp.cc b/example/passthrough_hp.cc
index 5c61928d719af1fbd9657e820bcb7a43fd5511b5..e86da5d30fefa2c8c2891fd20b28d460c286755f 100644 (file)
--- a/example/passthrough_hp.cc
+++ b/example/passthrough_hp.cc
@@ -43,7 +43,7 @@
  * \include passthrough_hp.cc
  */
 
-#define FUSE_USE_VERSION 35
+#define FUSE_USE_VERSION FUSE_MAKE_VERSION(3, 12)
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -1228,6 +1228,8 @@ static void maximize_fd_limit() {
 
 int main(int argc, char *argv[]) {
 
+    struct fuse_loop_config *loop_config = NULL;
+
     // Parse command line options
     auto options {parse_options(argc, argv)};
 
@@ -1274,15 +1276,15 @@ int main(int argc, char *argv[]) {
     umask(0);
 
     // Mount and run main loop
-    struct fuse_loop_config loop_config;
-    loop_config.clone_fd = 0;
-    loop_config.max_idle_threads = 10;
+    loop_config = fuse_loop_cfg_create();
+
     if (fuse_session_mount(se, argv[2]) != 0)
         goto err_out3;
     if (options.count("single"))
         ret = fuse_session_loop(se);
     else
-        ret = fuse_session_loop_mt(se, &loop_config);
+        ret = fuse_session_loop_mt(se, loop_config);
+
 
     fuse_session_unmount(se);
 
@@ -1291,6 +1293,8 @@ err_out3:
 err_out2:
     fuse_session_destroy(se);
 err_out1:
+
+    fuse_loop_cfg_destroy(loop_config);
     fuse_opt_free_args(&args);
 
     return ret ? 1 : 0;

diff --git a/include/fuse_common.h b/include/fuse_common.h
index b40814fdf16e57207b3d0ce3706e15ae74f7b354..e9d874556d838900aa35f918317dbe4858cfadee 100644 (file)
--- a/include/fuse_common.h
+++ b/include/fuse_common.h
@@ -857,13 +857,19 @@ struct fuse_loop_config *fuse_loop_cfg_create(void);
 void fuse_loop_cfg_destroy(struct fuse_loop_config *config);
 
 /**
- * fuse_loop_config2 setter to set the number of max idle threads.
+ * fuse_loop_config setter to set the number of max idle threads.
  */
 void fuse_loop_cfg_set_idle_threads(struct fuse_loop_config *config,
                                    unsigned int value);
 
 /**
- * fuse_loop_config2 setter to enable the clone_fd feature
+ * fuse_loop_config setter to set the number of max threads.
+ */
+void fuse_loop_cfg_set_max_threads(struct fuse_loop_config *config,
+                                  unsigned int value);
+
+/**
+ * fuse_loop_config setter to enable the clone_fd feature
  */
 void fuse_loop_cfg_set_clone_fd(struct fuse_loop_config *config,
                                unsigned int value);

diff --git a/include/fuse_lowlevel.h b/include/fuse_lowlevel.h
index 378742dfeadde71770b431988d4638b905c59090..53f0fcf4bd914695172cae099e7f43477760d428 100644 (file)
--- a/include/fuse_lowlevel.h
+++ b/include/fuse_lowlevel.h
@@ -1868,6 +1868,11 @@ void fuse_cmdline_help(void);
  * Filesystem setup & teardown                                 *
  * ----------------------------------------------------------- */
 
+/**
+ * Note: Any addition to this struct needs to create a compatibility symbol
+ *       for fuse_parse_cmdline(). For ABI compatibility reasons it is also
+ *       not possible to remove struct members.
+ */
 struct fuse_cmdline_opts {
        int singlethread;
        int foreground;
@@ -1877,7 +1882,11 @@ struct fuse_cmdline_opts {
        int show_version;
        int show_help;
        int clone_fd;
-       unsigned int max_idle_threads;
+       unsigned int max_idle_threads; /* discouraged, due to thread
+                                       * destruct overhead */
+
+       /* Added in libfuse-3.12 */
+       unsigned int max_threads;
 };
 
 /**
@@ -1898,8 +1907,20 @@ struct fuse_cmdline_opts {
  * @param opts output argument for parsed options
  * @return 0 on success, -1 on failure
  */
+#if (!defined(__UCLIBC__) && !defined(__APPLE__))
 int fuse_parse_cmdline(struct fuse_args *args,
                       struct fuse_cmdline_opts *opts);
+#else
+#if FUSE_USE_VERSION < FUSE_MAKE_VERSION(3, 12)
+int fuse_parse_cmdline_30(struct fuse_args *args,
+                          struct fuse_cmdline_opts *opts);
+#define fuse_parse_cmdline(args, opts) fuse_parse_cmdline_30(args, opts)
+#else
+int fuse_parse_cmdline_312(struct fuse_args *args,
+                          struct fuse_cmdline_opts *opts);
+#define fuse_parse_cmdline(args, opts) fuse_parse_cmdline_312(args, opts)
+#endif
+#endif
 
 /**
  * Create a low level session.

diff --git a/lib/fuse_i.h b/lib/fuse_i.h
index 6930a204206f06e8fea94310e8e665ce2f58ceff..42f0e5f276c19caf10246dbef61ad00982bbb10a 100644 (file)
--- a/lib/fuse_i.h
+++ b/lib/fuse_i.h
@@ -128,6 +128,13 @@ struct fuse_loop_config
         * The special value of -1 means that this parameter is disabled.
         */
        int max_idle_threads;
+
+       /**
+        *  max number of threads taking and processing kernel requests
+        *
+        *  As of now threads are created dynamically
+        */
+       unsigned int max_threads;
 };
 #endif
 

diff --git a/lib/fuse_loop_mt.c b/lib/fuse_loop_mt.c
index 2f0470b3ec8fbe3614a424f92e74077c869d7e2b..9ec1fb275eab7ee769c7eec21c2c5c7efe334586 100644 (file)
--- a/lib/fuse_loop_mt.c
+++ b/lib/fuse_loop_mt.c
@@ -31,6 +31,7 @@
 
 #define FUSE_LOOP_MT_V2_IDENTIFIER      INT_MAX - 2
 #define FUSE_LOOP_MT_DEF_CLONE_FD       0
+#define FUSE_LOOP_MT_DEF_MAX_THREADS 10
 #define FUSE_LOOP_MT_DEF_IDLE_THREADS -1 /* thread destruction is disabled
                                           * by default */
 
@@ -57,6 +58,7 @@ struct fuse_mt {
        int error;
        int clone_fd;
        int max_idle;
+       int max_threads;
 };
 
 static struct fuse_chan *fuse_chan_new(int fd)
@@ -161,7 +163,7 @@ static void *fuse_do_work(void *data)
 
                if (!isforget)
                        mt->numavail--;
-               if (mt->numavail == 0)
+               if (mt->numavail == 0 && mt->numworker < mt->max_threads)
                        fuse_loop_start_thread(mt);
                pthread_mutex_unlock(&mt->lock);
 
@@ -170,7 +172,14 @@ static void *fuse_do_work(void *data)
                pthread_mutex_lock(&mt->lock);
                if (!isforget)
                        mt->numavail++;
-               if (mt->numavail > mt->max_idle) {
+
+               /* creating and destroying threads is rather expensive - and there is
+                * not much gain from destroying existing threads. It is therefore
+                * discouraged to set max_idle to anything else than -1. If there
+                * is indeed a good reason to destruct threads it should be done
+                * delayed, a moving average might be useful for that.
+                */
+               if (mt->max_idle != -1 && mt->numavail > mt->max_idle) {
                        if (mt->exit) {
                                pthread_mutex_unlock(&mt->lock);
                                return NULL;
@@ -202,7 +211,10 @@ int fuse_start_thread(pthread_t *thread_id, void *(*func)(void *), void *arg)
        pthread_attr_t attr;
        char *stack_size;
 
-       /* Override default stack size */
+       /* Override default stack size
+        * XXX: This should ideally be a parameter option. It is rather
+        *      well hidden here.
+        */
        pthread_attr_init(&attr);
        stack_size = getenv(ENVNAME_THREAD_STACK);
        if (stack_size && pthread_attr_setstacksize(&attr, atoi(stack_size)))
@@ -328,6 +340,7 @@ int err;
        mt.numworker = 0;
        mt.numavail = 0;
        mt.max_idle = config->max_idle_threads;
+       mt.max_threads = config->max_threads;
        mt.main.thread_id = pthread_self();
        mt.main.prev = mt.main.next = &mt.main;
        sem_init(&mt.finish, 0, 0);
@@ -400,6 +413,7 @@ struct fuse_loop_config *fuse_loop_cfg_create(void)
 
        config->version_id       = FUSE_LOOP_MT_V2_IDENTIFIER;
        config->max_idle_threads = FUSE_LOOP_MT_DEF_IDLE_THREADS;
+       config->max_threads      = FUSE_LOOP_MT_DEF_MAX_THREADS;
        config->clone_fd         = FUSE_LOOP_MT_DEF_CLONE_FD;
 
        return config;
@@ -432,6 +446,12 @@ void fuse_loop_cfg_set_idle_threads(struct fuse_loop_config *config,
        config->max_idle_threads = value;
 }
 
+void fuse_loop_cfg_set_max_threads(struct fuse_loop_config *config,
+                                  unsigned int value)
+{
+       config->max_threads = value;
+}
+
 void fuse_loop_cfg_set_clone_fd(struct fuse_loop_config *config,
                                unsigned int value)
 {

diff --git a/lib/fuse_versionscript b/lib/fuse_versionscript
index aff7e84f696c6c32d75f68d9f0c6721ba509d67f..7e50e75078017072a8fe18708aa861b1cfbe0730 100644 (file)
--- a/lib/fuse_versionscript
+++ b/lib/fuse_versionscript
@@ -178,8 +178,12 @@ FUSE_3.12 {
                fuse_loop_cfg_create;
                fuse_loop_cfg_destroy;
                fuse_loop_cfg_set_idle_threads;
+               fuse_loop_cfg_set_max_threads;
                fuse_loop_cfg_set_clone_fd;
                fuse_loop_cfg_convert;
+               fuse_parse_cmdline;
+               fuse_parse_cmdline_30;
+               fuse_parse_cmdline_312;
 } FUSE_3.4;
 
 # Local Variables:

diff --git a/lib/helper.c b/lib/helper.c
index fc6a6ee1eb9cd5790163b1b3f6106727d554bb4a..ea5f87d1eb90288a1ee227dc93b4b22a33275cfe 100644 (file)
--- a/lib/helper.c
+++ b/lib/helper.c
@@ -50,6 +50,7 @@ static const struct fuse_opt fuse_helper_opts[] = {
 #endif
        FUSE_HELPER_OPT("clone_fd",     clone_fd),
        FUSE_HELPER_OPT("max_idle_threads=%u", max_idle_threads),
+       FUSE_HELPER_OPT("max_threads=%u", max_threads),
        FUSE_OPT_END
 };
 
@@ -136,6 +137,8 @@ void fuse_cmdline_help(void)
               "    -o clone_fd            use separate fuse device fd for each thread\n"
               "                           (may improve performance)\n"
               "    -o max_idle_threads    the maximum number of idle worker threads\n"
+              "                           allowed (default: -1)\n"
+              "    -o max_threads         the maximum number of worker threads\n"
               "                           allowed (default: 10)\n");
 }
 
@@ -199,12 +202,16 @@ static int add_default_subtype(const char *progname, struct fuse_args *args)
        return res;
 }
 
-int fuse_parse_cmdline(struct fuse_args *args,
-                      struct fuse_cmdline_opts *opts)
+int fuse_parse_cmdline_312(struct fuse_args *args,
+                          struct fuse_cmdline_opts *opts);
+FUSE_SYMVER("fuse_parse_cmdline_312", "fuse_parse_cmdline@@FUSE_3.12")
+int fuse_parse_cmdline_312(struct fuse_args *args,
+                          struct fuse_cmdline_opts *opts)
 {
        memset(opts, 0, sizeof(struct fuse_cmdline_opts));
 
-       opts->max_idle_threads = 10;
+       opts->max_idle_threads = -1; /* new default in fuse version 3.12 */
+       opts->max_threads = 10;
 
        if (fuse_opt_parse(args, opts, fuse_helper_opts,
                           fuse_helper_opt_proc) == -1)
@@ -221,6 +228,40 @@ int fuse_parse_cmdline(struct fuse_args *args,
        return 0;
 }
 
+/**
+ * struct fuse_cmdline_opts got extended in libfuse-3.12
+ */
+int fuse_parse_cmdline_30(struct fuse_args *args,
+                      struct fuse_cmdline_opts *opts);
+FUSE_SYMVER("fuse_parse_cmdline_37", "fuse_parse_cmdline@FUSE_3.0")
+int fuse_parse_cmdline_30(struct fuse_args *args,
+                         struct fuse_cmdline_opts *out_opts)
+{
+       struct fuse_cmdline_opts opts;
+
+
+       int rc = fuse_parse_cmdline_312(args, &opts);
+       if (rc == 0) {
+               /* copy up to the size of the old pre 3.12 struct */
+               memcpy(out_opts, &opts,
+                      offsetof(struct fuse_cmdline_opts, max_idle_threads) +
+                      sizeof(opts.max_idle_threads));
+       }
+
+       return rc;
+}
+
+/**
+ * Compatibility ABI symbol for systems that do not support version symboling
+ */
+#if (defined(__UCLIBC__) || defined(__APPLE__))
+int fuse_parse_cmdline(struct fuse_args *args,
+                      struct fuse_cmdline_opts *opts)
+{
+       return fuse_parse_cmdline_30(args, out_opts);
+}
+#endif
+
 
 int fuse_daemonize(int foreground)
 {