public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Yinghai Lu <Yinghai.Lu@Sun.COM>
Cc: Mike Travis <travis@sgi.com>,
	Christoph Lameter <clameter@sgi.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Sam Ravnborg <sam@ravnborg.org>
Subject: Re: [PATCH] x86_64: mark x86_cpu_to_node_map_init to __initdata like other xx_init
Date: Mon, 28 Jan 2008 11:34:39 +0100	[thread overview]
Message-ID: <20080128103439.GC24010@elte.hu> (raw)
In-Reply-To: <200801280116.54810.yinghai.lu@sun.com>


* Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:

> -int x86_cpu_to_node_map_init[NR_CPUS] = {
> +int x86_cpu_to_node_map_init[NR_CPUS] __initdata = {
>  	[0 ... NR_CPUS-1] = NUMA_NO_NODE
>  };

i remember some linker warning here. While this array should indeed only 
be used in early init, that decision is dynamic and our linker warnings 
do not notice it. There's a special marker for such cases: 
__initdata_refok. But ... i'm slightly nervous about turning off a vital 
warning like that.

Sam, it would be nice to have a DEBUG_INITDATA mode of operation: in 
this case free_initmem() would not truly free those pages but would 
unmap them via:

   kernel_map_pages(page, nrpages, 0);

could be made dependent on DEBUG_PAGEALLOC.

If this debugging is enabled then if any code references it, we get a 
hard page fault. If we had a debug mode like that then bugs in this area 
would not go unnoticed.

Or perhaps just make this part of normal DEBUG_PAGEALLOC. Like the patch 
below on top of latest x86.git. Hm?

	Ingo

------------------->
Subject: x86: init memory debugging
From: Ingo Molnar <mingo@elte.hu>

debug incorrect/late access to init memory, by permanently unmapping
the init memory ranges. Depends on CONFIG_DEBUG_PAGEALLOC=y.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/mm/init_32.c |   12 ++++++++++++
 arch/x86/mm/init_64.c |   11 +++++++++++
 2 files changed, 23 insertions(+)

Index: linux-x86.q/arch/x86/mm/init_32.c
===================================================================
--- linux-x86.q.orig/arch/x86/mm/init_32.c
+++ linux-x86.q/arch/x86/mm/init_32.c
@@ -794,6 +794,18 @@ void free_init_pages(char *what, unsigne
 	unsigned long addr;
 
 	/*
+	 * If debugging page accesses then do not free this memory but
+	 * mark them not present - any buggy init-section access will
+	 * create a kernel page fault:
+	 */
+#ifdef CONFIG_DEBUG_PAGEALLOC
+	printk(KERN_INFO "debug: unmapping init memory %08lx..%08lx\n",
+		begin, PAGE_ALIGN(end));
+	set_memory_np(begin, (end - begin) >> PAGE_SHIFT);
+	return;
+#endif
+	set_memory_rw(begin, (end - begin) >> PAGE_SHIFT);
+	/*
 	 * We just marked the kernel text read only above, now that
 	 * we are going to free part of that, we need to make that
 	 * writeable first.
Index: linux-x86.q/arch/x86/mm/init_64.c
===================================================================
--- linux-x86.q.orig/arch/x86/mm/init_64.c
+++ linux-x86.q/arch/x86/mm/init_64.c
@@ -580,6 +580,17 @@ void free_init_pages(char *what, unsigne
 	if (begin >= end)
 		return;
 
+	/*
+	 * If debugging page accesses then do not free this memory but
+	 * mark them not present - any buggy init-section access will
+	 * create a kernel page fault:
+	 */
+#ifdef CONFIG_DEBUG_PAGEALLOC
+	printk(KERN_INFO "debug: unmapping init memory %08lx..%08lx\n",
+		begin, PAGE_ALIGN(end));
+	set_memory_np(begin, (end - begin) >> PAGE_SHIFT);
+	return;
+#endif
 	printk(KERN_INFO "Freeing %s: %luk freed\n", what, (end - begin) >> 10);
 
 	for (addr = begin; addr < end; addr += PAGE_SIZE) {

  reply	other threads:[~2008-01-28 10:35 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-28  9:16 [PATCH] x86_64: mark x86_cpu_to_node_map_init to __initdata like other xx_init Yinghai Lu
2008-01-28 10:34 ` Ingo Molnar [this message]
     [not found] <200801290053.45776.yinghai.lu@sun.com>
2008-01-29  9:05 ` [PATCH 1/2] print out node_data addr and bootmap_start addr Yinghai Lu
     [not found]   ` <20080201170908.GB2159@elte.hu>
2008-02-01 21:29     ` [PATCH] x86_64: mark x86_cpu_to_node_map_init to __initdata like other xx_init Yinghai Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080128103439.GC24010@elte.hu \
    --to=mingo@elte.hu \
    --cc=Yinghai.Lu@Sun.COM \
    --cc=clameter@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sam@ravnborg.org \
    --cc=travis@sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox