On 5/31/22 09:01, Aaron Conole wrote:
Michael Santana <[email protected]> writes:
Additional threads are required to service upcalls when we have CPU
isolation (in per-cpu dispatch mode). The formula used to calculate
the number of handler threads to create is as follows
handlers_n = min(next_prime(active_cores+1), total_cores)
Where next_prime is a function that returns the next numeric prime
number that is strictly greater than active_cores
Fixes: b1e517bd2f81 ("dpif-netlink: Introduce per-cpu upcall dispatch.")
Signed-off-by: Michael Santana <[email protected]>
---
lib/dpif-netlink.c | 32 ++++++++++++++++++++-
lib/ovs-thread.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
lib/ovs-thread.h | 3 ++
3 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 06e1e8ca0..53de16e12 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2506,6 +2506,36 @@ dpif_netlink_handler_uninit(struct dpif_handler *handler)
}
#endif
+/*
+ * Calcuales and returns the number of handler threads needed based
+ * the following formula:
+ *
+ * handlers_n = min(next_prime(active_cores+1), total_cores)
+ *
+ * Where next_prime is a function that returns the next numeric prime
+ * number that is strictly greater than active_cores
+ */
+static uint32_t
+dpif_netlink_calculate_handlers_num(void)
+{
+ uint32_t next_prime_num;
+ uint32_t n_handlers = count_cpu_cores();
+ uint32_t total_cores = count_total_cores();
+
+ /*
+ * If we have isolated cores, add additional handler threads to
+ * service inactive cores in the unlikely event that traffic goes
+ * through inactive cores
+ */
+ if (n_handlers < total_cores) {
+ next_prime_num = next_prime(n_handlers +1, 10000000);
+ n_handlers = next_prime_num >= total_cores ?
+ total_cores : next_prime_num;
+ }
+
+ return n_handlers;
+}
+
static int
dpif_netlink_refresh_handlers_cpu_dispatch(struct dpif_netlink *dpif)
OVS_REQ_WRLOCK(dpif->upcall_lock)
@@ -2755,7 +2785,7 @@ dpif_netlink_number_handlers_required(struct dpif *dpif_,
uint32_t *n_handlers)
struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
if (dpif_netlink_upcall_per_cpu(dpif)) {
- *n_handlers = count_cpu_cores();
+ *n_handlers = dpif_netlink_calculate_handlers_num();
return true;
}
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 805cba622..fc3080d7c 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -18,6 +18,7 @@
#include "ovs-thread.h"
#include <errno.h>
#include <poll.h>
+#include <math.h>
#ifndef _WIN32
#include <signal.h>
#endif
@@ -663,6 +664,74 @@ count_cpu_cores(void)
return n_cores > 0 ? n_cores : 0;
}
+/* Returns the total number of cores on the system, or 0 if the
+ * number cannot be determined. */
+int
+count_total_cores(void) {
+ long int n_cores;
+
+#ifndef _WIN32
+ n_cores = sysconf(_SC_NPROCESSORS_CONF);
+#else
+ n_cores = 0;
+ errno = ENOTSUP;
+#endif
+
+ return n_cores > 0 ? n_cores : 0;
+}
+
+/*
+ * Returns 1 if num is a prime number,
+ * Otherwise return 0
+ */
Should this really be in ovs-thread? Does it need to be in the
"exposed" API? For example, we could just leave it out of the header
and make it static. WDYT?
This makes sense. The prime calculation stuff is not relevant for
threads. the count_total_cores can stay, but the other two functions
better to move.
Just one clarification if we make those functions static in the same
ovs-thread.c file they wont be visible to dpif-netlink.c which is where
we make use of them. We probably want to move those functions to this
file and make them static?
+uint32_t
+is_prime(uint32_t num)
+{
1 and 0 aren't prime numbers, but this will return '1' anyway. In this
particular patch usage it isn't important. In the future, maybe it
will be. I guess this should be something like:
Thanks for checking my math. Your suggestion does cover those edge
cases. I will add them on next revision
+ if ((num % 2) == 0) {
+ if ((num < 2) || ((num % 2) == 0)) {
+ return num == 2;
+ }
+
+ uint32_t i;
+ uint32_t limit = sqrt((float) num);
+ for (i = 3; i <= limit; i += 2) {
+ if (num % i == 0) {
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
+/*
+ * Returns num if num is a prime number. Otherwise returns the next
+ * prime greater than num. Search is limited by the limit variable.
+ *
+ * Returns 0 if num >= limit, or if no prime has been found between
+ * num and limit
+ */
+uint32_t
+next_prime(uint32_t num, uint32_t limit)
+{
+ if (num < 2) {
+ return 2;
+ }
+ if (num == 2) {
+ return 3;
+ }
+ if (num % 2 == 0) {
+ num++;
+ }
+
+ uint32_t i;
+ for (i = num; i < limit; i += 2) {
+ if (is_prime(i)) {
+ return i;
+ }
+ }
+
+ return 0;
+}
+
/* Returns 'true' if current thread is PMD thread. */
bool
thread_is_pmd(void)
diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index 3b444ccdc..6c1ebd098 100644
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -522,6 +522,9 @@ bool may_fork(void);
/* Useful functions related to threading. */
int count_cpu_cores(void);
+int count_total_cores(void);
+uint32_t is_prime(uint32_t num);
+uint32_t next_prime(uint32_t num, uint32_t limit);
Maybe this should be in a different location - I suggest the 'util'
files (util.{c,h}).
This wont be necessary if we make them static as mentioned above?
bool thread_is_pmd(void);
#endif /* ovs-thread.h */
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
