Currently, things like the number of handler and revalidator threads are
calculated based on the number of available CPUs. However, this number
is considered static and only calculated once, hence ignoring events
such as cpus being hotplugged, switched on/off or affinity mask
changing.
On the other hand, checking the number of available CPUs multiple times
per second seems like an overkill.
Affinity should not change that often and, even if it does, the impact
of destroying and recreating all the threads so often is probably a
price too expensive to pay.
This patch makes the number of cpus be calculated every time 5 seconds
which seems a reasonable middle point.
It generates an impact in the main loop duration of <1% and a worst-case
scenario impact in throughput of < 5% [1].
As a result of these changes (assuming the patch is backported):
- >=2.16: a change in the cpu affinity reflects on the number of threads
in (at most) 5 seconds.
- < 2.16: a change in the cpu affinity will be reflected on
the number of threads the next time there is a call to
bridge_reconfigure() (e.g: on the next DB change), and 5 seconds
have passed.
The difference in behavior is because on older versions the thread
number calculation was done on bridge reconfiguration while newer
versions moved this logic down to the dpif layer and is run on
dpif->run() stage.
Considering it has not been a huge problem up to today and that the
cpu change would be reflected sooner or later (e.g the user could
force a recalculation with a simple ovs-vsctl command), I think it
might be OK to leave like that.
[1] Tested in the worst-case scenario of disabling the kernel cache
(other_config:flow-size=0), modifying ovs-vswithd's affinity so the
number of handlers go up and down every 5 seconds and calculated the
difference in netperf's ops/sec.
Fixes: be15ec48d766 ("lib: Use a more accurate value for CPU count
(sched_getaffinity).")
Cc: [email protected]
Signed-off-by: Adrian Moreno <[email protected]>
---
lib/ovs-thread.c | 61 +++++++++++++++++++++++++++++-------------------
1 file changed, 37 insertions(+), 24 deletions(-)
diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index 78ed3e970..d1deb9c52 100644
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -31,6 +31,7 @@
#include "openvswitch/poll-loop.h"
#include "seq.h"
#include "socket-util.h"
+#include "timeval.h"
#include "util.h"
#ifdef __CHECKER__
@@ -627,42 +628,54 @@ ovs_thread_stats_next_bucket(const struct ovsthread_stats
*stats, size_t i)
}
-/* Returns the total number of cores available to this process, or 0 if the
- * number cannot be determined. */
-int
-count_cpu_cores(void)
+static int
+count_cpu_cores__(void)
{
- static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
- static long int n_cores;
+ long int n_cores;
- if (ovsthread_once_start(&once)) {
#ifndef _WIN32
- n_cores = sysconf(_SC_NPROCESSORS_ONLN);
+ n_cores = sysconf(_SC_NPROCESSORS_ONLN);
+#else
+ SYSTEM_INFO sysinfo;
+ GetSystemInfo(&sysinfo);
+ n_cores = sysinfo.dwNumberOfProcessors;
+#endif
#ifdef __linux__
- if (n_cores > 0) {
- cpu_set_t *set = CPU_ALLOC(n_cores);
+ if (n_cores > 0) {
+ cpu_set_t *set = CPU_ALLOC(n_cores);
- if (set) {
- size_t size = CPU_ALLOC_SIZE(n_cores);
+ if (set) {
+ size_t size = CPU_ALLOC_SIZE(n_cores);
- if (!sched_getaffinity(0, size, set)) {
- n_cores = CPU_COUNT_S(size, set);
- }
- CPU_FREE(set);
+ if (!sched_getaffinity(0, size, set)) {
+ n_cores = CPU_COUNT_S(size, set);
}
+ CPU_FREE(set);
}
-#endif
-#else
- SYSTEM_INFO sysinfo;
- GetSystemInfo(&sysinfo);
- n_cores = sysinfo.dwNumberOfProcessors;
-#endif
- ovsthread_once_done(&once);
}
-
+#endif
return n_cores > 0 ? n_cores : 0;
}
+/* It's unlikely that the available cpus change several times per second and
+ * even if it does, it's not needed (or desired) to react to such changes so
+ * quickly.*/
+#define COUNT_CPU_UPDATE_TIME_MS 5000
+/* Returns the current total number of cores available to this process, or 0
+ * if the number cannot be determined.
+ * It is assumed that this function is only called from the main thread.*/
+int count_cpu_cores(void) {
+ static long long int last_updated = 0;
+ long long int now = time_msec();
+ static int cpu_cores;
+
+ if (now - last_updated >= COUNT_CPU_UPDATE_TIME_MS) {
+ last_updated = now;
+ cpu_cores = count_cpu_cores__();
+ }
+ return cpu_cores;
+}
+
/* Returns the total number of cores on the system, or 0 if the
* number cannot be determined. */
int
--
2.37.3
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
