BUG/MEDIUM: ebtree: don't set attribute packed without unaligned access support

An alignment issue on Sparc64 with ebtrees was reported in early 2017
here https://www.mail-archive.com/haproxy@formilux.org/msg25937.html and
a similar one was finally reported in issue #512.

The problem has its roots in the fact that 64-bit keys will end up being
unaligned on such archs which do not support unaligned accesses. But on
most platforms supporting unaligned accesses, dealing with smaller nodes
results in better performance.

One of the possible problems caused by attribute packed there is that it
promotes a structure both to be unaligned and unpadded, which may come
with fun if some fields of the struct itself are accessed and such a node
is placed at an unaligned location. It's not a problem for regular unaligned
accesses but may become one for atomic operations such as the CAS on leaf_p
that's used in struct task. In practice we know that this struct is properly
aligned and is a very edge case so this patch adds comments there to remind
to be careful about it.

This patch depends on previous patch "MINOR: compiler: move CPU capabilities
definition from config.h and complete them" and could be backported to all
stable branches. It fixes issues #512 and #9.

diff --git a/ebtree/ebtree.h b/ebtree/ebtree.h
index 2e6ba03c08da58c6d851ab90045a0e29fd7cfdad..0ba42f1ba0637b913450c6d34a5c52ddb4e53531 100644 (file)
--- a/ebtree/ebtree.h
+++ b/ebtree/ebtree.h
@@ -370,6 +370,8 @@ struct eb_root {
  * and one for the node, which remains unused in the very first node inserted
  * into the tree. This structure is 20 bytes per node on 32-bit machines. Do
  * not change the order, benchmarks have shown that it's optimal this way.
+ * Note: be careful about this struct's alignment if it gets included into
+ * another struct and some atomic ops are expected on the keys or the node.
  */
 struct eb_node {
        struct eb_root branches; /* branches, must be at the beginning */
@@ -377,7 +379,10 @@ 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 */
-} __attribute__((packed));
+}
+#ifdef HA_UNALIGNED
+   __attribute__((packed));
+#endif
 
 /* Return the structure of type <type> whose member <member> points to <ptr> */
 #define eb_entry(ptr, type, member) container_of(ptr, type, member)