Re: [PATCH] x86/i386: make sure stack-protector segment base is cache aligned

Jeremy Fitzhardinge
2009-09-03T21:31:45Z

On 09/03/09 14:07, Eric Dumazet wrote:
> I was referring to .data.percpu alignement requirements, not to false sharing.
>

Yes, I know. But the intent of DECLARE_PER_CPU_SHARED_ALIGNED is to
avoid false sharing, and so it does no alignment when CONFIG_SMP isn't
enabled. We need alignment regardless.

> When we put a object with an align(64) requirement into a section, linker
> has to put this 2**6 alignment in resulting section.
>
> When several .o are linked together, linker has to take the biggest alignement,
> and has to put holes.
>
> Check .data.percpu size in vmlinux before and after your patch, to make sure
> it doesnt grow too much :)
>
> therefore, ____cacheline_aligned objects should be placed in
> .data.percpu.shared_aligned
>

That section doesn't exist without SMP. (Well,
PER_CPU_SHARED_ALIGNED_SECTION isn't defined.)

Anyway, this should sort it out.

J

>From 69022dd952e65608afe33372e902b41cb94ff126 Mon Sep 17 00:00:00 2001
From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Date: Thu, 3 Sep 2009 14:17:27 -0700
Subject: [PATCH] x86/32: put aligned stack-canary in percpu shared_aligned section

Pack aligned things together into a special section to minimize padding
holes.

Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index e597ecc..ac7e796 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -413,7 +413,7 @@ struct stack_canary {
char __pad[20]; /* canary at %gs:20 */
unsigned long canary;
};
-DECLARE_PER_CPU(struct stack_canary, stack_canary) ____cacheline_aligned;
+DECLARE_PER_CPU_ALIGNED(struct stack_canary, stack_canary);
#endif
#endif /* X86_64 */

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index e338b5c..1e8181c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1043,7 +1043,7 @@ DEFINE_PER_CPU(struct orig_ist, orig_ist);
#else /* CONFIG_X86_64 */

#ifdef CONFIG_CC_STACKPROTECTOR
-DEFINE_PER_CPU(struct stack_canary, stack_canary) ____cacheline_aligned;
+DEFINE_PER_CPU_ALIGNED(struct stack_canary, stack_canary);
#endif

/* Make sure %fs and %gs are initialized properly in idle threads */
diff --git a/include/asm-generic/percpu.h b/include/asm-generic/percpu.h
index aa00800..90079c3 100644
--- a/include/asm-generic/percpu.h
+++ b/include/asm-generic/percpu.h
@@ -81,14 +81,17 @@ extern void setup_per_cpu_areas(void);

#ifdef MODULE
#define PER_CPU_SHARED_ALIGNED_SECTION ""
+#define PER_CPU_ALIGNED_SECTION ""
#else
#define PER_CPU_SHARED_ALIGNED_SECTION ".shared_aligned"
+#define PER_CPU_ALIGNED_SECTION ".shared_aligned"
#endif
#define PER_CPU_FIRST_SECTION ".first"

#else

#define PER_CPU_SHARED_ALIGNED_SECTION ""
+#define PER_CPU_ALIGNED_SECTION ".shared_aligned"
#define PER_CPU_FIRST_SECTION ""

#endif
diff --git a/include/linux/percpu-defs.h b/include/linux/percpu-defs.h
index 68438e1..3058cf9 100644
--- a/include/linux/percpu-defs.h
+++ b/include/linux/percpu-defs.h
@@ -66,6 +66,14 @@
DEFINE_PER_CPU_SECTION(type, name, PER_CPU_SHARED_ALIGNED_SECTION) \
____cacheline_aligned_in_smp

+#define DECLARE_PER_CPU_ALIGNED(type, name) \
+ DECLARE_PER_CPU_SECTION(type, name, PER_CPU_ALIGNED_SECTION) \
+ ____cacheline_aligned
+
+#define DEFINE_PER_CPU_ALIGNED(type, name) \
+ DEFINE_PER_CPU_SECTION(type, name, PER_CPU_ALIGNED_SECTION) \
+ ____cacheline_aligned
+
/*
* Declaration/definition used for per-CPU variables that must be page aligned.
*/