cmap: New macro CMAP_INITIALIZER, for initializing an empty cmap.

Sometimes code is much simpler if we can statically initialize data
structures.  Until now, this has not been possible for cmap-based data
structures, so this commit introduces a CMAP_INITIALIZER macro.

This works by adding a singleton empty cmap_impl that simply forces the
first insertion into any cmap that points to it to allocate a real
cmap_impl.  There could be some risk that rogue code modifies the
singleton, so for safety it is also marked 'const' to allow the linker to
put it into a read-only page.

This adds a new OVS_ALIGNED_VAR macro with GCC and MSVC implementations.
The latter is based on Microsoft webpages, so developers who know Windows
might want to scrutinize it.

As examples of the kind of simplification this can make possible, this
commit removes an initialization function from ofproto-dpif-rid.c and a
call to cmap_init() from tnl-neigh-cache.c.  An upcoming commit will add
another user.

CC: Jarno Rajahalme <jarno@ovn.org>
CC: Gurucharan Shetty <guru@ovn.org>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Ryan Moats <rmoats@us.ibm.com>

diff --git a/include/openvswitch/compiler.h b/include/openvswitch/compiler.h
index 42c864f..6e779f3 100644 (file)
--- a/include/openvswitch/compiler.h
+++ b/include/openvswitch/compiler.h
@@ -183,18 +183,23 @@
 #define OVS_PACKED(DECL) __pragma(pack(push, 1)) DECL __pragma(pack(pop))
 #endif
 
-/* For defining a structure whose instances should aligned on an N-byte
- * boundary.
- *
- * e.g. The following:
+/* OVS_ALIGNED_STRUCT may be used to define a structure whose instances should
+ * aligned on an N-byte boundary.  This:
  *     OVS_ALIGNED_STRUCT(64, mystruct) { ... };
  * is equivalent to the following except that it specifies 64-byte alignment:
  *     struct mystruct { ... };
+ *
+ * OVS_ALIGNED_VAR defines a variable aligned on an N-byte boundary.  For
+ * example,
+ *     OVS_ALIGNED_VAR(64) int x;
+ * defines a "int" variable that is aligned on a 64-byte boundary.
  */
 #ifndef _MSC_VER
 #define OVS_ALIGNED_STRUCT(N, TAG) struct __attribute__((aligned(N))) TAG
+#define OVS_ALIGNED_VAR(N) __attribute__((aligned(N)))
 #else
 #define OVS_ALIGNED_STRUCT(N, TAG) __declspec(align(N)) struct TAG
+#define OVS_ALIGNED_VAR(N) __declspec(align(N))
 #endif
 
 /* Supplies code to be run at startup time before invoking main().

diff --git a/lib/cmap.c b/lib/cmap.c
index 7a54ea6..8c7312d 100644 (file)
--- a/lib/cmap.c
+++ b/lib/cmap.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014 Nicira, Inc.
+ * Copyright (c) 2014, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -170,13 +170,13 @@ struct cmap_impl {
     unsigned int min_n;         /* Min elements before shrinking. */
     uint32_t mask;              /* Number of 'buckets', minus one. */
     uint32_t basis;             /* Basis for rehashing client's hash values. */
-
-    /* Padding to make cmap_impl exactly one cache line long. */
-    uint8_t pad[CACHE_LINE_SIZE - sizeof(unsigned int) * 5];
-
-    struct cmap_bucket buckets[];
+    uint8_t pad[CACHE_LINE_SIZE - 4 * 5]; /* Pad to end of cache line. */
+    struct cmap_bucket buckets[1];
 };
-BUILD_ASSERT_DECL(sizeof(struct cmap_impl) == CACHE_LINE_SIZE);
+BUILD_ASSERT_DECL(sizeof(struct cmap_impl) == CACHE_LINE_SIZE * 2);
+
+/* An empty cmap. */
+OVS_ALIGNED_VAR(CACHE_LINE_SIZE) const struct cmap_impl empty_cmap;
 
 static struct cmap_impl *cmap_rehash(struct cmap *, uint32_t mask);
 
@@ -226,8 +226,9 @@ cmap_impl_create(uint32_t mask)
 
     ovs_assert(is_pow2(mask + 1));
 
-    impl = xzalloc_cacheline(sizeof *impl
-                             + (mask + 1) * sizeof *impl->buckets);
+    /* There are 'mask + 1' buckets but struct cmap_impl has one bucket built
+     * in, so we only need to add space for the extra 'mask' buckets. */
+    impl = xzalloc_cacheline(sizeof *impl + mask * sizeof *impl->buckets);
     impl->n = 0;
     impl->max_n = calc_max_n(mask);
     impl->min_n = calc_min_n(mask);
@@ -241,7 +242,7 @@ cmap_impl_create(uint32_t mask)
 void
 cmap_init(struct cmap *cmap)
 {
-    ovsrcu_set(&cmap->impl, cmap_impl_create(0));
+    ovsrcu_set(&cmap->impl, CONST_CAST(struct cmap_impl *, &empty_cmap));
 }
 
 /* Destroys 'cmap'.
@@ -252,7 +253,10 @@ void
 cmap_destroy(struct cmap *cmap)
 {
     if (cmap) {
-        ovsrcu_postpone(free_cacheline, cmap_get_impl(cmap));
+        struct cmap_impl *impl = cmap_get_impl(cmap);
+        if (impl != &empty_cmap) {
+            ovsrcu_postpone(free_cacheline, impl);
+        }
     }
 }
 
@@ -892,7 +896,9 @@ cmap_rehash(struct cmap *cmap, uint32_t mask)
 
     new->n = old->n;
     ovsrcu_set(&cmap->impl, new);
-    ovsrcu_postpone(free_cacheline, old);
+    if (old != &empty_cmap) {
+        ovsrcu_postpone(free_cacheline, old);
+    }
 
     return new;
 }

diff --git a/lib/cmap.h b/lib/cmap.h
index c7c65af..d390923 100644 (file)
--- a/lib/cmap.h
+++ b/lib/cmap.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014 Nicira, Inc.
+ * Copyright (c) 2014, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -79,6 +79,12 @@ struct cmap {
     OVSRCU_TYPE(struct cmap_impl *) impl;
 };
 
+/* Initializer for an empty cmap. */
+#define CMAP_INITIALIZER {                                              \
+        .impl = OVSRCU_INITIALIZER((struct cmap_impl *) &empty_cmap)    \
+    }
+extern OVS_ALIGNED_VAR(CACHE_LINE_SIZE) const struct cmap_impl empty_cmap;
+
 /* Initialization. */
 void cmap_init(struct cmap *);
 void cmap_destroy(struct cmap *);

diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c
index 8650e73..d01ec7f 100644 (file)
--- a/lib/tnl-neigh-cache.c
+++ b/lib/tnl-neigh-cache.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -55,7 +55,7 @@ struct tnl_neigh_entry {
     char br_name[IFNAMSIZ];
 };
 
-static struct cmap table;
+static struct cmap table = CMAP_INITIALIZER;
 static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
 
 static uint32_t
@@ -312,8 +312,6 @@ tnl_neigh_cache_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
 void
 tnl_neigh_cache_init(void)
 {
-    cmap_init(&table);
-
     unixctl_command_register("tnl/arp/show", "", 0, 0, tnl_neigh_cache_show, NULL);
     unixctl_command_register("tnl/arp/set", "BRIDGE IP MAC", 3, 3, tnl_neigh_cache_add, NULL);
     unixctl_command_register("tnl/arp/flush", "", 0, 0, tnl_neigh_cache_flush, NULL);

diff --git a/ofproto/ofproto-dpif-rid.c b/ofproto/ofproto-dpif-rid.c
index b3f64f9..f59775c 100644 (file)
--- a/ofproto/ofproto-dpif-rid.c
+++ b/ofproto/ofproto-dpif-rid.c
@@ -24,40 +24,22 @@
 
 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_rid);
 
-static struct ovs_mutex mutex;
+static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
 
-static struct cmap id_map;
-static struct cmap metadata_map;
+static struct cmap id_map = CMAP_INITIALIZER;
+static struct cmap metadata_map = CMAP_INITIALIZER;
 
-static struct ovs_list expiring OVS_GUARDED_BY(mutex);
-static struct ovs_list expired OVS_GUARDED_BY(mutex);
+static struct ovs_list expiring OVS_GUARDED_BY(mutex)
+    = OVS_LIST_INITIALIZER(&expiring);
+static struct ovs_list expired OVS_GUARDED_BY(mutex)
+    = OVS_LIST_INITIALIZER(&expired);
 
-static uint32_t next_id OVS_GUARDED_BY(mutex); /* Possible next free id. */
+static uint32_t next_id OVS_GUARDED_BY(mutex) = 1; /* Possible next free id. */
 
 #define RECIRC_POOL_STATIC_IDS 1024
 
 static void recirc_id_node_free(struct recirc_id_node *);
 
-void
-recirc_init(void)
-{
-    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
-
-    if (ovsthread_once_start(&once)) {
-        ovs_mutex_init(&mutex);
-        ovs_mutex_lock(&mutex);
-        next_id = 1; /* 0 is not a valid ID. */
-        cmap_init(&id_map);
-        cmap_init(&metadata_map);
-        ovs_list_init(&expiring);
-        ovs_list_init(&expired);
-        ovs_mutex_unlock(&mutex);
-
-        ovsthread_once_done(&once);
-    }
-
-}
-
 /* This should be called by the revalidator once at each round (every 500ms or
  * more). */
 void

diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 3efd85e..c34d760 100644 (file)
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -176,8 +176,6 @@ struct recirc_id_node {
     struct flow_tnl state_metadata_tunnel;
 };
 
-void recirc_init(void);
-
 /* This is only used for bonds and will go away when bonds implementation is
  * updated to use this mechanism instead of internal rules. */
 uint32_t recirc_alloc_id(struct ofproto_dpif *);

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 056eb5e..0c65df2 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -861,8 +861,6 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     const char *name;
     int error;
 
-    recirc_init();
-
     backer = shash_find_data(&all_dpif_backers, type);
     if (backer) {
         backer->refcount++;

diff --git a/tests/test-cmap.c b/tests/test-cmap.c
index c36ecb6..05aba00 100644 (file)
--- a/tests/test-cmap.c
+++ b/tests/test-cmap.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2013, 2014, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -210,10 +210,9 @@ test_cmap_insert_replace_delete(hash_func *hash)
     struct element elements[N_ELEMS];
     struct element copies[N_ELEMS];
     int values[N_ELEMS];
-    struct cmap cmap;
+    struct cmap cmap = CMAP_INITIALIZER;
     size_t i;
 
-    cmap_init(&cmap);
     for (i = 0; i < N_ELEMS; i++) {
         elements[i].value = i;
         cmap_insert(&cmap, &elements[i].node, hash(i));