From patchwork Fri Jun 4 10:00:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1487703 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=JgdHcBSd; dkim-atps=neutral Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FxJF827Rjz9sSn for ; Fri, 4 Jun 2021 20:01:16 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 62B8783DC0; Fri, 4 Jun 2021 10:01:14 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id sYZYVNNJM3VK; Fri, 4 Jun 2021 10:01:10 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp1.osuosl.org (Postfix) with ESMTP id 7B6B383DC4; Fri, 4 Jun 2021 10:01:09 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 4B239C000D; Fri, 4 Jun 2021 10:01:09 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 6452EC0001 for ; Fri, 4 Jun 2021 10:01:07 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 5686D6064E for ; Fri, 4 Jun 2021 10:01:07 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp3.osuosl.org (amavisd-new); dkim=pass (1024-bit key) header.d=redhat.com Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ODV7qQK9Z8ts for ; Fri, 4 Jun 2021 10:01:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id E2BBC6064F for ; Fri, 4 Jun 2021 10:01:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1622800860; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=AfYdAlMxtIaJZs0igVKAoZRx7lHYHLX6NCEjjJP72XQ=; b=JgdHcBSd5waM8xeZ3Oa1NWEezJGTi2swr4Kr7DCeLNhbvlgA++5G0z9JHzrJ/YdqAZkS2g k4zDCTxV6BiimX5PyRsWTmOli+hm/IHfCrKoRieD5vAfem3AE+C544/B/dQptiA7oq/xoc DvZoiThczq0QSaQ29rJ3URsHuwaF014= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-270-J0zSN8HfMJ6s301dN0MIzQ-1; Fri, 04 Jun 2021 06:00:57 -0400 X-MC-Unique: J0zSN8HfMJ6s301dN0MIzQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9235C107ACC7; Fri, 4 Jun 2021 10:00:56 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-64.ams2.redhat.com [10.36.114.64]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7B077E479; Fri, 4 Jun 2021 10:00:50 +0000 (UTC) From: Dumitru Ceara To: ovs-dev@openvswitch.org Date: Fri, 4 Jun 2021 12:00:47 +0200 Message-Id: <20210604100047.24979-1-dceara@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: dceara@redhat.com, i.maximets@ovn.org Subject: [ovs-dev] [PATCH ovn] lflow-cache: Auto trim when cache size is reduced significantly. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Due to a bug in glibc [0], M_TRIM_THRESHOLD (default 128KB) is not honored. The lflow cache is one of the largest memory consumers in ovn-controller and it used to trim memory whenever the cache was flushed. However, that required periodic intervention from the CMS side. Instead, we now automatically trim memory every time the lflow cache utilization decreases by 50%, with a threshold of at least LFLOW_CACHE_TRIM_LIMIT (10000) elements in the cache. [0] https://sourceware.org/bugzilla/show_bug.cgi?id=14827 Reported-at: https://bugzilla.redhat.com/1967882 Signed-off-by: Dumitru Ceara --- controller/lflow-cache.c | 68 ++++++++++++++++++++++------------- tests/ovn-lflow-cache.at | 76 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 25 deletions(-) diff --git a/controller/lflow-cache.c b/controller/lflow-cache.c index 56ddf1075..11935e7ae 100644 --- a/controller/lflow-cache.c +++ b/controller/lflow-cache.c @@ -40,6 +40,7 @@ COVERAGE_DEFINE(lflow_cache_delete); COVERAGE_DEFINE(lflow_cache_full); COVERAGE_DEFINE(lflow_cache_mem_full); COVERAGE_DEFINE(lflow_cache_made_room); +COVERAGE_DEFINE(lflow_cache_trim); static const char *lflow_cache_type_names[LCACHE_T_MAX] = { [LCACHE_T_CONJ_ID] = "cache-conj-id", @@ -49,6 +50,8 @@ static const char *lflow_cache_type_names[LCACHE_T_MAX] = { struct lflow_cache { struct hmap entries[LCACHE_T_MAX]; + uint32_t n_entries; + uint32_t high_watermark; uint32_t capacity; uint64_t mem_usage; uint64_t max_mem_usage; @@ -63,7 +66,8 @@ struct lflow_cache_entry { struct lflow_cache_value value; }; -static size_t lflow_cache_n_entries__(const struct lflow_cache *lc); +#define LFLOW_CACHE_TRIM_LIMIT 10000 + static bool lflow_cache_make_room__(struct lflow_cache *lc, enum lflow_cache_type type); static struct lflow_cache_value *lflow_cache_add__( @@ -71,18 +75,18 @@ static struct lflow_cache_value *lflow_cache_add__( enum lflow_cache_type type, uint64_t value_size); 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); struct lflow_cache * lflow_cache_create(void) { - struct lflow_cache *lc = xmalloc(sizeof *lc); + struct lflow_cache *lc = xzalloc(sizeof *lc); for (size_t i = 0; i < LCACHE_T_MAX; i++) { hmap_init(&lc->entries[i]); } lc->enabled = true; - lc->mem_usage = 0; return lc; } @@ -101,12 +105,8 @@ lflow_cache_flush(struct lflow_cache *lc) HMAP_FOR_EACH_SAFE (lce, lce_next, node, &lc->entries[i]) { lflow_cache_delete__(lc, lce); } - hmap_shrink(&lc->entries[i]); } - -#if HAVE_DECL_MALLOC_TRIM - malloc_trim(0); -#endif + lflow_cache_trim__(lc, true); } void @@ -134,7 +134,7 @@ lflow_cache_enable(struct lflow_cache *lc, bool enabled, uint32_t capacity, uint64_t max_mem_usage = max_mem_usage_kb * 1024; if ((lc->enabled && !enabled) - || capacity < lflow_cache_n_entries__(lc) + || capacity < lc->n_entries || max_mem_usage < lc->mem_usage) { lflow_cache_flush(lc); } @@ -164,6 +164,9 @@ lflow_cache_get_stats(const struct lflow_cache *lc, struct ds *output) ds_put_format(output, "Enabled: %s\n", lflow_cache_is_enabled(lc) ? "true" : "false"); + ds_put_format(output, "%-16s: %"PRIu32"\n", "high-watermark", + lc->high_watermark); + ds_put_format(output, "%-16s: %"PRIu32"\n", "total", lc->n_entries); for (size_t i = 0; i < LCACHE_T_MAX; i++) { ds_put_format(output, "%-16s: %"PRIuSIZE"\n", lflow_cache_type_names[i], @@ -254,20 +257,10 @@ lflow_cache_delete(struct lflow_cache *lc, const struct uuid *lflow_uuid) COVERAGE_INC(lflow_cache_delete); lflow_cache_delete__(lc, CONTAINER_OF(lcv, struct lflow_cache_entry, value)); + lflow_cache_trim__(lc, false); } } -static size_t -lflow_cache_n_entries__(const struct lflow_cache *lc) -{ - size_t n_entries = 0; - - for (size_t i = 0; i < LCACHE_T_MAX; i++) { - n_entries += hmap_count(&lc->entries[i]); - } - return n_entries; -} - static bool lflow_cache_make_room__(struct lflow_cache *lc, enum lflow_cache_type type) { @@ -319,7 +312,7 @@ lflow_cache_add__(struct lflow_cache *lc, const struct uuid *lflow_uuid, return NULL; } - if (lflow_cache_n_entries__(lc) == lc->capacity) { + if (lc->n_entries == lc->capacity) { if (!lflow_cache_make_room__(lc, type)) { COVERAGE_INC(lflow_cache_full); return NULL; @@ -336,17 +329,17 @@ lflow_cache_add__(struct lflow_cache *lc, const struct uuid *lflow_uuid, lce->size = size; lce->value.type = type; hmap_insert(&lc->entries[type], &lce->node, uuid_hash(lflow_uuid)); + lc->n_entries++; + lc->high_watermark = MAX(lc->high_watermark, lc->n_entries); return &lce->value; } static void lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce) { - if (!lce) { - return; - } - + ovs_assert(lc->n_entries > 0); hmap_remove(&lc->entries[lce->value.type], &lce->node); + lc->n_entries--; switch (lce->value.type) { case LCACHE_T_NONE: OVS_NOT_REACHED(); @@ -369,3 +362,28 @@ lflow_cache_delete__(struct lflow_cache *lc, struct lflow_cache_entry *lce) lc->mem_usage -= lce->size; free(lce); } + +static void +lflow_cache_trim__(struct lflow_cache *lc, bool force) +{ + /* Trim if we had at least 'TRIM_LIMIT' elements at some point and if the + * current usage is less than half of 'high_watermark'. + */ + ovs_assert(lc->high_watermark >= lc->n_entries); + if (!force + && (lc->high_watermark <= LFLOW_CACHE_TRIM_LIMIT + || lc->n_entries > lc->high_watermark / 2)) { + return; + } + + COVERAGE_INC(lflow_cache_trim); + for (size_t i = 0; i < LCACHE_T_MAX; i++) { + hmap_shrink(&lc->entries[i]); + } + +#if HAVE_DECL_MALLOC_TRIM + malloc_trim(0); +#endif + + lc->high_watermark = lc->n_entries; +} diff --git a/tests/ovn-lflow-cache.at b/tests/ovn-lflow-cache.at index e5e9ed1e8..df483c9d7 100644 --- a/tests/ovn-lflow-cache.at +++ b/tests/ovn-lflow-cache.at @@ -12,6 +12,8 @@ AT_CHECK( add matches 3 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -21,6 +23,8 @@ LOOKUP: conj_id_ofs: 1 type: conj-id Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 1 cache-expr : 0 cache-matches : 0 @@ -30,6 +34,8 @@ LOOKUP: conj_id_ofs: 2 type: expr Enabled: true +high-watermark : 2 +total : 2 cache-conj-id : 1 cache-expr : 1 cache-matches : 0 @@ -39,6 +45,8 @@ LOOKUP: conj_id_ofs: 0 type: matches Enabled: true +high-watermark : 3 +total : 3 cache-conj-id : 1 cache-expr : 1 cache-matches : 1 @@ -54,6 +62,8 @@ AT_CHECK( add-del matches 3 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -66,6 +76,8 @@ DELETE LOOKUP: not found Enabled: true +high-watermark : 1 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -78,6 +90,8 @@ DELETE LOOKUP: not found Enabled: true +high-watermark : 1 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -90,6 +104,8 @@ DELETE LOOKUP: not found Enabled: true +high-watermark : 1 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -105,6 +121,8 @@ AT_CHECK( add matches 3 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: false +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -113,6 +131,8 @@ ADD conj-id: LOOKUP: not found Enabled: false +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -121,6 +141,8 @@ ADD expr: LOOKUP: not found Enabled: false +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -129,6 +151,8 @@ ADD matches: LOOKUP: not found Enabled: false +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -153,6 +177,8 @@ AT_CHECK( flush | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -162,6 +188,8 @@ LOOKUP: conj_id_ofs: 1 type: conj-id Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 1 cache-expr : 0 cache-matches : 0 @@ -171,6 +199,8 @@ LOOKUP: conj_id_ofs: 2 type: expr Enabled: true +high-watermark : 2 +total : 2 cache-conj-id : 1 cache-expr : 1 cache-matches : 0 @@ -180,11 +210,15 @@ LOOKUP: conj_id_ofs: 0 type: matches Enabled: true +high-watermark : 3 +total : 3 cache-conj-id : 1 cache-expr : 1 cache-matches : 1 DISABLE Enabled: false +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -193,6 +227,8 @@ ADD conj-id: LOOKUP: not found Enabled: false +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -201,6 +237,8 @@ ADD expr: LOOKUP: not found Enabled: false +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -209,11 +247,15 @@ ADD matches: LOOKUP: not found Enabled: false +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 ENABLE Enabled: true +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -223,6 +265,8 @@ LOOKUP: conj_id_ofs: 7 type: conj-id Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 1 cache-expr : 0 cache-matches : 0 @@ -232,6 +276,8 @@ LOOKUP: conj_id_ofs: 8 type: expr Enabled: true +high-watermark : 2 +total : 2 cache-conj-id : 1 cache-expr : 1 cache-matches : 0 @@ -241,11 +287,15 @@ LOOKUP: conj_id_ofs: 0 type: matches Enabled: true +high-watermark : 3 +total : 3 cache-conj-id : 1 cache-expr : 1 cache-matches : 1 FLUSH Enabled: true +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -270,6 +320,8 @@ AT_CHECK( add matches 10 | grep -v 'Mem usage (KB)'], [0], [dnl Enabled: true +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -279,6 +331,8 @@ LOOKUP: conj_id_ofs: 1 type: conj-id Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 1 cache-expr : 0 cache-matches : 0 @@ -288,6 +342,8 @@ LOOKUP: conj_id_ofs: 2 type: expr Enabled: true +high-watermark : 2 +total : 2 cache-conj-id : 1 cache-expr : 1 cache-matches : 0 @@ -297,6 +353,8 @@ LOOKUP: conj_id_ofs: 0 type: matches Enabled: true +high-watermark : 3 +total : 3 cache-conj-id : 1 cache-expr : 1 cache-matches : 1 @@ -305,6 +363,8 @@ dnl dnl Max capacity smaller than current usage, cache should be flushed. dnl Enabled: true +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -314,6 +374,8 @@ LOOKUP: conj_id_ofs: 4 type: conj-id Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 1 cache-expr : 0 cache-matches : 0 @@ -327,6 +389,8 @@ dnl Cache is full but we can evict the conj-id entry because we're adding dnl an expr one. dnl Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 0 cache-expr : 1 cache-matches : 0 @@ -340,6 +404,8 @@ dnl Cache is full but we can evict the expr entry because we're adding dnl a matches one. dnl Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 0 cache-expr : 0 cache-matches : 1 @@ -352,6 +418,8 @@ dnl Cache is full and we're adding a conj-id entry so we shouldn't evict dnl anything else. dnl Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 0 cache-expr : 0 cache-matches : 1 @@ -361,6 +429,8 @@ dnl Max memory usage smaller than current memory usage, cache should be dnl flushed. dnl Enabled: true +high-watermark : 0 +total : 0 cache-conj-id : 0 cache-expr : 0 cache-matches : 0 @@ -370,6 +440,8 @@ LOOKUP: conj_id_ofs: 8 type: conj-id Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 1 cache-expr : 0 cache-matches : 0 @@ -382,6 +454,8 @@ dnl Cache is full and we're adding a cache entry that would go over the max dnl memory limit so adding should fail. dnl Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 1 cache-expr : 0 cache-matches : 0 @@ -394,6 +468,8 @@ dnl Cache is full and we're adding a cache entry that would go over the max dnl memory limit so adding should fail. dnl Enabled: true +high-watermark : 1 +total : 1 cache-conj-id : 1 cache-expr : 0 cache-matches : 0