BUILD: ebtree: improve architecture-specific alignment

Commit 2c315ee75e ("BUG/MEDIUM: ebtree: don't set attribute packed
without unaligned access support") addressed alignment issues in
ebtrees in a way that is not really optimal since it will leave holes
in eb32trees for example.

This fix is better in that it restores the packed attribute on ebnode
but enforces proper alignment on the carrying nodes where necessary.
This also has the benefit of closing holes wherever possible and to
align data to the minimally required size.

The only thing it cannot close is the 32-bit hole at the end of ebmbnode
due to the required 64-bit on certain archs but at least it guarantees
that the key correctly points to the end of the node and that there is
never a hole after it.

This is a better fix than the one above and should be backported to
branches where the one above will be backported.

diff --git a/ebtree/eb32sctree.h b/ebtree/eb32sctree.h
index f547ff039c8ae40e13992d71b414c7f58d1285cb..70613088694d46de1d26de63623136daf7067f2c 100644 (file)
--- a/ebtree/eb32sctree.h
+++ b/ebtree/eb32sctree.h
@@ -38,13 +38,18 @@ typedef   signed int s32;
  * have put some sort of transparent union here to reduce the indirection
  * level, but the fact is, the end user is not meant to manipulate internals,
  * so this is pointless.
+ * In case sizeof(void*)>=sizeof(long), we know there will be some padding after
+ * the leaf if it's unaligned. In this case we force the alignment on void* so
+ * that we prefer to have the padding before for more efficient accesses.
  */
 struct eb32sc_node {
        struct eb_node node; /* the tree node, must be at the beginning */
+       MAYBE_ALIGN(sizeof(u32));
        u32 key;
+       ALWAYS_ALIGN(sizeof(void*));
        unsigned long node_s; /* visibility of this node's branches */
        unsigned long leaf_s; /* visibility of this node's leaf */
-};
+} ALIGNED(sizeof(void*));
 
 /*
  * Exported functions and macros.

diff --git a/ebtree/eb32tree.h b/ebtree/eb32tree.h
index 8c1a47025cc43e476dc2d3194e14fadf024668db..3896fb81bca82e011caebcb294d0dd567317039d 100644 (file)
--- a/ebtree/eb32tree.h
+++ b/ebtree/eb32tree.h
@@ -41,8 +41,9 @@ typedef   signed int s32;
  */
 struct eb32_node {
        struct eb_node node; /* the tree node, must be at the beginning */
+       MAYBE_ALIGN(sizeof(u32));
        u32 key;
-};
+} ALIGNED(sizeof(void*));
 
 /*
  * Exported functions and macros.

diff --git a/ebtree/eb64tree.h b/ebtree/eb64tree.h
index 9c8feebd319627ed0b3f838664dade27349067fb..ff2feee44eb6a2b3335a581d93a45d62decb381c 100644 (file)
--- a/ebtree/eb64tree.h
+++ b/ebtree/eb64tree.h
@@ -38,11 +38,16 @@ typedef   signed long long s64;
  * eb_node so that it can be cast into an eb_node. We could also have put some
  * sort of transparent union here to reduce the indirection level, but the fact
  * is, the end user is not meant to manipulate internals, so this is pointless.
+ * In case sizeof(void*)>=sizeof(u64), we know there will be some padding after
+ * the key if it's unaligned. In this case we force the alignment on void* so
+ * that we prefer to have the padding before for more efficient accesses.
  */
 struct eb64_node {
        struct eb_node node; /* the tree node, must be at the beginning */
+       MAYBE_ALIGN(sizeof(u64));
+       ALWAYS_ALIGN(sizeof(void*));
        u64 key;
-};
+} ALIGNED(sizeof(void*));
 
 /*
  * Exported functions and macros.

diff --git a/ebtree/ebmbtree.h b/ebtree/ebmbtree.h
index 3200a126e09d27847a6c7ce6ef94a0279d047910..8262ec61454defc7dc19989ab690e414b21e0468 100644 (file)
--- a/ebtree/ebmbtree.h
+++ b/ebtree/ebmbtree.h
@@ -36,11 +36,16 @@
  * is, the end user is not meant to manipulate internals, so this is pointless.
  * The 'node.bit' value here works differently from scalar types, as it contains
  * the number of identical bits between the two branches.
+ * Note that we take a great care of making sure the key is located exactly at
+ * the end of the struct even if that involves holes before it, so that it
+ * always aliases any external key a user would append after. This is why the
+ * key uses the same alignment as the struct.
  */
 struct ebmb_node {
        struct eb_node node; /* the tree node, must be at the beginning */
+       ALWAYS_ALIGN(sizeof(void*));
        unsigned char key[0]; /* the key, its size depends on the application */
-};
+} ALIGNED(sizeof(void*));
 
 /*
  * Exported functions and macros.

diff --git a/ebtree/ebpttree.h b/ebtree/ebpttree.h
index a1db03b2875d5d96adce57eaacab6fcf3a1d2f30..6cd66598874f266f02bcaead49965a9d4336fa4d 100644 (file)
--- a/ebtree/ebpttree.h
+++ b/ebtree/ebpttree.h
@@ -44,11 +44,14 @@ typedef PTR_INT_TYPE ptr_t;
  * sort of transparent union here to reduce the indirection level, but the fact
  * is, the end user is not meant to manipulate internals, so this is pointless.
  * Internally, it is automatically cast as an eb32_node or eb64_node.
+ * We always align the key since the struct itself will be padded to the same
+ * size anyway.
  */
 struct ebpt_node {
        struct eb_node node; /* the tree node, must be at the beginning */
+       ALWAYS_ALIGN(sizeof(void*));
        void *key;
-};
+} ALIGNED(sizeof(void*));
 
 /*
  * Exported functions and macros.

diff --git a/ebtree/ebtree.h b/ebtree/ebtree.h
index e87e961c3301197741c1b2c7f2879328920a1813..dff044b3902d595ad9e99e5058069e08e7329c5d 100644 (file)
--- a/ebtree/ebtree.h
+++ b/ebtree/ebtree.h
@@ -379,11 +379,7 @@ struct eb_node {
        eb_troot_t    *leaf_p;  /* leaf node's parent */
        short int      bit;     /* link's bit position. */
        short unsigned int pfx; /* data prefix length, always related to leaf */
-}
-#ifdef HA_UNALIGNED
-   __attribute__((packed))
-#endif
-   ;
+} __attribute__((packed));
 
 /* Return the structure of type <type> whose member <member> points to <ptr> */
 #define eb_entry(ptr, type, member) container_of(ptr, type, member)