BUG/MEDIUM: fd: always align fdtab[] to 64 bytes

There's a risk that fdtab is not 64-byte aligned. The first effect is that
it may cause false sharing between cache lines resulting in contention
when adjacent FDs are used by different threads. The second is related
to what is explained in commit "BUG/MAJOR: compiler: relax alignment
constraints on certain structures", i.e. that modern compilers might
make use of aligned vector operations to zero some entries, and would
crash. We do not use any memset() or so on fdtab, so the risk is almost
inexistent, but that's not a reason for violating some valid assumptions.

This patch addresses this by allocating 64 extra bytes and aligning the
structure manually (this is an extremely cheap solution for this specific
case). The original address is stored in a new variable "fdtab_addr" and
is the one that gets freed. This remains extremely simple and should be
easily backportable. A dedicated aligned allocator later would help, of
course.

This needs to be backported as far as 2.2. No issue related to this was
reported yet, but it could very well happen as compilers evolve. In
addition this should preserve high performance across restarts (i.e.
no more dependency on allocator's alignment).

diff --git a/src/fd.c b/src/fd.c
index cbfe73a2de42c09e25e5df16913720cb237eaa29..30fefc473efcfd0c52a508baba60e56debdf45f9 100644 (file)
--- a/src/fd.c
+++ b/src/fd.c
@@ -116,6 +116,7 @@ THREAD_LOCAL int poller_rd_pipe = -1; // Pipe to wake the thread
 int poller_wr_pipe[MAX_THREADS] __read_mostly; // Pipe to wake the threads
 
 volatile int ha_used_fds = 0; // Number of FD we're currently using
+static struct fdtab *fdtab_addr;  /* address of the allocated area containing fdtab */
 
 #define _GET_NEXT(fd, off) ((volatile struct fdlist_entry *)(void *)((char *)(&fdtab[fd]) + off))->next
 #define _GET_PREV(fd, off) ((volatile struct fdlist_entry *)(void *)((char *)(&fdtab[fd]) + off))->prev
@@ -797,11 +798,14 @@ int init_pollers()
        int p;
        struct poller *bp;
 
-       if ((fdtab = calloc(global.maxsock, sizeof(*fdtab))) == NULL) {
+       if ((fdtab_addr = calloc(global.maxsock, sizeof(*fdtab) + 64)) == NULL) {
                ha_alert("Not enough memory to allocate %d entries for fdtab!\n", global.maxsock);
                goto fail_tab;
        }
 
+       /* always provide an aligned fdtab */
+       fdtab = (struct fdtab*)((((size_t)fdtab_addr) + 63) & -(size_t)64);
+
        if ((polled_mask = calloc(global.maxsock, sizeof(*polled_mask))) == NULL) {
                ha_alert("Not enough memory to allocate %d entries for polled_mask!\n", global.maxsock);
                goto fail_polledmask;
@@ -838,7 +842,7 @@ int init_pollers()
  fail_info:
        free(polled_mask);
  fail_polledmask:
-       free(fdtab);
+       free(fdtab_addr);
  fail_tab:
        return 0;
 }
@@ -859,7 +863,7 @@ void deinit_pollers() {
        }
 
        ha_free(&fdinfo);
-       ha_free(&fdtab);
+       ha_free(&fdtab_addr);
        ha_free(&polled_mask);
 }