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 dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev