summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Goulet <dgoulet@torproject.org>2022-11-09 12:49:23 -0500
committerDavid Goulet <dgoulet@torproject.org>2022-11-09 13:13:21 -0500
commite3f6908984c6f2e6361a1a15f37d6bb0647efda9 (patch)
treec7afb64fa267fb53625ff9f56e32dac178368a55
parent9c8c7804d535b4248e7e029c969d9a77a54947f6 (diff)
downloadtor-e3f6908984c6f2e6361a1a15f37d6bb0647efda9.tar.gz
tor-e3f6908984c6f2e6361a1a15f37d6bb0647efda9.zip
relay: Make the max pending tasks per CPU a consensus parameter
Until now, there was this magic number (64) used as the maximum number of tasks a CPU worker can take at once. This commit makes it a consensus parameter so our future selves can think of a better value depending on network conditions. Part of #40704 Signed-off-by: David Goulet <dgoulet@torproject.org>
-rw-r--r--changes/ticket407042
-rw-r--r--src/core/mainloop/cpuworker.c42
-rw-r--r--src/core/mainloop/cpuworker.h5
-rw-r--r--src/feature/nodelist/networkstatus.c2
-rw-r--r--src/lib/thread/numcpus.c8
5 files changed, 51 insertions, 8 deletions
diff --git a/changes/ticket40704 b/changes/ticket40704
index b65e88ea09..b1a83488da 100644
--- a/changes/ticket40704
+++ b/changes/ticket40704
@@ -2,3 +2,5 @@
- Two new consensus parameters are added to control the wait time in queue
of the onionskins. One of them is the torrc MaxOnionQueueDelay options
which supersedes the consensus parameter. Closes ticket 40704.
+ - Change a hardcoded value for the maximum of per CPU tasks into a
+ consensus parameter.
diff --git a/src/core/mainloop/cpuworker.c b/src/core/mainloop/cpuworker.c
index ab970259b5..3ad556a184 100644
--- a/src/core/mainloop/cpuworker.c
+++ b/src/core/mainloop/cpuworker.c
@@ -32,6 +32,7 @@
#include "feature/relay/onion_queue.h"
#include "feature/stats/rephist.h"
#include "feature/relay/router.h"
+#include "feature/nodelist/networkstatus.h"
#include "lib/evloop/workqueue.h"
#include "core/crypto/onion_crypto.h"
@@ -75,8 +76,42 @@ worker_state_free_void(void *arg)
static replyqueue_t *replyqueue = NULL;
static threadpool_t *threadpool = NULL;
-static int total_pending_tasks = 0;
-static int max_pending_tasks = 128;
+static uint32_t total_pending_tasks = 0;
+static uint32_t max_pending_tasks = 128;
+
+/** Return the consensus parameter max pending tasks per CPU. */
+static uint32_t
+get_max_pending_tasks_per_cpu(const networkstatus_t *ns)
+{
+/* Total voodoo. Can we make this more sensible? Maybe, that is why we made it
+ * a consensus parameter so our future self can figure out this magic. */
+#define MAX_PENDING_TASKS_PER_CPU_DEFAULT 64
+#define MAX_PENDING_TASKS_PER_CPU_MIN 1
+#define MAX_PENDING_TASKS_PER_CPU_MAX INT32_MAX
+
+ return networkstatus_get_param(ns, "max_pending_tasks_per_cpu",
+ MAX_PENDING_TASKS_PER_CPU_DEFAULT,
+ MAX_PENDING_TASKS_PER_CPU_MIN,
+ MAX_PENDING_TASKS_PER_CPU_MAX);
+}
+
+/** Set the max pending tasks per CPU worker. This uses the consensus to check
+ * for the allowed number per CPU. The ns parameter can be NULL as in that no
+ * consensus is available at the time of setting this value. */
+static void
+set_max_pending_tasks(const networkstatus_t *ns)
+{
+ max_pending_tasks =
+ get_num_cpus(get_options()) * get_max_pending_tasks_per_cpu(ns);
+}
+
+/** Called when the consensus has changed. */
+void
+cpuworker_consensus_has_changed(const networkstatus_t *ns)
+{
+ tor_assert(ns);
+ set_max_pending_tasks(ns);
+}
/** Initialize the cpuworker subsystem. It is OK to call this more than once
* during Tor's lifetime.
@@ -106,8 +141,7 @@ cpu_init(void)
tor_assert(r == 0);
}
- /* Total voodoo. Can we make this more sensible? */
- max_pending_tasks = get_num_cpus(get_options()) * 64;
+ set_max_pending_tasks(NULL);
}
/** Magic numbers to make sure our cpuworker_requests don't grow any
diff --git a/src/core/mainloop/cpuworker.h b/src/core/mainloop/cpuworker.h
index 9e589a3004..94545af586 100644
--- a/src/core/mainloop/cpuworker.h
+++ b/src/core/mainloop/cpuworker.h
@@ -12,8 +12,13 @@
#ifndef TOR_CPUWORKER_H
#define TOR_CPUWORKER_H
+#include "feature/nodelist/networkstatus_st.h"
+
void cpu_init(void);
void cpuworkers_rotate_keyinfo(void);
+
+void cpuworker_consensus_has_changed(const networkstatus_t *ns);
+
struct workqueue_entry_t;
enum workqueue_reply_t;
enum workqueue_priority_t;
diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c
index aaddf2331d..fa1e38dac0 100644
--- a/src/feature/nodelist/networkstatus.c
+++ b/src/feature/nodelist/networkstatus.c
@@ -40,6 +40,7 @@
#include "core/or/or.h"
#include "app/config/config.h"
#include "core/mainloop/connection.h"
+#include "core/mainloop/cpuworker.h"
#include "core/mainloop/mainloop.h"
#include "core/mainloop/netstatus.h"
#include "core/or/channel.h"
@@ -1668,6 +1669,7 @@ notify_before_networkstatus_changes(const networkstatus_t *old_c,
relay_consensus_has_changed(new_c);
hs_dos_consensus_has_changed(new_c);
rep_hist_consensus_has_changed(new_c);
+ cpuworker_consensus_has_changed(new_c);
}
/* Called after a new consensus has been put in the global state. It is safe
diff --git a/src/lib/thread/numcpus.c b/src/lib/thread/numcpus.c
index e0ddb69931..40fac7dbe4 100644
--- a/src/lib/thread/numcpus.c
+++ b/src/lib/thread/numcpus.c
@@ -53,10 +53,10 @@ compute_num_cpus_impl(void)
cpus = cpus_onln;
} else if (cpus_onln > 0 && cpus_conf > 0) {
if (cpus_onln < cpus_conf) {
- log_notice(LD_GENERAL, "I think we have %ld CPUS, but only %ld of them "
- "are available. Telling Tor to only use %ld. You can over"
- "ride this with the NumCPUs option",
- cpus_conf, cpus_onln, cpus_onln);
+ log_info(LD_GENERAL, "I think we have %ld CPUS, but only %ld of them "
+ "are available. Telling Tor to only use %ld. You can over"
+ "ride this with the NumCPUs option",
+ cpus_conf, cpus_onln, cpus_onln);
}
cpus = cpus_onln;
}