Browse Source

Fix some issues with the scheduler configuration options

1) Set them to the values that (according to Rob) avoided performance
   regressions.  This means that the scheduler won't get much exercise
   until we implement KIST or something like it.

2) Rename the options to end with a __, since I think they might be
   going away, and nobody should mess with them.

3) Use the correct types for the option variables. MEMUNIT needs to be a
   uint64_t; UINT needs to be (I know, I know!) an int.

4) Validate the values in options_validate(); do the switch in
   options_act(). This way, setting the option to an invalid value on
   a running Tor will get backed out.
Nick Mathewson 9 years ago
parent
commit
b1e1b439b8
2 changed files with 21 additions and 23 deletions
  1. 18 20
      src/or/config.c
  2. 3 3
      src/or/or.h

+ 18 - 20
src/or/config.c

@@ -369,9 +369,9 @@ static config_var_t option_vars_[] = {
   V(ServerDNSSearchDomains,      BOOL,     "0"),
   V(ServerDNSTestAddresses,      CSV,
       "www.google.com,www.mit.edu,www.yahoo.com,www.slashdot.org"),
-  V(SchedulerLowWaterMark,       MEMUNIT,  "16 kB"),
-  V(SchedulerHighWaterMark,      MEMUNIT,  "32 kB"),
-  V(SchedulerMaxFlushCells,      UINT,     "16"),
+  V(SchedulerLowWaterMark__,     MEMUNIT,  "100 MB"),
+  V(SchedulerHighWaterMark__,    MEMUNIT,  "101 MB"),
+  V(SchedulerMaxFlushCells__,    UINT,     "1000"),
   V(ShutdownWaitLength,          INTERVAL, "30 seconds"),
   V(SocksListenAddress,          LINELIST, NULL),
   V(SocksPolicy,                 LINELIST, NULL),
@@ -1539,23 +1539,10 @@ options_act(const or_options_t *old_options)
   }
 
   /* Set up scheduler thresholds */
-  if (options->SchedulerLowWaterMark > 0 &&
-      options->SchedulerHighWaterMark > options->SchedulerLowWaterMark) {
-    scheduler_set_watermarks(options->SchedulerLowWaterMark,
-                             options->SchedulerHighWaterMark,
-                             (options->SchedulerMaxFlushCells > 0) ?
-                              options->SchedulerMaxFlushCells : 16);
-  } else {
-    if (options->SchedulerLowWaterMark == 0) {
-      log_warn(LD_GENERAL, "Bad SchedulerLowWaterMark option");
-    }
-
-    if (options->SchedulerHighWaterMark <= options->SchedulerLowWaterMark) {
-      log_warn(LD_GENERAL, "Bad SchedulerHighWaterMark option");
-    }
-
-    return -1;
-  }
+  scheduler_set_watermarks((uint32_t)options->SchedulerLowWaterMark__,
+                           (uint32_t)options->SchedulerHighWaterMark__,
+                           (options->SchedulerMaxFlushCells__ > 0) ?
+                           options->SchedulerMaxFlushCells__ : 1000);
 
   /* Set up accounting */
   if (accounting_parse_options(options, 0)<0) {
@@ -2654,6 +2641,17 @@ options_validate(or_options_t *old_options, or_options_t *options,
     routerset_union(options->ExcludeExitNodesUnion_,options->ExcludeNodes);
   }
 
+  if (options->SchedulerLowWaterMark__ == 0 ||
+      options->SchedulerLowWaterMark__ > UINT32_MAX) {
+    log_warn(LD_GENERAL, "Bad SchedulerLowWaterMark__ option");
+    return -1;
+  } else if (options->SchedulerHighWaterMark__ <=
+             options->SchedulerLowWaterMark__ ||
+             options->SchedulerHighWaterMark__ > UINT32_MAX) {
+    log_warn(LD_GENERAL, "Bad SchedulerHighWaterMark option");
+    return -1;
+  }
+
   if (options->NodeFamilies) {
     options->NodeFamilySets = smartlist_new();
     for (cl = options->NodeFamilies; cl; cl = cl->next) {

+ 3 - 3
src/or/or.h

@@ -4251,15 +4251,15 @@ typedef struct {
   /** Low-water mark for global scheduler - start sending when estimated
    * queued size falls below this threshold.
    */
-  uint32_t SchedulerLowWaterMark;
+  uint64_t SchedulerLowWaterMark__;
   /** High-water mark for global scheduler - stop sending when estimated
    * queued size exceeds this threshold.
    */
-  uint32_t SchedulerHighWaterMark;
+  uint64_t SchedulerHighWaterMark__;
   /** Flush size for global scheduler - flush this many cells at a time
    * when sending.
    */
-  unsigned int SchedulerMaxFlushCells;
+  int SchedulerMaxFlushCells__;
 } or_options_t;
 
 /** Persistent state for an onion router, as saved to disk. */