This is an automated email from the git hooks/post-receive script.
dgoulet pushed a commit to branch main in repository tor.
commit e3f6908984c6f2e6361a1a15f37d6bb0647efda9 Author: David Goulet dgoulet@torproject.org AuthorDate: Wed Nov 9 12:49:23 2022 -0500
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 --- changes/ticket40704 | 2 ++ src/core/mainloop/cpuworker.c | 42 ++++++++++++++++++++++++++++++++---- src/core/mainloop/cpuworker.h | 5 +++++ src/feature/nodelist/networkstatus.c | 2 ++ src/lib/thread/numcpus.c | 8 +++---- 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; }