On Fri, Jun 17, 2022 at 12:10 PM Mike Pattrick <m...@redhat.com> wrote:
>
> On Mon, Jun 6, 2022 at 3:00 PM Michael Santana <msant...@redhat.com> wrote:
> >
> > Additional threads are required to service upcalls when we have CPU
> > isolation (in per-cpu dispatch mode). The reason additional threads
> > are required is because it creates a more fair distribution. With more
> > threads we decrease the load of each thread as more threads would
> > decrease the number of cores each threads is assigned. Adding as many
> > threads are there are cores could have a performance hit when the number
> > of active cores (which all threads have to share) is very low. For this
> > reason we avoid creating as many threads as there are cores and instead
> > meet somewhere in the middle.
> >
> > 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)
>
> Why are primes superior to just any odd number in between active and
> total_cores?
They aren't. Both strategies are valid and sufficient. The primes do
provide a more or less even distribution. We dont want too many
threads and not enough additional threads
I had proposed the following but it again it is not better or worse
than the prime approach. It mainly just scales linearly
handlers_n = min(active_cores/4, (total_cores-active_cores)/4)
>
> I may just be missing something; but if there were 8 active cores and
> 12 total cores, why would 11 handlers be better than 9?
>
> Cheers,
> M
>
> >
> > Where next_prime is a function that returns the next numeric prime
> > number that is strictly greater than active_cores
> >
> > Assume default behavior when total_cores <= 2, that is do not create
> > additional threads when we have less than 2 total cores on the system
> >
> > Fixes: b1e517bd2f81 ("dpif-netlink: Introduce per-cpu upcall dispatch.")
> > Signed-off-by: Michael Santana <msant...@redhat.com>
> > ---
> > lib/dpif-netlink.c | 86 ++++++++++++++++++++++++++++++++++++++++++++--
> > lib/ovs-thread.c | 16 +++++++++
> > lib/ovs-thread.h | 1 +
> > 3 files changed, 101 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > index 06e1e8ca0..e948a829b 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -31,6 +31,7 @@
> > #include <sys/epoll.h>
> > #include <sys/stat.h>
> > #include <unistd.h>
> > +#include <math.h>
> >
> > #include "bitmap.h"
> > #include "dpif-netlink-rtnl.h"
> > @@ -2506,6 +2507,87 @@ dpif_netlink_handler_uninit(struct dpif_handler
> > *handler)
> > }
> > #endif
> >
> > +/*
> > + * Returns 1 if num is a prime number,
> > + * Otherwise return 0
> > + */
> > +static uint32_t
> > +is_prime(uint32_t num)
> > +{
> > + 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 UINT32_MAX.
> > + *
> > + * Returns 0 if no prime has been found between num and UINT32_MAX
> > + */
> > +static uint32_t
> > +next_prime(uint32_t num)
> > +{
> > + if (num < 2) {
> > + return 2;
> > + }
> > + if (num == 2) {
> > + return 3;
> > + }
> > + if (num % 2 == 0) {
> > + num++;
> > + }
> > +
> > + uint32_t i;
> > + for (i = num; i < UINT32_MAX; i += 2) {
> > + if (is_prime(i)) {
> > + return i;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * 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 && total_cores > 2) {
> > + next_prime_num = next_prime(n_handlers + 1);
> > + 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)
> > @@ -2515,7 +2597,7 @@ dpif_netlink_refresh_handlers_cpu_dispatch(struct
> > dpif_netlink *dpif)
> > uint32_t n_handlers;
> > uint32_t *upcall_pids;
> >
> > - n_handlers = count_cpu_cores();
> > + n_handlers = dpif_netlink_calculate_handlers_num();
> > if (dpif->n_handlers != n_handlers) {
> > VLOG_DBG("Dispatch mode(per-cpu): initializing %d handlers",
> > n_handlers);
> > @@ -2755,7 +2837,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..2172b3d3f 100644
> > --- a/lib/ovs-thread.c
> > +++ b/lib/ovs-thread.c
> > @@ -663,6 +663,22 @@ 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 '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..aac5e19c9 100644
> > --- a/lib/ovs-thread.h
> > +++ b/lib/ovs-thread.h
> > @@ -522,6 +522,7 @@ bool may_fork(void);
> > /* Useful functions related to threading. */
> >
> > int count_cpu_cores(void);
> > +int count_total_cores(void);
> > bool thread_is_pmd(void);
> >
> > #endif /* ovs-thread.h */
> > --
> > 2.33.1
> >
> > _______________________________________________
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
