[tor-commits] [tor/master] sched: Make KISTSchedRunInterval non negative
nickm at torproject.org
nickm at torproject.org
Mon Sep 25 15:13:00 UTC 2017
commit ef2a449cceceb777126119d20c15cda707c17d61
Author: David Goulet <dgoulet at torproject.org>
Date: Fri Sep 22 11:33:50 2017 -0400
sched: Make KISTSchedRunInterval non negative
Fixes #23539.
Signed-off-by: David Goulet <dgoulet at torproject.org>
---
changes/bug23539 | 4 ++++
src/or/or.h | 2 +-
src/or/scheduler.c | 9 ++-------
src/or/scheduler.h | 2 +-
src/or/scheduler_kist.c | 47 ++++++++++++++++++++++++-----------------------
src/test/test_scheduler.c | 14 +++-----------
6 files changed, 35 insertions(+), 43 deletions(-)
diff --git a/changes/bug23539 b/changes/bug23539
new file mode 100644
index 000000000..b3f6d3b45
--- /dev/null
+++ b/changes/bug23539
@@ -0,0 +1,4 @@
+ o Minor bugfixes (scheduler, KIST):
+ - Make the KISTSchedRunInterval option a non negative value. With this,
+ the way to disable KIST through the consensus is to set it to 0.
+ Fixes bug 23539; bugfix on 0.3.2.1-alpha.
diff --git a/src/or/or.h b/src/or/or.h
index 10a2b5741..9a9a70ccc 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4615,7 +4615,7 @@ typedef struct {
* not use the KIST scheduler but use the old vanilla scheduler instead. If
* zero, do what the consensus says and fall back to using KIST as if this is
* set to "10 msec" if the consensus doesn't say anything. */
- int64_t KISTSchedRunInterval;
+ uint32_t KISTSchedRunInterval;
/** A multiplier for the KIST per-socket limit calculation. */
double KISTSockBufSizeFactor;
diff --git a/src/or/scheduler.c b/src/or/scheduler.c
index cc0161122..4a3289030 100644
--- a/src/or/scheduler.c
+++ b/src/or/scheduler.c
@@ -249,13 +249,8 @@ select_scheduler(void)
case SCHEDULER_KIST:
if (!scheduler_can_use_kist()) {
#ifdef HAVE_KIST_SUPPORT
- if (get_options()->KISTSchedRunInterval == -1) {
- log_info(LD_SCHED, "Scheduler type KIST can not be used. It is "
- "disabled because KISTSchedRunInterval=-1");
- } else {
- log_notice(LD_SCHED, "Scheduler type KIST has been disabled by "
- "the consensus.");
- }
+ log_notice(LD_SCHED, "Scheduler type KIST has been disabled by "
+ "the consensus or no kernel support.");
#else /* !(defined(HAVE_KIST_SUPPORT)) */
log_info(LD_SCHED, "Scheduler type KIST not built in");
#endif /* defined(HAVE_KIST_SUPPORT) */
diff --git a/src/or/scheduler.h b/src/or/scheduler.h
index f740230b1..e32a662c8 100644
--- a/src/or/scheduler.h
+++ b/src/or/scheduler.h
@@ -189,7 +189,7 @@ int scheduler_can_use_kist(void);
void scheduler_kist_set_full_mode(void);
void scheduler_kist_set_lite_mode(void);
scheduler_t *get_kist_scheduler(void);
-int32_t kist_scheduler_run_interval(const networkstatus_t *ns);
+uint32_t kist_scheduler_run_interval(const networkstatus_t *ns);
#ifdef TOR_UNIT_TESTS
extern int32_t sched_run_interval;
diff --git a/src/or/scheduler_kist.c b/src/or/scheduler_kist.c
index 5ba31bb87..8fe3ff556 100644
--- a/src/or/scheduler_kist.c
+++ b/src/or/scheduler_kist.c
@@ -208,8 +208,8 @@ update_socket_info_impl, (socket_table_ent_t *ent))
* support. */
log_notice(LD_SCHED, "Looks like our kernel doesn't have the support "
"for KIST anymore. We will fallback to the naive "
- "approach. Set KISTSchedRunInterval=-1 to disable "
- "KIST.");
+ "approach. Remove KIST from the Schedulers list "
+ "to disable.");
kist_no_kernel_support = 1;
}
goto fallback;
@@ -218,8 +218,8 @@ update_socket_info_impl, (socket_table_ent_t *ent))
if (errno == EINVAL) {
log_notice(LD_SCHED, "Looks like our kernel doesn't have the support "
"for KIST anymore. We will fallback to the naive "
- "approach. Set KISTSchedRunInterval=-1 to disable "
- "KIST.");
+ "approach. Remove KIST from the Schedulers list "
+ "to disable.");
/* Same reason as the above. */
kist_no_kernel_support = 1;
}
@@ -491,7 +491,7 @@ static void
kist_scheduler_init(void)
{
kist_scheduler_on_new_options();
- IF_BUG_ONCE(sched_run_interval <= 0) {
+ IF_BUG_ONCE(sched_run_interval == 0) {
log_warn(LD_SCHED, "We are initing the KIST scheduler and noticed the "
"KISTSchedRunInterval is telling us to not use KIST. That's "
"weird! We'll continue using KIST, but at %dms.",
@@ -705,33 +705,34 @@ get_kist_scheduler(void)
return &kist_scheduler;
}
-/* Check the torrc for the configured KIST scheduler run interval.
- * - If torrc < 0, then return the negative torrc value (shouldn't even be
- * using KIST)
+/* Check the torrc (and maybe consensus) for the configured KIST scheduler run
+ * interval.
* - If torrc > 0, then return the positive torrc value (should use KIST, and
* should use the set value)
* - If torrc == 0, then look in the consensus for what the value should be.
- * - If == 0, then return -1 (don't use KIST)
+ * - If == 0, then return 0 (don't use KIST)
* - If > 0, then return the positive consensus value
- * - If consensus doesn't say anything, return 10 milliseconds
+ * - If consensus doesn't say anything, return 10 milliseconds, default.
*/
-int32_t
+uint32_t
kist_scheduler_run_interval(const networkstatus_t *ns)
{
- int32_t run_interval = (int32_t)get_options()->KISTSchedRunInterval;
+ uint32_t run_interval = get_options()->KISTSchedRunInterval;
+
if (run_interval != 0) {
- log_debug(LD_SCHED, "Found KISTSchedRunInterval in torrc. Using that.");
+ log_debug(LD_SCHED, "Found KISTSchedRunInterval=%" PRIu32 " in torrc. "
+ "Using that.", run_interval);
return run_interval;
}
- log_debug(LD_SCHED, "Turning to the consensus for KISTSchedRunInterval");
- run_interval = networkstatus_get_param(ns, "KISTSchedRunInterval",
- KIST_SCHED_RUN_INTERVAL_DEFAULT,
- KIST_SCHED_RUN_INTERVAL_MIN,
- KIST_SCHED_RUN_INTERVAL_MAX);
- if (run_interval <= 0)
- return -1;
- return run_interval;
+ log_debug(LD_SCHED, "KISTSchedRunInterval=0, turning to the consensus.");
+
+ /* Will either be the consensus value or the default. Note that 0 can be
+ * returned which means the consensus wants us to NOT use KIST. */
+ return networkstatus_get_param(ns, "KISTSchedRunInterval",
+ KIST_SCHED_RUN_INTERVAL_DEFAULT,
+ KIST_SCHED_RUN_INTERVAL_MIN,
+ KIST_SCHED_RUN_INTERVAL_MAX);
}
/* Set KISTLite mode that is KIST without kernel support. */
@@ -767,9 +768,9 @@ scheduler_can_use_kist(void)
/* We do have the support, time to check if we can get the interval that the
* consensus can be disabling. */
- int64_t run_interval = kist_scheduler_run_interval(NULL);
+ uint32_t run_interval = kist_scheduler_run_interval(NULL);
log_debug(LD_SCHED, "Determined KIST sched_run_interval should be "
- "%" PRId64 ". Can%s use KIST.",
+ "%" PRIu32 ". Can%s use KIST.",
run_interval, (run_interval > 0 ? "" : " not"));
return run_interval > 0;
}
diff --git a/src/test/test_scheduler.c b/src/test/test_scheduler.c
index 51bedb3f9..d679d7cfe 100644
--- a/src/test/test_scheduler.c
+++ b/src/test/test_scheduler.c
@@ -84,7 +84,7 @@ mock_vanilla_networkstatus_get_param(
(void)max_val;
// only support KISTSchedRunInterval right now
tor_assert(strcmp(param_name, "KISTSchedRunInterval")==0);
- return -1;
+ return 0;
}
static int32_t
@@ -628,7 +628,7 @@ test_scheduler_loop_vanilla(void *arg)
MOCK(get_options, mock_get_options);
clear_options();
set_scheduler_options(SCHEDULER_VANILLA);
- mocked_options.KISTSchedRunInterval = -1;
+ mocked_options.KISTSchedRunInterval = 0;
/* Set up libevent and scheduler */
@@ -932,14 +932,6 @@ test_scheduler_can_use_kist(void *arg)
int res_should, res_freq;
MOCK(get_options, mock_get_options);
- /* Test force disabling of KIST */
- clear_options();
- mocked_options.KISTSchedRunInterval = -1;
- res_should = scheduler_can_use_kist();
- res_freq = kist_scheduler_run_interval(NULL);
- tt_int_op(res_should, ==, 0);
- tt_int_op(res_freq, ==, -1);
-
/* Test force enabling of KIST */
clear_options();
mocked_options.KISTSchedRunInterval = 1234;
@@ -985,7 +977,7 @@ test_scheduler_can_use_kist(void *arg)
res_should = scheduler_can_use_kist();
res_freq = kist_scheduler_run_interval(NULL);
tt_int_op(res_should, ==, 0);
- tt_int_op(res_freq, ==, -1);
+ tt_int_op(res_freq, ==, 0);
UNMOCK(networkstatus_get_param);
done:
More information about the tor-commits
mailing list