[master] da279c2ea Move def and units fields up in struct parspec

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Mar 2 14:09:09 UTC 2020


commit da279c2eaf4003737c022852244717886edc0c65
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Mar 2 11:36:03 2020 +0100

    Move def and units fields up in struct parspec
    
    It's a bit weird that the default value and the unit is separated from
    the min and max. This aligns struct parspec field order with the PARAM()
    macro for these two fields.
    
    In static parspec definitions, the flags field can now be omitted if
    there are no flags, and the parameter is not subject to dynamic bounds.

diff --git a/bin/varnishd/mgt/mgt_param.h b/bin/varnishd/mgt/mgt_param.h
index 98d47f77d..e3a68fbee 100644
--- a/bin/varnishd/mgt/mgt_param.h
+++ b/bin/varnishd/mgt/mgt_param.h
@@ -43,6 +43,8 @@ struct parspec {
 	volatile void	*priv;
 	const char	*min;
 	const char	*max;
+	const char	*def;
+	const char	*units;
 	const char	*descr;
 	int		 flags;
 #define DELAYED_EFFECT	(1<<0)
@@ -55,9 +57,6 @@ struct parspec {
 #define ONLY_ROOT	(1<<7)
 #define NOT_IMPLEMENTED	(1<<8)
 
-	const char	*def;
-	const char	*units;
-
 	const char	*dyn_min_reason;
 	const char	*dyn_max_reason;
 	char		*dyn_min;
diff --git a/bin/varnishd/mgt/mgt_param_bits.c b/bin/varnishd/mgt/mgt_param_bits.c
index 14adeba6d..cf39ee027 100644
--- a/bin/varnishd/mgt/mgt_param_bits.c
+++ b/bin/varnishd/mgt/mgt_param_bits.c
@@ -252,27 +252,32 @@ tweak_feature(struct vsb *vsb, const struct parspec *par, const char *arg)
  */
 
 struct parspec VSL_parspec[] = {
-	{ "vsl_mask", tweak_vsl_mask, NULL, NULL, NULL,
+	{ "vsl_mask", tweak_vsl_mask, NULL,
+		NULL, NULL, "default",
+		NULL,
 		"Mask individual VSL messages from being logged.\n"
 		"\tdefault\tSet default value\n"
 		"\nUse +/- prefix in front of VSL tag name to unmask/mask "
-		"individual VSL messages.",
-		0, "default", "" },
-	{ "debug", tweak_debug, NULL, NULL, NULL,
+		"individual VSL messages." },
+	{ "debug", tweak_debug, NULL,
+		NULL, NULL, "none",
+		NULL,
 		"Enable/Disable various kinds of debugging.\n"
 		"\tnone\tDisable all debugging\n\n"
 		"Use +/- prefix to set/reset individual bits:"
 #define DEBUG_BIT(U, l, d) "\n\t" #l "\t" d
 #include "tbl/debug_bits.h"
 #undef DEBUG_BIT
-		, 0, "none", "" },
-	{ "feature", tweak_feature, NULL, NULL, NULL,
+		},
+	{ "feature", tweak_feature, NULL,
+		NULL, NULL, "none",
+		NULL,
 		"Enable/Disable various minor features.\n"
 		"\tnone\tDisable all features.\n\n"
 		"Use +/- prefix to enable/disable individual feature:"
 #define FEATURE_BIT(U, l, d, ld) "\n\t" #l "\t" d
 #include "tbl/feature_bits.h"
 #undef FEATURE_BIT
-		, 0, "none", "" },
+		},
 	{ NULL, NULL, NULL }
 };
diff --git a/bin/varnishd/mgt/mgt_param_tbl.c b/bin/varnishd/mgt/mgt_param_tbl.c
index 09bff349a..0f5f087d0 100644
--- a/bin/varnishd/mgt/mgt_param_tbl.c
+++ b/bin/varnishd/mgt/mgt_param_tbl.c
@@ -46,66 +46,59 @@
 
 struct parspec mgt_parspec[] = {
 #define PARAM(nm, ty, mi, ma, de, un, fl, st, ...)			\
-	{ #nm, tweak_##ty, &mgt_param.nm, mi, ma, st, fl, de, un,	\
+	{ #nm, tweak_##ty, &mgt_param.nm, mi, ma, de, un, st, fl,	\
 	    __VA_ARGS__ },
 #include "tbl/params.h"
 
 	{ "cc_command", tweak_string, &mgt_cc_cmd,
-		NULL, NULL,
+		NULL, NULL, VCC_CC,
+		NULL,
 		"Command used for compiling the C source code to a "
 		"dlopen(3) loadable object.  Any occurrence of %s in "
 		"the string will be replaced with the source file name, "
 		"and %o will be replaced with the output file name.",
-		MUST_RELOAD,
-		VCC_CC , NULL },
+		MUST_RELOAD },
 	{ "vcl_path", tweak_string, &mgt_vcl_path,
-		NULL, NULL,
+		NULL, NULL, VARNISH_VCL_DIR,
+		NULL,
 		"Directory (or colon separated list of directories) "
 		"from which relative VCL filenames (vcl.load and "
 		"include) are to be found.  By default Varnish searches "
 		"VCL files in both the system configuration and shared "
 		"data directories to allow packages to drop their VCL "
 		"files in a standard location where relative includes "
-		"would work.",
-		0,
-		VARNISH_VCL_DIR,
-		NULL },
+		"would work." },
 	{ "vmod_path", tweak_string, &mgt_vmod_path,
-		NULL, NULL,
+		NULL, NULL, VARNISH_VMOD_DIR,
+		NULL,
 		"Directory (or colon separated list of directories) "
-		"where VMODs are to be found.",
-		0,
-		VARNISH_VMOD_DIR,
-		NULL },
+		"where VMODs are to be found." },
 	{ "vcc_err_unref", tweak_bool, &mgt_vcc_err_unref,
-		NULL, NULL,
-		"Unreferenced VCL objects result in error.",
-		0,
-		"on", "bool" },
+		NULL, NULL, "on",
+		"bool",
+		"Unreferenced VCL objects result in error." },
 	{ "vcc_allow_inline_c", tweak_bool, &mgt_vcc_allow_inline_c,
-		NULL, NULL,
-		"Allow inline C code in VCL.",
-		0,
-		"off", "bool" },
+		NULL, NULL, "off",
+		"bool",
+		"Allow inline C code in VCL." },
 	{ "vcc_unsafe_path", tweak_bool, &mgt_vcc_unsafe_path,
-		NULL, NULL,
+		NULL, NULL, "on",
+		"bool",
 		"Allow '/' in vmod & include paths.\n"
-		"Allow 'import ... from ...'.",
-		0,
-		"on", "bool" },
+		"Allow 'import ... from ...'." },
 	{ "pcre_match_limit", tweak_uint,
 		&mgt_param.vre_limits.match,
-		"1", NULL,
+		"1", NULL, "10000",
+		NULL,
 		"The limit for the number of calls to the internal match()"
 		" function in pcre_exec().\n\n"
 		"(See: PCRE_EXTRA_MATCH_LIMIT in pcre docs.)\n\n"
 		"This parameter limits how much CPU time"
-		" regular expression matching can soak up.",
-		0,
-		"10000", ""},
+		" regular expression matching can soak up." },
 	{ "pcre_match_limit_recursion", tweak_uint,
 		&mgt_param.vre_limits.match_recursion,
-		"1", NULL,
+		"1", NULL, "20",
+		NULL,
 		"The recursion depth-limit for the internal match() function"
 		" in a pcre_exec().\n\n"
 		"(See: PCRE_EXTRA_MATCH_LIMIT_RECURSION in pcre docs.)\n\n"
@@ -116,27 +109,22 @@ struct parspec mgt_parspec[] = {
 		" matching failures.\n\n"
 		"Matching failures will show up in the log as VCL_Error"
 		" messages with regexp errors -27 or -21.\n\n"
-		"Testcase r01576 can be useful when tuning this parameter.",
-		0,
-		"20", ""},
+		"Testcase r01576 can be useful when tuning this parameter." },
 	{ "pool_req", tweak_poolparam, &mgt_param.req_pool,
-		NULL, NULL,
+		NULL, NULL, "10,100,10",
+		NULL,
 		"Parameters for per worker pool request memory pool.\n\n"
-		MEMPOOL_TEXT,
-		0,
-		"10,100,10", ""},
+		MEMPOOL_TEXT },
 	{ "pool_sess", tweak_poolparam, &mgt_param.sess_pool,
-		NULL, NULL,
+		NULL, NULL, "10,100,10",
+		NULL,
 		"Parameters for per worker pool session memory pool.\n\n"
-		MEMPOOL_TEXT,
-		0,
-		"10,100,10", ""},
+		MEMPOOL_TEXT },
 	{ "pool_vbo", tweak_poolparam, &mgt_param.vbo_pool,
-		NULL, NULL,
+		NULL, NULL, "10,100,10",
+		NULL,
 		"Parameters for backend object fetch memory pool.\n\n"
-		MEMPOOL_TEXT,
-		0,
-		"10,100,10", ""},
+		MEMPOOL_TEXT },
 
 	{ NULL, NULL, NULL }
 };
diff --git a/bin/varnishd/mgt/mgt_pool.c b/bin/varnishd/mgt/mgt_pool.c
index dc8b847c8..4d30ca427 100644
--- a/bin/varnishd/mgt/mgt_pool.c
+++ b/bin/varnishd/mgt/mgt_pool.c
@@ -89,7 +89,8 @@ tweak_thread_pool_max(struct vsb *vsb, const struct parspec *par,
 
 struct parspec WRK_parspec[] = {
 	{ "thread_pools", tweak_uint, &mgt_param.wthread_pools,
-		"1", NULL,
+		"1", NULL, "2",
+		"pools",
 		"Number of worker thread pools.\n"
 		"\n"
 		"Increasing the number of worker pools decreases lock "
@@ -103,21 +104,20 @@ struct parspec WRK_parspec[] = {
 		"\n"
 		"Can be increased on the fly, but decreases require a "
 		"restart to take effect.",
-		EXPERIMENTAL | DELAYED_EFFECT,
-		"2", "pools" },
+		EXPERIMENTAL | DELAYED_EFFECT },
 	{ "thread_pool_max", tweak_thread_pool_max, &mgt_param.wthread_max,
-		NULL, NULL,
+		NULL, NULL, "5000",
+		"threads",
 		"The maximum number of worker threads in each pool.\n"
 		"\n"
 		"Do not set this higher than you have to, since excess "
 		"worker threads soak up RAM and CPU and generally just get "
 		"in the way of getting work done.",
 		DELAYED_EFFECT,
-		"5000", "threads",
 		"thread_pool_min" },
 	{ "thread_pool_min", tweak_thread_pool_min, &mgt_param.wthread_min,
-		"5", // TASK_QUEUE__END
-		NULL,
+		"5" /* TASK_QUEUE__END */, NULL, "100",
+		"threads",
 		"The minimum number of worker threads in each pool.\n"
 		"\n"
 		"Increasing this may help ramp up faster from low load "
@@ -127,11 +127,11 @@ struct parspec WRK_parspec[] = {
 		"but this parameter is strongly recommended to be "
 		"at least 10", // 2 * TASK_QUEUE__END
 		DELAYED_EFFECT,
-		"100", "threads",
 		NULL, "thread_pool_max" },
 	{ "thread_pool_reserve", tweak_uint,
 		&mgt_param.wthread_reserve,
-		NULL, NULL,
+		NULL, NULL, "0",
+		"threads",
 		"The number of worker threads reserved for vital tasks "
 		"in each pool.\n"
 		"\n"
@@ -147,37 +147,37 @@ struct parspec WRK_parspec[] = {
 		"Default is 0 to auto-tune (5% of thread_pool_min).\n"
 		"Minimum is 1 otherwise, maximum is 95% of thread_pool_min.",
 		DELAYED_EFFECT,
-		"0", "threads",
 		NULL, "95% of thread_pool_min" },
 	{ "thread_pool_timeout",
 		tweak_timeout, &mgt_param.wthread_timeout,
-		"10", NULL,
+		"10", NULL, "300",
+		"seconds",
 		"Thread idle threshold.\n"
 		"\n"
 		"Threads in excess of thread_pool_min, which have been idle "
 		"for at least this long, will be destroyed.",
-		EXPERIMENTAL | DELAYED_EFFECT,
-		"300", "seconds" },
+		EXPERIMENTAL | DELAYED_EFFECT },
 	{ "thread_pool_watchdog",
 		tweak_timeout, &mgt_param.wthread_watchdog,
-		"0.1", NULL,
+		"0.1", NULL, "60",
+		"seconds",
 		"Thread queue stuck watchdog.\n"
 		"\n"
 		"If no queued work have been released for this long,"
 		" the worker process panics itself.",
-		EXPERIMENTAL,
-		"60", "seconds" },
+		EXPERIMENTAL },
 	{ "thread_pool_destroy_delay",
 		tweak_timeout, &mgt_param.wthread_destroy_delay,
-		"0.01", NULL,
+		"0.01", NULL, "1",
+		"seconds",
 		"Wait this long after destroying a thread.\n"
 		"\n"
 		"This controls the decay of thread pools when idle(-ish).",
-		EXPERIMENTAL | DELAYED_EFFECT,
-		"1", "seconds" },
+		EXPERIMENTAL | DELAYED_EFFECT },
 	{ "thread_pool_add_delay",
 		tweak_timeout, &mgt_param.wthread_add_delay,
-		"0", NULL,
+		"0", NULL, "0",
+		"seconds",
 		"Wait at least this long after creating a thread.\n"
 		"\n"
 		"Some (buggy) systems may need a short (sub-second) "
@@ -186,11 +186,11 @@ struct parspec WRK_parspec[] = {
 		"'threads_failed' counter grow too much.\n"
 		"\n"
 		"Setting this too high results in insufficient worker threads.",
-		EXPERIMENTAL,
-		"0", "seconds" },
+		EXPERIMENTAL },
 	{ "thread_pool_fail_delay",
 		tweak_timeout, &mgt_param.wthread_fail_delay,
-		"10e-3", NULL,
+		"10e-3", NULL, "0.2",
+		"seconds",
 		"Wait at least this long after a failed thread creation "
 		"before trying to create another thread.\n"
 		"\n"
@@ -205,31 +205,31 @@ struct parspec WRK_parspec[] = {
 		"It may also help to increase thread_pool_timeout and "
 		"thread_pool_min, to reduce the rate at which treads are "
 		"destroyed and later recreated.",
-		EXPERIMENTAL,
-		"0.2", "seconds" },
+		EXPERIMENTAL },
 	{ "thread_stats_rate",
 		tweak_uint, &mgt_param.wthread_stats_rate,
-		"0", NULL,
+		"0", NULL, "10",
+		"requests",
 		"Worker threads accumulate statistics, and dump these into "
 		"the global stats counters if the lock is free when they "
 		"finish a job (request/fetch etc.)\n"
 		"This parameters defines the maximum number of jobs "
 		"a worker thread may handle, before it is forced to dump "
 		"its accumulated stats into the global counters.",
-		EXPERIMENTAL,
-		"10", "requests" },
+		EXPERIMENTAL },
 	{ "thread_queue_limit", tweak_uint, &mgt_param.wthread_queue_limit,
-		"0", NULL,
+		"0", NULL, "20",
+		NULL,
 		"Permitted request queue length per thread-pool.\n"
 		"\n"
 		"This sets the number of requests we will queue, waiting "
 		"for an available thread.  Above this limit sessions will "
 		"be dropped instead of queued.",
-		EXPERIMENTAL,
-		"20", "" },
+		EXPERIMENTAL },
 	{ "thread_pool_stack",
 		tweak_bytes, &mgt_param.wthread_stacksize,
-		NULL, NULL,
+		NULL, NULL, NULL,	// default set in mgt_main.c
+		"bytes",
 		"Worker thread stack size.\n"
 		"This will likely be rounded up to a multiple of 4k"
 		" (or whatever the page_size might be) by the kernel.\n"
@@ -264,7 +264,6 @@ struct parspec WRK_parspec[] = {
 		" overflow occurs. Setting it in 150%-200%"
 		" increments is recommended until stack overflows"
 		" cease to occur.",
-		DELAYED_EFFECT,
-		NULL, "bytes" },	// default set in mgt_main.c
+		DELAYED_EFFECT },
 	{ NULL, NULL, NULL }
 };


More information about the varnish-commit mailing list