fuse_loop_cfg_convert: restore v1 max_idle_threads semantics

Commit 30a126c5 ("API update for fuse_loop_config additions") introduced
fuse_loop_cfg_convert() to translate legacy fuse_loop_config_v1 structs
to the new v2 API, but only converted max_idle_threads — it did not set
max_threads.

In the v1 API, max_idle_threads was the effective pool cap: the thread
pool would grow on demand and threads exceeding max_idle_threads were
destroyed after processing each request.  A filesystem that configured
max_idle_threads=100 expected up to 100 threads to be available.

After the conversion, max_threads retains its default of 10, silently
capping the pool at 10 workers regardless of max_idle_threads.  A busy
filesystem with all 10 threads processing long-running requests has no
thread available to receive the next kernel request, causing it to hang.

Fix fuse_loop_cfg_convert() to call fuse_loop_cfg_set_max_threads() with
the same value as max_idle_threads before calling fuse_loop_cfg_set_idle_threads(),
preserving the original v1 semantics.

Also add warnings to both setters when max_idle_threads and max_threads
are set in an inconsistent order (max_idle > max_threads).  Reorder the
two setter calls in helper.c so that max_threads is set before
max_idle_threads, preventing a spurious warning when both are supplied as
consistent command-line options.  Add a unit test to document and verify
the correct behaviour.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>

lib/fuse_loop_mt.c

@@ -505,6 +505,14 @@ int fuse_loop_cfg_verify(const struct fuse_loop_config *config)
 void fuse_loop_cfg_convert(struct fuse_loop_config *config,
 			   const struct fuse_loop_config_v1 *v1_conf)
 {
+	/*
+	 * In the v1 API, max_idle_threads was the effective pool cap —
+	 * threads beyond max_idle_threads were destroyed after each request.
+	 * Set max_threads to the same value to preserve that behaviour.
+	 * Set max_threads first so the subsequent set_idle_threads call
+	 * does not trigger the max_idle > max_threads warning.
+	 */
+	fuse_loop_cfg_set_max_threads(config, v1_conf->max_idle_threads);
 	fuse_loop_cfg_set_idle_threads(config, v1_conf->max_idle_threads);
 
 	fuse_loop_cfg_set_clone_fd(config, v1_conf->clone_fd);
@@ -521,12 +529,31 @@ void fuse_loop_cfg_set_idle_threads(struct fuse_loop_config *config,
 				 FUSE_LOOP_MT_MAX_THREADS);
 		return;
 	}
+	/*
+	 * Warn if max_idle_threads > max_threads —
+	 * idle thread reaping would never trigger since the pool can never
+	 * exceed max_threads.
+	 */
+	if (value > 0 && value > config->max_threads)
+		fuse_log(FUSE_LOG_WARNING,
+			 "fuse: max_idle_threads %u is greater than max_threads %u\n",
+			 value, config->max_threads);
 	config->max_idle_threads = value;
 }
 
 void fuse_loop_cfg_set_max_threads(struct fuse_loop_config *config,
 				   unsigned int value)
 {
+	/*
+	 * Warn if max_threads < max_idle_threads —
+	 * idle thread reaping would never trigger since the pool can never
+	 * exceed max_threads.
+	 */
+	if (config->max_idle_threads > 0 &&
+	    value < (unsigned int)config->max_idle_threads)
+		fuse_log(FUSE_LOG_WARNING,
+			 "fuse: max_threads %u is less than max_idle_threads %d\n",
+			 value, config->max_idle_threads);
 	config->max_threads = value;
 }
 

lib/helper.c

@@ -389,8 +389,8 @@ int fuse_main_real_versioned(int argc, char *argv[],
 
 		fuse_loop_cfg_set_clone_fd(loop_config, opts.clone_fd);
 
-		fuse_loop_cfg_set_idle_threads(loop_config, opts.max_idle_threads);
 		fuse_loop_cfg_set_max_threads(loop_config, opts.max_threads);
+		fuse_loop_cfg_set_idle_threads(loop_config, opts.max_idle_threads);
 		res = fuse_loop_mt(fuse, loop_config);
 	}
 	if (res)

test/meson.build

@@ -28,6 +28,11 @@ td += executable('test_teardown_watchdog', 'test_teardown_watchdog.c',
 td += executable('test_abi', 'test_abi.c',
                  dependencies: [ libfuse_dep ],
                  install: false)
+td += executable('test_loop_config', 'test_loop_config.c',
+                 include_directories: include_dirs,
+                 link_with: [ libfuse ],
+                 dependencies: thread_dep,
+                 install: false)
 
 test_scripts = [ 'conftest.py', 'pytest.ini', 'test_examples.py',
                  'util.py', 'test_ctests.py', 'test_custom_io.py' ]

test/test_ctests.py

@@ -24,6 +24,10 @@ def test_abi():
     cmdline = [ pjoin(basename, 'test', 'test_abi') ]
     subprocess.check_call(cmdline)
 
+def test_loop_config():
+    """Unit test for fuse_loop_cfg setter interaction — no FUSE mount needed."""
+    subprocess.check_call([ pjoin(basename, 'test', 'test_loop_config') ])
+
 @pytest.mark.skipif('FUSE_CAP_WRITEBACK_CACHE' not in fuse_caps,
                     reason='not supported by running kernel')
 @pytest.mark.parametrize("writeback", (False, True))

test/test_loop_config.c

@@ -0,0 +1,288 @@
+/*
+ * Unit tests for fuse_loop_cfg_set_idle_threads() /
+ * fuse_loop_cfg_set_max_threads() interaction.
+ *
+ * No FUSE mount is needed; the tests exercise the setter logic directly.
+ */
+
+#define FUSE_USE_VERSION FUSE_MAKE_VERSION(3, 12)
+
+#include "fuse_i.h"          /* internal struct fuse_loop_config (v2) */
+#include "fuse_lowlevel.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+
+/* ------------------------------------------------------------------ */
+/* Log capture                                                         */
+/* ------------------------------------------------------------------ */
+
+static char last_log_msg[512];
+static enum fuse_log_level last_log_level;
+
+static void test_log_handler(enum fuse_log_level level, const char *fmt,
+			     va_list ap)
+{
+	last_log_level = level;
+	vsnprintf(last_log_msg, sizeof(last_log_msg), fmt, ap);
+}
+
+static void clear_log(void)
+{
+	last_log_msg[0] = '\0';
+	last_log_level = FUSE_LOG_DEBUG;
+}
+
+/* ------------------------------------------------------------------ */
+/* Helpers                                                             */
+/* ------------------------------------------------------------------ */
+
+#define PASS(name) do { printf("PASS: %s\n", name); } while (0)
+#define FAIL(name, fmt, ...) \
+	do { fprintf(stderr, "FAIL: %s — " fmt "\n", name, ##__VA_ARGS__); \
+	     exit(1); } while (0)
+
+#define CHECK(name, cond) \
+	do { if (!(cond)) FAIL(name, "assertion failed: %s", #cond); } while (0)
+
+/* ------------------------------------------------------------------ */
+/* Tests                                                               */
+/* ------------------------------------------------------------------ */
+
+/*
+ * Normal case: set_max_threads with no idle constraint → value accepted as-is.
+ */
+static void test_max_threads_normal(void)
+{
+	const char *name = "max_threads_normal";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+
+	clear_log();
+	fuse_loop_cfg_set_max_threads(cfg, 20);
+
+	CHECK(name, cfg->max_threads == 20);
+	CHECK(name, last_log_msg[0] == '\0'); /* no warning */
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * idle disabled (default -1): set_max_threads to any value is accepted.
+ */
+static void test_max_threads_idle_disabled(void)
+{
+	const char *name = "max_threads_idle_disabled";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+
+	/* default max_idle_threads == -1 (disabled) */
+	clear_log();
+	fuse_loop_cfg_set_max_threads(cfg, 5);
+
+	CHECK(name, cfg->max_threads == 5);
+	CHECK(name, last_log_msg[0] == '\0'); /* no warning */
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * idle == 0 (disabled): set_max_threads to a small value is accepted.
+ */
+static void test_max_threads_idle_zero(void)
+{
+	const char *name = "max_threads_idle_zero";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+
+	fuse_loop_cfg_set_idle_threads(cfg, 0);
+	clear_log();
+	fuse_loop_cfg_set_max_threads(cfg, 5);
+
+	CHECK(name, cfg->max_threads == 5);
+	CHECK(name, last_log_msg[0] == '\0'); /* no warning */
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * Warning: set_idle(100) then set_max(5) → value is set but a warning is logged.
+ * The resulting config is technically invalid (idle reaping can never trigger)
+ * but we honour the caller's explicit request.
+ */
+static void test_warn_max_below_idle(void)
+{
+	const char *name = "warn_max_below_idle";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+
+	fuse_loop_cfg_set_idle_threads(cfg, 100);
+	clear_log();
+	fuse_loop_cfg_set_max_threads(cfg, 5);
+
+	/* value is set as requested */
+	CHECK(name, cfg->max_threads == 5);
+	CHECK(name, cfg->max_idle_threads == 100);
+	/* a warning must have been logged */
+	CHECK(name, last_log_level == FUSE_LOG_WARNING);
+	CHECK(name, strstr(last_log_msg, "max_threads") != NULL);
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * set_idle(100) then set_max(100) → exact equality is valid, no warning.
+ */
+static void test_max_equals_idle(void)
+{
+	const char *name = "max_equals_idle";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+
+	fuse_loop_cfg_set_idle_threads(cfg, 100);
+	clear_log();
+	fuse_loop_cfg_set_max_threads(cfg, 100);
+
+	CHECK(name, cfg->max_threads == 100);
+	CHECK(name, last_log_msg[0] == '\0'); /* no warning */
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * set_idle(100) then set_max(200) → valid, no warning.
+ */
+static void test_max_above_idle(void)
+{
+	const char *name = "max_above_idle";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+
+	fuse_loop_cfg_set_idle_threads(cfg, 100);
+	clear_log();
+	fuse_loop_cfg_set_max_threads(cfg, 200);
+
+	CHECK(name, cfg->max_threads == 200);
+	CHECK(name, cfg->max_idle_threads == 100);
+	CHECK(name, last_log_msg[0] == '\0'); /* no warning */
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * helper.c order: set_max(200) first, then set_idle(100).
+ * Neither setter should warn when called in this order.
+ */
+static void test_helper_order_no_warning(void)
+{
+	const char *name = "helper_order_no_warning";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+
+	clear_log();
+	fuse_loop_cfg_set_max_threads(cfg, 200);
+	fuse_loop_cfg_set_idle_threads(cfg, 100);
+
+	CHECK(name, cfg->max_threads == 200);
+	CHECK(name, cfg->max_idle_threads == 100);
+	CHECK(name, last_log_msg[0] == '\0');
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * Reverse order: set_max(10) first, then set_idle(100).
+ * Value is set but a warning is logged.
+ */
+static void test_warn_idle_above_max(void)
+{
+	const char *name = "warn_idle_above_max";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+
+	fuse_loop_cfg_set_max_threads(cfg, 10);
+	clear_log();
+	fuse_loop_cfg_set_idle_threads(cfg, 100);
+
+	CHECK(name, cfg->max_threads == 10);
+	CHECK(name, cfg->max_idle_threads == 100);
+	CHECK(name, last_log_level == FUSE_LOG_WARNING);
+	CHECK(name, strstr(last_log_msg, "max_idle_threads") != NULL);
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * v1 legacy API: fuse_loop_cfg_convert with max_idle_threads=100.
+ * In v1 semantics, max_idle_threads WAS the effective pool cap.
+ * convert() must set max_threads = max_idle_threads to preserve that.
+ */
+static void test_v1_convert_sets_max_threads(void)
+{
+	const char *name = "v1_convert_sets_max_threads";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+	struct fuse_loop_config_v1 v1 = {
+		.max_idle_threads = 100,
+		.clone_fd = 0,
+	};
+
+	clear_log();
+	fuse_loop_cfg_convert(cfg, &v1);
+
+	/* both must be 100 — v1 max_idle_threads was the effective cap */
+	CHECK(name, cfg->max_idle_threads == 100);
+	CHECK(name, cfg->max_threads == 100);
+	/* convert sets max_threads first, so no ordering warning */
+	CHECK(name, last_log_msg[0] == '\0');
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/*
+ * v1 legacy API: default max_idle_threads=10 (old libfuse default).
+ * convert() → max_idle=10, max_threads=10.
+ */
+static void test_v1_convert_default(void)
+{
+	const char *name = "v1_convert_default";
+	struct fuse_loop_config *cfg = fuse_loop_cfg_create();
+	struct fuse_loop_config_v1 v1 = {
+		.max_idle_threads = 10,  /* old libfuse default */
+		.clone_fd = 0,
+	};
+
+	clear_log();
+	fuse_loop_cfg_convert(cfg, &v1);
+
+	CHECK(name, cfg->max_idle_threads == 10);
+	CHECK(name, cfg->max_threads == 10);
+	CHECK(name, last_log_msg[0] == '\0');
+
+	fuse_loop_cfg_destroy(cfg);
+	PASS(name);
+}
+
+/* ------------------------------------------------------------------ */
+/* main                                                                */
+/* ------------------------------------------------------------------ */
+
+int main(void)
+{
+	fuse_set_log_func(test_log_handler);
+
+	test_max_threads_normal();
+	test_max_threads_idle_disabled();
+	test_max_threads_idle_zero();
+	test_warn_max_below_idle();
+	test_max_equals_idle();
+	test_max_above_idle();
+	test_helper_order_no_warning();
+	test_warn_idle_above_max();
+	test_v1_convert_sets_max_threads();
+	test_v1_convert_default();
+
+	printf("All loop_config tests passed.\n");
+	return 0;
+}