diff mbox series

[ovs-dev,v3] Double postponing to free subtables.

Message ID 1556017465-18600-1-git-send-email-fuzhantao@huawei.com
State Accepted
Commit 0fcf0776c72aafbf4b5f7ae01de68e1698472490
Headers show
Series [ovs-dev,v3] Double postponing to free subtables. | expand

Commit Message

fuzhantao April 23, 2019, 11:04 a.m. UTC 
From: Zhantao Fu <fuzhantao@huawei.com>

Subtable destruction should be double postponed because readers could still obtain old values while iterating over pvector implementation before its new version published.

---
change since v2:
- use git send-email to re-send patch

change since v1:
- delete the unused parameter in code, modify the subject  and commit-message.

Signed-off-by: Zhantao Fu <fuzhantao@huawei.com>
---
 lib/classifier.c  | 31 +++++++++++++++++++------------
 lib/dpif-netdev.c | 10 ++++++++--
 2 files changed, 27 insertions(+), 14 deletions(-)

Comments

0-day Robot April 23, 2019, 12:50 p.m. UTC | #1 
Bleep bloop.  Greetings fuzhantao, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Zhantao Fu <fuzhantao@huawei.com> needs to sign off.
Lines checked: 96, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Ben Pfaff April 23, 2019, 4:17 p.m. UTC | #2 
On Tue, Apr 23, 2019 at 07:04:25PM +0800, fuzhantao wrote:
> From: Zhantao Fu <fuzhantao@huawei.com>
> 
> Subtable destruction should be double postponed because readers could still obtain old values while iterating over pvector implementation before its new version published.
> 
> ---
> change since v2:
> - use git send-email to re-send patch
> 
> change since v1:
> - delete the unused parameter in code, modify the subject  and commit-message.
> 
> Signed-off-by: Zhantao Fu <fuzhantao@huawei.com>

Thanks, I applied this to master.

(The Signed-off-by should go before the ---.  I took care of the problem
before I applied it.)
diff mbox series

Patch

diff --git a/lib/classifier.c b/lib/classifier.c
index edb40c8..0fad953 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1468,6 +1468,24 @@  miniflow_get_map_in_range(const struct miniflow *miniflow, uint8_t start,
     return map;
 }
 
+static void
+subtable_destroy_cb(struct cls_subtable *subtable)
+{
+    int i;
+
+    ovs_assert(ovsrcu_get_protected(struct trie_node *, &subtable->ports_trie)
+               == NULL);
+    ovs_assert(cmap_is_empty(&subtable->rules));
+    ovs_assert(rculist_is_empty(&subtable->rules_list));
+
+    for (i = 0; i < subtable->n_indices; i++) {
+        ccmap_destroy(&subtable->indices[i]);
+    }
+    cmap_destroy(&subtable->rules);
+
+    ovsrcu_postpone(free, subtable);
+}
+
 /* The new subtable will be visible to the readers only after this. */
 static struct cls_subtable *
 insert_subtable(struct classifier *cls, const struct minimask *mask)
@@ -1534,22 +1552,11 @@  insert_subtable(struct classifier *cls, const struct minimask *mask)
 static void
 destroy_subtable(struct classifier *cls, struct cls_subtable *subtable)
 {
-    int i;
-
     pvector_remove(&cls->subtables, subtable);
     cmap_remove(&cls->subtables_map, &subtable->cmap_node,
                 minimask_hash(&subtable->mask, 0));
 
-    ovs_assert(ovsrcu_get_protected(struct trie_node *, &subtable->ports_trie)
-               == NULL);
-    ovs_assert(cmap_is_empty(&subtable->rules));
-    ovs_assert(rculist_is_empty(&subtable->rules_list));
-
-    for (i = 0; i < subtable->n_indices; i++) {
-        ccmap_destroy(&subtable->indices[i]);
-    }
-    cmap_destroy(&subtable->rules);
-    ovsrcu_postpone(free, subtable);
+    ovsrcu_postpone(subtable_destroy_cb, subtable);
 }
 
 static unsigned int be_get_bit_at(const ovs_be32 value[], unsigned int ofs);
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4d6d0c3..8657a61 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7592,6 +7592,13 @@  struct dpcls_subtable {
     /* 'mask' must be the last field, additional space is allocated here. */
 };
 
+static void
+dpcls_subtable_destroy_cb(struct dpcls_subtable *subtable)
+{
+    cmap_destroy(&subtable->rules);
+    ovsrcu_postpone(free, subtable);
+}
+
 /* Initializes 'cls' as a classifier that initially contains no classification
  * rules. */
 static void
@@ -7608,8 +7615,7 @@  dpcls_destroy_subtable(struct dpcls *cls, struct dpcls_subtable *subtable)
     pvector_remove(&cls->subtables, subtable);
     cmap_remove(&cls->subtables_map, &subtable->cmap_node,
                 subtable->mask.hash);
-    cmap_destroy(&subtable->rules);
-    ovsrcu_postpone(free, subtable);
+    ovsrcu_postpone(dpcls_subtable_destroy_cb, subtable);
 }
 
 /* Destroys 'cls'.  Rules within 'cls', if any, are not freed; this is the