[master] 9008a85 More parameter code polishing.
Poul-Henning Kamp
phk at varnish-cache.org
Tue Nov 12 19:11:44 CET 2013
commit 9008a85aedf337a8cebc7e61a85e4b8dfa8ebf01
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Tue Nov 12 18:11:31 2013 +0000
More parameter code polishing.
diff --git a/bin/varnishd/mgt/mgt.h b/bin/varnishd/mgt/mgt.h
index eb729de..a7ca900 100644
--- a/bin/varnishd/mgt/mgt.h
+++ b/bin/varnishd/mgt/mgt.h
@@ -75,6 +75,8 @@ const void *pick(const struct choice *cp, const char *which, const char *kind);
void MCF_InitParams(struct cli *);
void MCF_CollectParams(void);
void MCF_SetDefault(const char *param, const char *def);
+void MCF_SetMinimum(const char *param, const char *def);
+void MCF_SetMaximum(const char *param, const char *def);
void MCF_ParamSet(struct cli *, const char *param, const char *val);
void MCF_ParamProtect(struct cli *, const char *arg);
void MCF_DumpRstParam(void);
diff --git a/bin/varnishd/mgt/mgt_main.c b/bin/varnishd/mgt/mgt_main.c
index 820552f..d8299cc 100644
--- a/bin/varnishd/mgt/mgt_main.c
+++ b/bin/varnishd/mgt/mgt_main.c
@@ -339,6 +339,42 @@ make_secret(const char *dirname)
/*--------------------------------------------------------------------*/
+static char stackmin[20];
+
+static void
+init_params(struct cli *cli)
+{
+ ssize_t low;
+
+ MCF_CollectParams();
+
+ /* If we have nobody/nogroup, use them as defaults */
+ if (getpwnam("nobody") != NULL)
+ MCF_SetDefault("user", "nobody");
+ if (getgrnam("nogroup") != NULL)
+ MCF_SetDefault("group", "nogroup");
+
+ if (sizeof(void *) < 8) {
+ /*
+ * Adjust default parameters for 32 bit systems to conserve
+ * VM space.
+ */
+ MCF_SetDefault("workspace_client", "24k");
+ MCF_SetDefault("workspace_backend", "16k");
+ MCF_SetDefault("http_resp_size", "8k");
+ MCF_SetDefault("http_req_size", "12k");
+ MCF_SetDefault("gzip_buffer", "4k");
+ }
+
+ low = sysconf(_SC_THREAD_STACK_MIN);
+ bprintf(stackmin, "%jd", (intmax_t)low);
+ MCF_SetMinimum("thread_pool_stack", stackmin);
+
+ MCF_InitParams(cli);
+}
+
+/*--------------------------------------------------------------------*/
+
int
main(int argc, char * const *argv)
{
@@ -388,6 +424,7 @@ main(int argc, char * const *argv)
/* for ASSERT_MGT() */
mgt_pid = getpid();
+
/*
* Run in UTC timezone, on the off-chance that this operating
* system does not have a timegm() function, and translates
@@ -415,28 +452,7 @@ main(int argc, char * const *argv)
VTAILQ_INIT(&heritage.socks);
- MCF_CollectParams();
-
- /* If we have nobody/nogroup, use them as defaults */
- if (getpwnam("nobody") != NULL)
- MCF_SetDefault("user", "nobody");
- if (getgrnam("nogroup") != NULL)
- MCF_SetDefault("group", "nogroup");
-
- if (sizeof(void *) < 8) {
- /*
- * Adjust default parameters for 32 bit systems to conserve
- * VM space.
- */
- MCF_SetDefault("workspace_client", "24k");
- MCF_SetDefault("workspace_backend", "16k");
- MCF_SetDefault("http_resp_size", "8k");
- MCF_SetDefault("http_req_size", "12k");
- MCF_SetDefault("gzip_buffer", "4k");
- }
-
- MCF_InitParams(cli);
-
+ init_params(cli);
cli_check(cli);
while ((o = getopt(argc, argv,
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index 8f90bd2..05c8820 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -30,18 +30,15 @@
#include "config.h"
#include <ctype.h>
-#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
-#include <unistd.h>
#include "mgt/mgt.h"
#include "common/heritage.h"
#include "common/params.h"
#include "mgt/mgt_param.h"
-#include "waiter/waiter.h"
#include "vav.h"
#include "vcli.h"
#include "vcli_common.h"
@@ -468,13 +465,36 @@ MCF_CollectParams(void)
/*--------------------------------------------------------------------*/
void
-MCF_SetDefault(const char *param, const char *def)
+MCF_SetDefault(const char *param, const char *new_def)
{
struct parspec *pp;
pp = mcf_findpar(param, NULL);
AN(pp);
- pp->def = def;
+ pp->def = new_def;
+ AN(pp->def);
+}
+
+void
+MCF_SetMinimum(const char *param, const char *new_min)
+{
+ struct parspec *pp;
+
+ pp = mcf_findpar(param, NULL);
+ AN(pp);
+ pp->min = new_min;
+ AN(pp->min);
+}
+
+void
+MCF_SetMaximum(const char *param, const char *new_max)
+{
+ struct parspec *pp;
+
+ pp = mcf_findpar(param, NULL);
+ AN(pp);
+ pp->max = new_max;
+ AN(pp->max);
}
/*--------------------------------------------------------------------*/
diff --git a/bin/varnishd/mgt/mgt_param_tweak.c b/bin/varnishd/mgt/mgt_param_tweak.c
index 4b22bc3..55520f4 100644
--- a/bin/varnishd/mgt/mgt_param_tweak.c
+++ b/bin/varnishd/mgt/mgt_param_tweak.c
@@ -36,7 +36,6 @@
#include <limits.h>
#include <math.h>
#include <pwd.h>
-#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
@@ -48,14 +47,9 @@
#include "mgt/mgt_param.h"
#include "waiter/waiter.h"
#include "vav.h"
-#include "vcli.h"
-#include "vcli_common.h"
-#include "vcli_priv.h"
#include "vnum.h"
#include "vss.h"
-#include "mgt_cli.h"
-
/*--------------------------------------------------------------------
* Generic handling of double typed parameters
*/
@@ -156,10 +150,6 @@ int
tweak_bool(struct vsb *vsb, const struct parspec *par, const char *arg)
{
volatile unsigned *dest;
- int mode = 0;
-
- if (!strcmp(par->def, "off") || !strcmp(par->def, "on"))
- mode = 1;
dest = par->priv;
if (arg != NULL) {
@@ -180,16 +170,11 @@ tweak_bool(struct vsb *vsb, const struct parspec *par, const char *arg)
else if (!strcasecmp(arg, "true"))
*dest = 1;
else {
- VSB_printf(vsb, "%s",
- mode ?
- "use \"on\" or \"off\"\n" :
- "use \"true\" or \"false\"\n");
+ VSB_printf(vsb, "use \"on\" or \"off\"\n");
return (-1);
}
- } else if (mode) {
- VSB_printf(vsb, "%s", *dest ? "on" : "off");
} else {
- VSB_printf(vsb, "%s", *dest ? "true" : "false");
+ VSB_printf(vsb, "%s", *dest ? "on" : "off");
}
return (0);
}
diff --git a/bin/varnishd/mgt/mgt_pool.c b/bin/varnishd/mgt/mgt_pool.c
index 8096ed6..91c0982 100644
--- a/bin/varnishd/mgt/mgt_pool.c
+++ b/bin/varnishd/mgt/mgt_pool.c
@@ -43,58 +43,48 @@
#include "config.h"
#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
#include "mgt/mgt.h"
#include "common/params.h"
#include "mgt/mgt_param.h"
-/*--------------------------------------------------------------------*/
-
-static int
-tweak_thread_pool_min(struct vsb *vsb, const struct parspec *par,
- const char *arg)
-{
-
- return (tweak_generic_uint(vsb, &mgt_param.wthread_min, arg,
- // par->min, mgt_param.wthread_max));
- par->min, NULL));
-}
-
/*--------------------------------------------------------------------
- * This is utterly ridiculous: POSIX does not guarantee that the
- * minimum thread stack size is a compile time constant.
- * XXX: "32bit" is a magic marker for 32bit systems.
+ * The min/max values automatically update the opposites appropriate
+ * limit, so they don't end up crossing.
*/
+static char min_val[20];
+static char max_val[20];
+
static int
-tweak_stack_size(struct vsb *vsb, const struct parspec *par,
+tweak_thread_pool_min(struct vsb *vsb, const struct parspec *par,
const char *arg)
{
- ssize_t low;
-
- low = sysconf(_SC_THREAD_STACK_MIN);
+ struct vsb v2;
- if (tweak_bytes(vsb, par, arg))
+ if (tweak_generic_uint(vsb, par->priv, arg, par->min, par->max))
return (-1);
- if (mgt_param.wthread_stacksize < low)
- mgt_param.wthread_stacksize = low;
+ AN(VSB_new(&v2, min_val, sizeof min_val, 0));
+ AZ(tweak_generic_uint(&v2, &mgt_param.wthread_min, NULL, NULL, NULL));
+ AZ(VSB_finish(&v2));
+ MCF_SetMinimum("thread_pool_max", min_val);
return (0);
}
-/*--------------------------------------------------------------------*/
-
static int
tweak_thread_pool_max(struct vsb *vsb, const struct parspec *par,
const char *arg)
{
+ struct vsb v2;
- (void)par;
- return (tweak_generic_uint(vsb, &mgt_param.wthread_max, arg,
- //mgt_param.wthread_min, NULL));
- NULL, NULL));
+ if (tweak_generic_uint(vsb, par->priv, arg, par->min, par->max))
+ return (-1);
+ AN(VSB_new(&v2, max_val, sizeof max_val, 0));
+ AZ(tweak_generic_uint(&v2, &mgt_param.wthread_max, NULL, NULL, NULL));
+ AZ(VSB_finish(&v2));
+ MCF_SetMaximum("thread_pool_min", max_val);
+ return (0);
}
/*--------------------------------------------------------------------*/
@@ -114,8 +104,8 @@ struct parspec WRK_parspec[] = {
"restart to take effect.",
EXPERIMENTAL | DELAYED_EFFECT,
"2", "pools" },
- { "thread_pool_max", tweak_thread_pool_max, NULL,
- "10", NULL,
+ { "thread_pool_max", tweak_thread_pool_max, &mgt_param.wthread_max,
+ NULL, NULL,
"The maximum number of worker threads in each pool.\n"
"\n"
"Do not set this higher than you have to, since excess "
@@ -125,8 +115,8 @@ struct parspec WRK_parspec[] = {
"Minimum is 10 threads.",
DELAYED_EFFECT,
"5000", "threads" },
- { "thread_pool_min", tweak_thread_pool_min, NULL,
- "10", NULL,
+ { "thread_pool_min", tweak_thread_pool_min, &mgt_param.wthread_min,
+ NULL, NULL,
"The minimum number of worker threads in each pool.\n"
"\n"
"Increasing this may help ramp up faster from low load "
@@ -218,11 +208,11 @@ struct parspec WRK_parspec[] = {
EXPERIMENTAL,
"3", "requests per request" },
{ "thread_pool_stack",
- tweak_stack_size, &mgt_param.wthread_stacksize,
- "0", NULL,
+ tweak_bytes, &mgt_param.wthread_stacksize,
+ NULL, NULL,
"Worker thread stack size.\n"
- "This is likely rounded up to a multiple of 4k by the kernel.\n"
- "The kernel/OS has a lower limit which will be enforced.",
+ "This will likely be rounded up to a multiple of 4k"
+ " (or whatever the page_size might be) by the kernel.",
EXPERIMENTAL,
"48k", "bytes" },
{ NULL, NULL, NULL }
diff --git a/bin/varnishtest/tests/b00042.vtc b/bin/varnishtest/tests/b00042.vtc
index 1a6f947..1aeb4a4 100644
--- a/bin/varnishtest/tests/b00042.vtc
+++ b/bin/varnishtest/tests/b00042.vtc
@@ -17,12 +17,17 @@ varnish v1 -clierr "106" "param.set prefer_ipv6 foobar"
varnish v1 -clierr "106" "param.set http_max_hdr 0"
varnish v1 -clierr "106" "param.set http_max_hdr 1000000"
varnish v1 -clierr "106" "param.set workspace_thread 1b"
-varnish v1 -clierr "106" "param.set workspace_thread 1b"
+varnish v1 -clierr "106" "param.set workspace_thread 1m"
varnish v1 -clierr "106" "param.set workspace_thread 1x"
varnish v1 -clierr "106" "param.set workspace_thread x"
varnish v1 -clierr "106" "param.set user ///"
varnish v1 -clierr "106" "param.set user ///"
varnish v1 -clierr "106" {param.set listen_address ""}
-varnish v1 -clierr "106" {param.set listen_address ",,"}
+varnish v1 -clierr "106" {param.set listen_address "&&"}
varnish v1 -clierr "106" {param.set listen_address "\""}
varnish v1 -clierr "106" {param.set pool_sess "\""}
+varnish v1 -clierr "200" {param.set thread_pool_max 110}
+varnish v1 -clierr "106" {param.set thread_pool_min 111}
+varnish v1 -clierr "200" {param.set thread_pool_min 51}
+varnish v1 -clierr "106" {param.set thread_pool_max 50}
+varnish v1 -clierr "200" {param.set thread_pool_max 51}
More information about the varnish-commit
mailing list