From patchwork Wed Nov 2 04:09:07 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1698008 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4N2D203pKqz23lk for ; Wed, 2 Nov 2022 15:10:15 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id D10E3410A7; Wed, 2 Nov 2022 04:10:11 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org D10E3410A7 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UsPgdRSbTXvD; Wed, 2 Nov 2022 04:10:10 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 8F773410A4; Wed, 2 Nov 2022 04:10:09 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 8F773410A4 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D536DC0032; Wed, 2 Nov 2022 04:10:08 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id ED18BC002D for ; Wed, 2 Nov 2022 04:10:07 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id B325281429 for ; Wed, 2 Nov 2022 04:10:07 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org B325281429 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 kDBplyU_pvE9 for ; Wed, 2 Nov 2022 04:10:06 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 0F4648140D Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::225]) by smtp1.osuosl.org (Postfix) with ESMTPS id 0F4648140D for ; Wed, 2 Nov 2022 04:10:05 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id C93511C0007; Wed, 2 Nov 2022 04:10:01 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 1 Nov 2022 21:09:07 -0700 Message-Id: <20221102040908.3292152-1-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Cc: Ilya Maximets Subject: [ovs-dev] [PATCH 1/2] ovsdb/transaction.c: Refactor assess_weak_refs. 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" The loops for adding weak refs are quite similar. Abstract to a function, which will be used by one more cases later. The patch also changes the txn_row arg to the source row. Signed-off-by: Han Zhou --- ovsdb/transaction.c | 77 +++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 42 deletions(-) diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index bb997b45b5d1..1c418349429d 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -587,7 +587,7 @@ ovsdb_txn_update_weak_refs(struct ovsdb_txn *txn OVS_UNUSED, } static void -add_weak_ref(struct ovsdb_txn_row *txn_row, const struct ovsdb_row *dst_, +add_weak_ref(const struct ovsdb_row *src, const struct ovsdb_row *dst_, struct ovs_list *ref_list, const union ovsdb_atom *key, const union ovsdb_atom *value, bool by_key, const struct ovsdb_column *column) @@ -595,13 +595,13 @@ add_weak_ref(struct ovsdb_txn_row *txn_row, const struct ovsdb_row *dst_, struct ovsdb_row *dst = CONST_CAST(struct ovsdb_row *, dst_); struct ovsdb_weak_ref *weak; - if (txn_row->new == dst) { + if (src == dst) { return; } weak = xzalloc(sizeof *weak); - weak->src_table = txn_row->new->table; - weak->src = *ovsdb_row_get_uuid(txn_row->new); + weak->src_table = src->table; + weak->src = *ovsdb_row_get_uuid(src); weak->dst_table = dst->table; weak->dst = *ovsdb_row_get_uuid(dst); ovsdb_type_clone(&weak->type, &column->type); @@ -616,7 +616,7 @@ add_weak_ref(struct ovsdb_txn_row *txn_row, const struct ovsdb_row *dst_, } static void -find_and_add_weak_ref(struct ovsdb_txn_row *txn_row, +find_and_add_weak_ref(const struct ovsdb_row *src, const union ovsdb_atom *key, const union ovsdb_atom *value, const struct ovsdb_column *column, @@ -628,7 +628,7 @@ find_and_add_weak_ref(struct ovsdb_txn_row *txn_row, : ovsdb_table_get_row(column->type.value.uuid.refTable, &value->uuid); if (row) { - add_weak_ref(txn_row, row, ref_list, key, value, by_key, column); + add_weak_ref(src, row, ref_list, key, value, by_key, column); } else if (not_found) { if (uuid_is_zero(by_key ? &key->uuid : &value->uuid)) { *zero = true; @@ -637,6 +637,30 @@ find_and_add_weak_ref(struct ovsdb_txn_row *txn_row, } } +static void +find_and_add_weak_refs(const struct ovsdb_row *src, + const struct ovsdb_datum *datum, + const struct ovsdb_column *column, + struct ovs_list *ref_list, + struct ovsdb_datum *not_found, bool *zero) +{ + unsigned int i; + if (ovsdb_base_type_is_weak_ref(&column->type.key)) { + for (i = 0; i < datum->n; i++) { + find_and_add_weak_ref(src, &datum->keys[i], + datum->values ? &datum->values[i] : NULL, + column, true, ref_list, not_found, zero); + } + } + + if (ovsdb_base_type_is_weak_ref(&column->type.value)) { + for (i = 0; i < datum->n; i++) { + find_and_add_weak_ref(src, &datum->keys[i], &datum->values[i], + column, false, ref_list, not_found, zero); + } + } +} + static struct ovsdb_error * OVS_WARN_UNUSED_RESULT assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) { @@ -678,7 +702,7 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) const struct ovsdb_column *column = node->data; struct ovsdb_datum *datum = &txn_row->new->fields[column->index]; struct ovsdb_datum added, removed, deleted_refs; - unsigned int orig_n, i; + unsigned int orig_n; bool zero = false; orig_n = datum->n; @@ -712,23 +736,8 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) /* Checking added data and creating new references. */ ovsdb_datum_init_empty(&deleted_refs); - if (ovsdb_base_type_is_weak_ref(&column->type.key)) { - for (i = 0; i < added.n; i++) { - find_and_add_weak_ref(txn_row, &added.keys[i], - added.values ? &added.values[i] : NULL, - column, true, &txn_row->added_refs, - &deleted_refs, &zero); - } - } - - if (ovsdb_base_type_is_weak_ref(&column->type.value)) { - for (i = 0; i < added.n; i++) { - find_and_add_weak_ref(txn_row, &added.keys[i], - &added.values[i], - column, false, &txn_row->added_refs, - &deleted_refs, &zero); - } - } + find_and_add_weak_refs(txn_row->new, &added, column, + &txn_row->added_refs, &deleted_refs, &zero); if (deleted_refs.n) { /* Removing all the references that doesn't point to valid rows. */ ovsdb_datum_sort_unique(&deleted_refs, &column->type); @@ -741,24 +750,8 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) /* Creating refs that needs to be removed on commit. This includes * both: the references that got directly removed from the datum and * references removed due to deletion of a referenced row. */ - if (ovsdb_base_type_is_weak_ref(&column->type.key)) { - for (i = 0; i < removed.n; i++) { - find_and_add_weak_ref(txn_row, &removed.keys[i], - removed.values - ? &removed.values[i] : NULL, - column, true, &txn_row->deleted_refs, - NULL, NULL); - } - } - - if (ovsdb_base_type_is_weak_ref(&column->type.value)) { - for (i = 0; i < removed.n; i++) { - find_and_add_weak_ref(txn_row, &removed.keys[i], - &removed.values[i], - column, false, &txn_row->deleted_refs, - NULL, NULL); - } - } + find_and_add_weak_refs(txn_row->new, &removed, column, + &txn_row->deleted_refs, NULL, NULL); ovsdb_datum_destroy(&removed, &column->type); if (datum->n != orig_n) { From patchwork Wed Nov 2 04:09:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1698009 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4N2D211R27z23lm for ; Wed, 2 Nov 2022 15:10:16 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 5F0D0408B8; Wed, 2 Nov 2022 04:10:14 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 5F0D0408B8 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KcEoWaCd_Q3r; Wed, 2 Nov 2022 04:10:13 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp2.osuosl.org (Postfix) with ESMTPS id 39A094062A; Wed, 2 Nov 2022 04:10:12 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 39A094062A Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1D8F0C007E; Wed, 2 Nov 2022 04:10:10 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1A272C002D for ; Wed, 2 Nov 2022 04:10:08 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id E79C440160 for ; Wed, 2 Nov 2022 04:10:07 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org E79C440160 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id kSXozAPr1euf for ; Wed, 2 Nov 2022 04:10:06 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 95D6E4012D Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp2.osuosl.org (Postfix) with ESMTPS id 95D6E4012D for ; Wed, 2 Nov 2022 04:10:06 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by mail.gandi.net (Postfix) with ESMTPSA id A079E1C0008; Wed, 2 Nov 2022 04:10:03 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Tue, 1 Nov 2022 21:09:08 -0700 Message-Id: <20221102040908.3292152-2-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221102040908.3292152-1-hzhou@ovn.org> References: <20221102040908.3292152-1-hzhou@ovn.org> MIME-Version: 1.0 Cc: Ilya Maximets Subject: [ovs-dev] [PATCH 2/2] ovsdb/transaction.c: Fix weak reference leak. 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" When a row is deleted, if the row has weak references to other rows, the weak reference nodes attached to the destination rows (through weak->dst_node hmap) are not destroyed. Deleting weak references is properly handled when a row is modified. The removed references are taken care by: 1. assess_weak_refs() figures out the deleted references from the row and add them to txn_row->deleted_refs. 2. before commit, in ovsdb_txn_update_weak_refs() it finds the destination row for each item in txn_row->deleted_refs (from step 1), and destroy the corresponding weak references of the destionation row. However, when the row is deleted, the step 1 in assess_weak_refs() is missing. It directly returns without adding the deleted references to txn_row->deleted_refs. So, the detination nodes will keep those weak references although the source side of the references are already deleted. When such rows that originating weak references are created and deleted, more and more such useless weak reference structures accumulate in the memory, and can stay there until the destination rows are deleted. It is possible that the detination row is never deleted, and in such case the ovsdb-server memory keeps growing (although it is not strictly memory leak, because the structures are still referenced). This problem has an impact to applications like OVN SB DB - the memory grows very fast in long-runing deployments and finally causes OOM. This patch fixes it by generating deleted_refs for deleted rows in assess_weak_refs(). Fixes: 4dbff9f0a685 ("ovsdb: transaction: Incremental reassessment of weak refs.") Signed-off-by: Han Zhou --- ovsdb/transaction.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index 1c418349429d..b0c0c311c0ef 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -665,7 +665,7 @@ static struct ovsdb_error * OVS_WARN_UNUSED_RESULT assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) { struct ovsdb_weak_ref *weak; - struct ovsdb_table *table; + struct ovsdb_table *table = txn_row->table; struct shash_node *node; if (txn_row->old && !txn_row->new) { @@ -687,6 +687,14 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) ovs_assert(ovs_list_is_empty(&weak->src_node)); ovs_list_insert(&src_txn_row->deleted_refs, &weak->src_node); } + + /* Creating refs that needs to be removed on commit. */ + SHASH_FOR_EACH (node, &table->schema->columns) { + const struct ovsdb_column *column = node->data; + struct ovsdb_datum *datum = &txn_row->old->fields[column->index]; + find_and_add_weak_refs(txn_row->old, datum, column, + &txn_row->deleted_refs, NULL, NULL); + } } if (!txn_row->new) { @@ -697,7 +705,6 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) return NULL; } - table = txn_row->table; SHASH_FOR_EACH (node, &table->schema->columns) { const struct ovsdb_column *column = node->data; struct ovsdb_datum *datum = &txn_row->new->fields[column->index];