diff mbox series

[ovs-dev,v2] dpif-netdev: Do not create handler threads.

Message ID adca60435549e40886484de485769097ec0d0a05.1708421494.git.echaudro@redhat.com
State Accepted
Commit f0d1beca6cbb7a27cd6b854bc975f26da0781955
Headers show
Series [ovs-dev,v2] dpif-netdev: Do not create handler threads. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Eelco Chaudron Feb. 20, 2024, 9:31 a.m. UTC 
Avoid unnecessary thread creation as no upcalls are generated,
resulting in idle threads waiting for process termination.

This optimization significantly reduces memory usage, cutting it
by half on a 128 CPU/thread system during testing, with the number
of threads reduced from 95 to 0.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---

v2: Sending this as a separate patch.

 lib/dpif-netdev.c             | 10 +++++++++-
 ofproto/ofproto-dpif-upcall.c | 25 ++++++++++++++++---------
 2 files changed, 25 insertions(+), 10 deletions(-)

Comments

Mike Pattrick Feb. 20, 2024, 4:50 p.m. UTC | #1 
On Tue, Feb 20, 2024 at 4:32 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>
> Avoid unnecessary thread creation as no upcalls are generated,
> resulting in idle threads waiting for process termination.
>
> This optimization significantly reduces memory usage, cutting it
> by half on a 128 CPU/thread system during testing, with the number
> of threads reduced from 95 to 0.
>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---

I tested this out a bit, and it seems to work well.

Acked-by: Mike Pattrick <mkp@redhat.com>
Eelco Chaudron Feb. 29, 2024, 12:29 p.m. UTC | #2 
On 20 Feb 2024, at 17:50, Mike Pattrick wrote:

> On Tue, Feb 20, 2024 at 4:32 AM Eelco Chaudron <echaudro@redhat.com> wrote:
>>
>> Avoid unnecessary thread creation as no upcalls are generated,
>> resulting in idle threads waiting for process termination.
>>
>> This optimization significantly reduces memory usage, cutting it
>> by half on a 128 CPU/thread system during testing, with the number
>> of threads reduced from 95 to 0.
>>
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
>
> I tested this out a bit, and it seems to work well.
>
> Acked-by: Mike Pattrick <mkp@redhat.com>

Thanks Mike for the review, applied to the main branch.

//Eelco
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c1981137f..1a29ab7ac 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5250,6 +5250,14 @@  dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
     return 0;
 }
 
+static bool
+dpif_netdev_number_handlers_required(struct dpif *dpif_ OVS_UNUSED,
+                                     uint32_t *n_handlers)
+{
+    *n_handlers = 0;
+    return true;
+}
+
 /* Parses affinity list and returns result in 'core_ids'. */
 static int
 parse_affinity_list(const char *affinity_list, unsigned *core_ids, int n_rxq)
@@ -9985,7 +9993,7 @@  const struct dpif_class dpif_netdev_class = {
     dpif_netdev_offload_stats_get,
     NULL,                       /* recv_set */
     NULL,                       /* handlers_set */
-    NULL,                       /* number_handlers_required */
+    dpif_netdev_number_handlers_required,
     dpif_netdev_set_config,
     dpif_netdev_queue_to_priority,
     NULL,                       /* recv */
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index b5cbeed87..9a5c5c29c 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -584,7 +584,7 @@  static void
 udpif_start_threads(struct udpif *udpif, uint32_t n_handlers_,
                     uint32_t n_revalidators_)
 {
-    if (udpif && n_handlers_ && n_revalidators_) {
+    if (udpif && n_revalidators_) {
         /* Creating a thread can take a significant amount of time on some
          * systems, even hundred of milliseconds, so quiesce around it. */
         ovsrcu_quiesce_start();
@@ -592,14 +592,19 @@  udpif_start_threads(struct udpif *udpif, uint32_t n_handlers_,
         udpif->n_handlers = n_handlers_;
         udpif->n_revalidators = n_revalidators_;
 
-        udpif->handlers = xzalloc(udpif->n_handlers * sizeof *udpif->handlers);
-        for (size_t i = 0; i < udpif->n_handlers; i++) {
-            struct handler *handler = &udpif->handlers[i];
+        if (udpif->n_handlers) {
+            udpif->handlers = xzalloc(udpif->n_handlers
+                                      * sizeof *udpif->handlers);
+            for (size_t i = 0; i < udpif->n_handlers; i++) {
+                struct handler *handler = &udpif->handlers[i];
 
-            handler->udpif = udpif;
-            handler->handler_id = i;
-            handler->thread = ovs_thread_create(
-                "handler", udpif_upcall_handler, handler);
+                handler->udpif = udpif;
+                handler->handler_id = i;
+                handler->thread = ovs_thread_create(
+                    "handler", udpif_upcall_handler, handler);
+            }
+        } else {
+            udpif->handlers = NULL;
         }
 
         atomic_init(&udpif->enable_ufid, udpif->backer->rt_support.ufid);
@@ -662,7 +667,9 @@  udpif_set_threads(struct udpif *udpif, uint32_t n_handlers_,
     if (dpif_number_handlers_required(udpif->dpif, &n_handlers_requested)) {
         forced = true;
         if (!n_revalidators_) {
-            n_revalidators_requested = n_handlers_requested / 4 + 1;
+            n_revalidators_requested = (n_handlers_requested
+                                        ? n_handlers_requested
+                                        : MAX(count_cpu_cores(), 2)) / 4 + 1;
         } else {
             n_revalidators_requested = n_revalidators_;
         }