Re: [bisected] Re: [PATCH 05 of 12] xen: add p2m mfn_list_list

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Jeremy Fitzhardinge
Date: Wednesday, May 28, 2008 - 7:02 am

Ingo Molnar wrote:

Thanks for that. That's pretty subtle...


The use of __section(.data.page_aligned) (or worse 
__attribute__((section(".data.page_aligned"))) is fairly verbose and 
brittle. I've got a (totally untested) proposed patch below, to 
introduce __page_aligned_data|bss which sets the section and the 
alignment. This will work, but it requires that all page-aligned 
variables also have an alignment associated with them, so that mis-sized 
ones don't push the others around.

There aren't very many users of .data|bss.page_aligned, so it should be 
easy enough to fix them all up.

A link-time warning would be good too, of course.


Yes, that's right. It can never get to page size, but it must be on its 
own page because its only ever referred to by page number; these pages 
are read from outside the VM when doing save/restore.


That's a bit severe - it expands it to 4 pages ;)

J


Subject: make page-aligned data and bss less fragile

Making a variable page-aligned by using
__attribute__((section(".data.page_aligned"))) is fragile because if
sizeof(variable) is not also a multiple of page size, it leaves
variables in the remainder of the section unaligned.

This patch introduces two new qualifiers, __page_aligned_data and
__page_aligned_bss to set the section *and* the alignment of
variables.  This makes page-aligned variables more robust because the
linker will make sure they're aligned properly.  Unfortunately it
requires *all* page-aligned data to use these macros...

---
 arch/x86/kernel/irq_32.c  |    7 ++-----
 arch/x86/kernel/setup64.c |    2 +-
 arch/x86/mm/ioremap.c     |    3 +--
 arch/x86/xen/mmu.c        |   12 +++++-------
 include/linux/linkage.h   |    4 ++++
 5 files changed, 13 insertions(+), 15 deletions(-)

===================================================================
--- a/arch/x86/kernel/irq_32.c
+++ b/arch/x86/kernel/irq_32.c
@@ -83,11 +83,8 @@
 static union irq_ctx *hardirq_ctx[NR_CPUS] __read_mostly;
 static union irq_ctx *softirq_ctx[NR_CPUS] __read_mostly;
 
-static char softirq_stack[NR_CPUS * THREAD_SIZE]
-		__attribute__((__section__(".bss.page_aligned")));
-
-static char hardirq_stack[NR_CPUS * THREAD_SIZE]
-		__attribute__((__section__(".bss.page_aligned")));
+static char softirq_stack[NR_CPUS * THREAD_SIZE] __page_aligned_bss;
+static char hardirq_stack[NR_CPUS * THREAD_SIZE] __page_aligned_bss;
 
 static void call_on_stack(void *func, void *stack)
 {
===================================================================
--- a/arch/x86/kernel/setup64.c
+++ b/arch/x86/kernel/setup64.c
@@ -40,7 +40,7 @@
 
 struct desc_ptr idt_descr = { 256 * 16 - 1, (unsigned long) idt_table };
 
-char boot_cpu_stack[IRQSTACKSIZE] __attribute__((section(".bss.page_aligned")));
+char boot_cpu_stack[IRQSTACKSIZE] __page_aligned_bss;
 
 unsigned long __supported_pte_mask __read_mostly = ~0UL;
 EXPORT_SYMBOL_GPL(__supported_pte_mask);
===================================================================
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -403,8 +403,7 @@
 early_param("early_ioremap_debug", early_ioremap_debug_setup);
 
 static __initdata int after_paging_init;
-static pte_t bm_pte[PAGE_SIZE/sizeof(pte_t)]
-		__section(.bss.page_aligned);
+static pte_t bm_pte[PAGE_SIZE/sizeof(pte_t)] __page_aligned_bss;
 
 static inline pmd_t * __init early_ioremap_pmd(unsigned long addr)
 {
===================================================================
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -46,6 +46,7 @@
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>
 #include <asm/paravirt.h>
+#include <asm/linkage.h>
 
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
@@ -60,21 +61,18 @@
 #define TOP_ENTRIES		(MAX_DOMAIN_PAGES / P2M_ENTRIES_PER_PAGE)
 
 /* Placeholder for holes in the address space */
-static unsigned long p2m_missing[P2M_ENTRIES_PER_PAGE]
-	__attribute__((section(".data.page_aligned"))) =
+static unsigned long p2m_missing[P2M_ENTRIES_PER_PAGE] __page_aligned_data =
 		{ [ 0 ... P2M_ENTRIES_PER_PAGE-1 ] = ~0UL };
 
  /* Array of pointers to pages containing p2m entries */
-static unsigned long *p2m_top[TOP_ENTRIES]
-	__attribute__((section(".data.page_aligned"))) =
+static unsigned long *p2m_top[TOP_ENTRIES] __page_aligned_data =
 		{ [ 0 ... TOP_ENTRIES - 1] = &p2m_missing[0] };
 
 /* Arrays of p2m arrays expressed in mfns used for save/restore */
-static unsigned long p2m_top_mfn[TOP_ENTRIES]
-	__attribute__((section(".bss.page_aligned")));
+static unsigned long p2m_top_mfn[TOP_ENTRIES] __page_aligned_bss;
 
 static unsigned long p2m_top_mfn_list[TOP_ENTRIES / P2M_ENTRIES_PER_PAGE]
-	__attribute__((section(".bss.page_aligned")));
+	__page_aligned_bss;
 
 static inline unsigned p2m_top_index(unsigned long pfn)
 {
===================================================================
--- a/include/linux/linkage.h
+++ b/include/linux/linkage.h
@@ -1,6 +1,7 @@
 #ifndef _LINUX_LINKAGE_H
 #define _LINUX_LINKAGE_H
 
+#include <linux/compiler.h>
 #include <asm/linkage.h>
 
 #define notrace __attribute__((no_instrument_function))
@@ -18,6 +19,9 @@
 #ifndef asmregparm
 # define asmregparm
 #endif
+
+#define __page_aligned_data	__section(.data.page_aligned) __aligned(PAGE_SIZE)
+#define __page_aligned_bss	__section(.bss.page_aligned) __aligned(PAGE_SIZE)
 
 /*
  * This is used by architectures to keep arguments on the stack


--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH 00 of 12] xen: add save/restore/migrate for Xen domains, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 01 of 12] xen: make phys_to_machine structure dynamic, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 02 of 12] xen: add configurable max domain size, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 03 of 12] xen: efficiently support a holey p2m table, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 04 of 12] xen: make dummy_shared_info non-static, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 05 of 12] xen: add p2m mfn_list_list, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 06 of 12] xen: add rebind_evtchn_irq, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 07 of 12] xen: fix unbind_from_irq(), Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 08 of 12] xen-console: add save/restore, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 09 of 12] xenbus: rebind irq on restore, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 10 of 12] xen: implement save/restore, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 11 of 12] xen: maintain clock offset over save/restore, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
[PATCH 12 of 12] hrtimer: remove warning in hres_timers_resume, Jeremy Fitzhardinge, (Fri May 23, 6:41 am)
Re: [bisected] Re: [PATCH 05 of 12] xen: add p2m mfn_list_list, Jeremy Fitzhardinge, (Wed May 28, 7:02 am)
Re: [PATCH 10 of 12] xen: implement save/restore, Ingo Molnar, (Thu May 29, 12:31 am)
Re: [PATCH 10 of 12] xen: implement save/restore, Jeremy Fitzhardinge, (Thu May 29, 1:00 am)
[PATCH] xen: fix compilation when CONFIG_PM_SLEEP is disabled, Jeremy Fitzhardinge, (Thu May 29, 1:02 am)
Re: [bisected] Re: [PATCH 05 of 12] xen: add p2m mfn_list_list, Jeremy Fitzhardinge, (Fri May 30, 1:04 am)
Re: [Xen-devel] [PATCH 10 of 12] xen: implement save/restore, Jeremy Fitzhardinge, (Mon Jun 2, 3:03 am)
Re: [Xen-devel] [PATCH 10 of 12] xen: implement save/restore, Jeremy Fitzhardinge, (Mon Jun 2, 3:52 am)
Re: [PATCH 08 of 12] xen-console: add save/restore, Ingo Molnar, (Mon Jun 2, 4:17 am)
Re: [PATCH 08 of 12] xen-console: add save/restore, Ingo Molnar, (Mon Jun 2, 4:18 am)
Re: [PATCH 08 of 12] xen-console: add save/restore, Jeremy Fitzhardinge, (Mon Jun 2, 4:50 am)
Re: [bisected] Re: [PATCH 05 of 12] xen: add p2m mfn_list_list, Jeremy Fitzhardinge, (Mon Jun 2, 6:12 am)