To do this we factor the memory trimming code out into its own module,
memory-trim.  We use this now for both the lflow cache (in
ovn-controller) and for ovn-northd.

Signed-off-by: Dumitru Ceara <dce...@redhat.com>
---
 controller/lflow-cache.c |   68 +++++---------------------
 lib/automake.mk          |    2 +
 lib/memory-trim.c        |  120 ++++++++++++++++++++++++++++++++++++++++++++++
 lib/memory-trim.h        |   33 +++++++++++++
 northd/inc-proc-northd.c |    4 +-
 northd/inc-proc-northd.h |    2 -
 northd/ovn-northd.c      |   33 ++++++++++++-
 ovn-nb.xml               |    9 +++
 8 files changed, 212 insertions(+), 59 deletions(-)
 create mode 100644 lib/memory-trim.c
 create mode 100644 lib/memory-trim.h

diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c
index 9fca2d7441..9d0e9cd10b 100644
--- a/controller/lflow-cache.c
+++ b/controller/lflow-cache.c
@@ -24,10 +24,9 @@
 #include "coverage.h"
 #include "lflow-cache.h"
 #include "lib/uuid.h"
-#include "openvswitch/poll-loop.h"
+#include "memory-trim.h"
 #include "openvswitch/vlog.h"
 #include "ovn/expr.h"
-#include "timeval.h"
 
 VLOG_DEFINE_THIS_MODULE(lflow_cache);
 
@@ -54,6 +53,7 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
 
 struct lflow_cache {
     struct hmap entries[LCACHE_T_MAX];
+    struct memory_trimmer *mt;
     uint32_t n_entries;
     uint32_t high_watermark;
     uint32_t capacity;
@@ -61,10 +61,7 @@ struct lflow_cache {
     uint64_t max_mem_usage;
     uint32_t trim_limit;
     uint32_t trim_wmark_perc;
-    uint32_t trim_timeout_ms;
     uint64_t trim_count;
-    long long int last_active_ms;
-    bool recently_active;
     bool enabled;
 };
 
@@ -84,7 +81,6 @@ static struct lflow_cache_value *lflow_cache_add__(
 static void lflow_cache_delete__(struct lflow_cache *lc,
                                  struct lflow_cache_entry *lce);
 static void lflow_cache_trim__(struct lflow_cache *lc, bool force);
-static void lflow_cache_record_activity__(struct lflow_cache *lc);
 
 struct lflow_cache *
 lflow_cache_create(void)
@@ -94,6 +90,7 @@ lflow_cache_create(void)
     for (size_t i = 0; i < LCACHE_T_MAX; i++) {
         hmap_init(&lc->entries[i]);
     }
+    lc->mt = memory_trimmer_create();
 
     return lc;
 }
@@ -127,6 +124,7 @@ lflow_cache_destroy(struct lflow_cache *lc)
     for (size_t i = 0; i < LCACHE_T_MAX; i++) {
         hmap_destroy(&lc->entries[i]);
     }
+    memory_trimmer_destroy(lc->mt);
     free(lc);
 }
 
@@ -146,13 +144,6 @@ lflow_cache_enable(struct lflow_cache *lc, bool enabled, 
uint32_t capacity,
         trim_wmark_perc = 100;
     }
 
-    if (trim_timeout_ms < 1000) {
-        VLOG_WARN_RL(&rl, "Invalid requested trim timeout: "
-                     "requested %"PRIu32" ms, using 1000 ms instead",
-                     trim_timeout_ms);
-        trim_timeout_ms = 1000;
-    }
-
     uint64_t max_mem_usage = max_mem_usage_kb * 1024;
     bool need_flush = false;
     bool need_trim = false;
@@ -172,13 +163,13 @@ lflow_cache_enable(struct lflow_cache *lc, bool enabled, 
uint32_t capacity,
     lc->max_mem_usage = max_mem_usage;
     lc->trim_limit = lflow_trim_limit;
     lc->trim_wmark_perc = trim_wmark_perc;
-    lc->trim_timeout_ms = trim_timeout_ms;
+    memory_trimmer_set(lc->mt, trim_timeout_ms);
 
     if (need_flush) {
-        lflow_cache_record_activity__(lc);
+        memory_trimmer_record_activity(lc->mt);
         lflow_cache_flush(lc);
     } else if (need_trim) {
-        lflow_cache_record_activity__(lc);
+        memory_trimmer_record_activity(lc->mt);
         lflow_cache_trim__(lc, false);
     }
 }
@@ -286,7 +277,7 @@ lflow_cache_delete(struct lflow_cache *lc, const struct 
uuid *lflow_uuid)
         lflow_cache_delete__(lc, CONTAINER_OF(lcv, struct lflow_cache_entry,
                                               value));
         lflow_cache_trim__(lc, false);
-        lflow_cache_record_activity__(lc);
+        memory_trimmer_record_activity(lc->mt);
     }
 }
 
@@ -327,41 +318,15 @@ lflow_cache_get_memory_usage(const struct lflow_cache 
*lc, struct simap *usage)
 void
 lflow_cache_run(struct lflow_cache *lc)
 {
-    if (!lc->recently_active) {
-        return;
-    }
-
-    long long int now = time_msec();
-    if (now < lc->last_active_ms || now < lc->trim_timeout_ms) {
-        VLOG_WARN_RL(&rl, "Detected cache last active timestamp overflow");
-        lc->recently_active = false;
-        lflow_cache_trim__(lc, true);
-        return;
-    }
-
-    if (now < lc->trim_timeout_ms) {
-        VLOG_WARN_RL(&rl, "Detected very large trim timeout: "
-                     "now %lld ms timeout %"PRIu32" ms",
-                     now, lc->trim_timeout_ms);
-        return;
-    }
-
-    if (now - lc->trim_timeout_ms >= lc->last_active_ms) {
-        VLOG_INFO_RL(&rl, "Detected cache inactivity "
-                    "(last active %lld ms ago): trimming cache",
-                    now - lc->last_active_ms);
+    if (memory_trimmer_run(lc->mt)) {
         lflow_cache_trim__(lc, true);
-        lc->recently_active = false;
     }
 }
 
 void
 lflow_cache_wait(struct lflow_cache *lc)
 {
-    if (!lc->recently_active) {
-        return;
-    }
-    poll_timer_wait_until(lc->last_active_ms + lc->trim_timeout_ms);
+    memory_trimmer_wait(lc->mt);
 }
 
 static struct lflow_cache_value *
@@ -388,7 +353,7 @@ lflow_cache_add__(struct lflow_cache *lc, const struct uuid 
*lflow_uuid,
         }
     }
 
-    lflow_cache_record_activity__(lc);
+    memory_trimmer_record_activity(lc->mt);
     lc->mem_usage += size;
 
     COVERAGE_INC(lflow_cache_add);
@@ -447,17 +412,8 @@ lflow_cache_trim__(struct lflow_cache *lc, bool force)
         hmap_shrink(&lc->entries[i]);
     }
 
-#if HAVE_DECL_MALLOC_TRIM
-    malloc_trim(0);
-#endif
+    memory_trimmer_trim(lc->mt);
 
     lc->high_watermark = lc->n_entries;
     lc->trim_count++;
 }
-
-static void
-lflow_cache_record_activity__(struct lflow_cache *lc)
-{
-    lc->last_active_ms = time_msec();
-    lc->recently_active = true;
-}
diff --git a/lib/automake.mk b/lib/automake.mk
index 15d4f84bbb..b69e854b0b 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -30,6 +30,8 @@ lib_libovn_la_SOURCES = \
        lib/mac-binding-index.h \
        lib/mcast-group-index.c \
        lib/mcast-group-index.h \
+       lib/memory-trim.c \
+       lib/memory-trim.h \
        lib/lex.c \
        lib/objdep.c \
        lib/objdep.h \
diff --git a/lib/memory-trim.c b/lib/memory-trim.c
new file mode 100644
index 0000000000..a6791073ee
--- /dev/null
+++ b/lib/memory-trim.c
@@ -0,0 +1,120 @@
+/*
+ * Copyright (c) 2022, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+#include <config.h>
+
+#if HAVE_DECL_MALLOC_TRIM
+#include <malloc.h>
+#endif
+
+#include "memory-trim.h"
+
+#include "openvswitch/poll-loop.h"
+#include "openvswitch/vlog.h"
+#include "timeval.h"
+
+VLOG_DEFINE_THIS_MODULE(memory_trim);
+
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+
+struct memory_trimmer {
+    uint32_t trim_timeout_ms;
+    long long int last_active_ms;
+    bool recently_active;
+};
+
+struct memory_trimmer *
+memory_trimmer_create(void)
+{
+    struct memory_trimmer *mt = xzalloc(sizeof *mt);
+    return mt;
+}
+
+void
+memory_trimmer_destroy(struct memory_trimmer *mt)
+{
+    free(mt);
+}
+
+void
+memory_trimmer_set(struct memory_trimmer *mt, uint32_t trim_timeout_ms)
+{
+    if (trim_timeout_ms < 1000) {
+        VLOG_WARN_RL(&rl, "Invalid requested trim timeout: "
+                     "requested %"PRIu32" ms, using 1000 ms instead",
+                     trim_timeout_ms);
+        trim_timeout_ms = 1000;
+    }
+    mt->trim_timeout_ms = trim_timeout_ms;
+}
+
+/* Returns true if trimming due to inactivity should happen. */
+bool
+memory_trimmer_run(struct memory_trimmer *mt)
+{
+    if (!mt->recently_active) {
+        return false;
+    }
+
+    long long int now = time_msec();
+    if (now < mt->last_active_ms || now < mt->trim_timeout_ms) {
+        VLOG_WARN_RL(&rl, "Detected last active timestamp overflow");
+        mt->recently_active = false;
+        return true;
+    }
+
+    if (now < mt->trim_timeout_ms) {
+        VLOG_WARN_RL(&rl, "Detected very large trim timeout: "
+                     "now %lld ms timeout %"PRIu32" ms",
+                     now, mt->trim_timeout_ms);
+        return false;
+    }
+
+    if (now - mt->trim_timeout_ms >= mt->last_active_ms) {
+        VLOG_INFO_RL(&rl, "Detected inactivity "
+                    "(last active %lld ms ago): trimming memory",
+                    now - mt->last_active_ms);
+        mt->recently_active = false;
+        return true;
+    }
+
+    return false;
+}
+
+void
+memory_trimmer_wait(struct memory_trimmer *mt)
+{
+    if (!mt->recently_active) {
+        return;
+    }
+    poll_timer_wait_until(mt->last_active_ms + mt->trim_timeout_ms);
+}
+
+void
+memory_trimmer_trim(struct memory_trimmer *mt OVS_UNUSED)
+{
+#if HAVE_DECL_MALLOC_TRIM
+        malloc_trim(0);
+#endif
+}
+
+void
+memory_trimmer_record_activity(struct memory_trimmer *mt)
+{
+    mt->last_active_ms = time_msec();
+    mt->recently_active = true;
+}
diff --git a/lib/memory-trim.h b/lib/memory-trim.h
new file mode 100644
index 0000000000..2bf30292f0
--- /dev/null
+++ b/lib/memory-trim.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2022, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef MEMORY_TRIM_H
+#define MEMORY_TRIM_H 1
+
+#include <stdbool.h>
+#include <stdint.h>
+
+struct memory_trimmer;
+
+struct memory_trimmer *memory_trimmer_create(void);
+void memory_trimmer_destroy(struct memory_trimmer *);
+void memory_trimmer_set(struct memory_trimmer *, uint32_t trim_timeout_ms);
+bool memory_trimmer_run(struct memory_trimmer *);
+void memory_trimmer_wait(struct memory_trimmer *);
+void memory_trimmer_trim(struct memory_trimmer *);
+void memory_trimmer_record_activity(struct memory_trimmer *);
+
+#endif /* lib/memory-trim.h */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 363e384bd1..d23993a551 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -308,7 +308,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
                                 sbrec_address_set_by_name);
 }
 
-void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
+/* Returns true if the incremental processing ended up updating nodes. */
+bool inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
                          struct ovsdb_idl_txn *ovnsb_txn,
                          bool recompute) {
     engine_init_run();
@@ -347,6 +348,7 @@ void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
     } else {
         engine_set_force_recompute(false);
     }
+    return engine_has_updated();
 }
 
 void inc_proc_northd_cleanup(void)
diff --git a/northd/inc-proc-northd.h b/northd/inc-proc-northd.h
index 1300253791..9b81c7ee03 100644
--- a/northd/inc-proc-northd.h
+++ b/northd/inc-proc-northd.h
@@ -8,7 +8,7 @@
 
 void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
                           struct ovsdb_idl_loop *sb);
-void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
+bool inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
                          struct ovsdb_idl_txn *ovnsb_txn,
                          bool recompute);
 void inc_proc_northd_cleanup(void);
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 3a575b02a5..081081f9e4 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -25,6 +25,7 @@
 #include "inc-proc-northd.h"
 #include "lib/ip-mcast-index.h"
 #include "lib/mcast-group-index.h"
+#include "lib/memory-trim.h"
 #include "memory.h"
 #include "northd.h"
 #include "ovs-numa.h"
@@ -727,6 +728,34 @@ run_idl_loop(struct ovsdb_idl_loop *idl_loop, const char 
*name)
     return txn;
 }
 
+#define DEFAULT_NORTHD_TRIM_TO_MS 30000
+
+static void
+run_memory_trimmer(struct ovsdb_idl *ovnnb_idl, bool activity)
+{
+    static struct memory_trimmer *mt = NULL;
+
+    if (!mt) {
+        mt = memory_trimmer_create();
+    }
+
+    const struct nbrec_nb_global *nb = nbrec_nb_global_first(ovnnb_idl);
+    if (nb) {
+        memory_trimmer_set(mt, smap_get_uint(&nb->options,
+                                             "northd_trim_timeout",
+                                             DEFAULT_NORTHD_TRIM_TO_MS));
+    }
+
+    if (activity) {
+        memory_trimmer_record_activity(mt);
+    }
+
+    if (memory_trimmer_run(mt)) {
+        memory_trimmer_trim(mt);
+    }
+    memory_trimmer_wait(mt);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -905,7 +934,8 @@ main(int argc, char *argv[])
 
             if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) {
                 int64_t loop_start_time = time_wall_msec();
-                inc_proc_northd_run(ovnnb_txn, ovnsb_txn, recompute);
+                bool activity = inc_proc_northd_run(ovnnb_txn, ovnsb_txn,
+                                                    recompute);
                 recompute = false;
                 if (ovnsb_txn) {
                     check_and_add_supported_dhcp_opts_to_sb_db(
@@ -937,6 +967,7 @@ main(int argc, char *argv[])
                               "force recompute next time.");
                     recompute = true;
                 }
+                run_memory_trimmer(ovnnb_idl_loop.idl, activity);
             } else {
                 /* Make sure we send any pending requests, e.g., lock. */
                 ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop);
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 4cceef14e0..d14d83dec0 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -202,6 +202,15 @@
         </p>
       </column>
 
+      <column name="options" key="northd_trim_timeout">
+        <p>
+          When used, this configuration value specifies the time, in
+          milliseconds, since the last <code>ovn-northd</code> active operation
+          after which memory trimming is performed.  By default this is set to
+          30000 (30 seconds).
+        </p>
+      </column>
+
       <column name="options" key="use_logical_dp_groups">
         <p>
           Note: This option is deprecated, the only behavior is to always

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to