Hi,
I'm reading through the lib/cmap.c and I wonder if we need an extra lock at
cmap_insert_bfs().
When cmap_insert_bfs() found a path and started to move all elements
backward, it goes through the for loop below and calls cmap_set_bucket()
one-by-one, and eventually place the new_node. However, is it possible that
another writer at the same time inserts an element, which is among one of
the elements in the path, as a result, making the path no longer valid?
Should we introduce another lock like below?
--- a/lib/cmap.c
+++ b/lib/cmap.c
@@ -717,6 +717,7 @@ cmap_insert_bfs(struct cmap_impl *impl, struct
cmap_node *new_node,
* Move all the nodes across the path "backward". After
each
* step some node appears in two buckets. Thus, every
node is
* always visible to a concurrent search. */
+// add a lock() here
for (k = path->n + 1; k > 0; k--) {
int slot = slots[k - 1];
@@ -730,6 +731,7 @@ cmap_insert_bfs(struct cmap_impl *impl, struct
cmap_node *new_node,
* 'new_node'. */
cmap_set_bucket(buckets[0], slots[0], new_node, hash);
+// unlock()
return true;
}
Thanks
William
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev