[PATCH 1/4] Add a basic debugging framework for memory initialisation

[PATCH 1/4] Add a basic debugging framework for memory initialisation

[Mel Gorman]

This patch creates a new file mm/mm_init.c which memory initialisation should
be moved to over time to avoid further polluting page_alloc.c. This patch
introduces a simple mminit_debug_printk() function and an (undocumented)
mminit_debug_level commmand-line parameter for setting the level of tracing
and verification that should be done.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 mm/Makefile     |    2 +-
 mm/internal.h   |    9 +++++++++
 mm/mm_init.c    |   40 ++++++++++++++++++++++++++++++++++++++++
 mm/page_alloc.c |   16 ++++++++++------
 4 files changed, 60 insertions(+), 7 deletions(-)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-clean/mm/internal.h linux-2.6.25-rc9-0010_mminit_debug_framework/mm/internal.h
--- linux-2.6.25-rc9-clean/mm/internal.h	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/mm/internal.h	2008-04-16 14:44:19.000000000 +0100
@@ -60,4 +60,13 @@ static inline unsigned long page_order(s
 #define __paginginit __init
 #endif
 
+/* Memory initilisation debug and verification */
+enum mminit_levels {
+	MMINIT_NORMAL,
+	MMINIT_VERIFY,
+	MMINIT_TRACE
+};
+
+extern void mminit_debug_printk(unsigned int level, const char *prefix,
+				const char *fmt, ...);
 #endif
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-clean/mm/Makefile linux-2.6.25-rc9-0010_mminit_debug_framework/mm/Makefile
--- linux-2.6.25-rc9-clean/mm/Makefile	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/mm/Makefile	2008-04-16 14:44:19.000000000 +0100
@@ -11,7 +11,7 @@ obj-y			:= bootmem.o filemap.o mempool.o
 			   page_alloc.o page-writeback.o pdflush.o \
 			   readahead.o swap.o truncate.o vmscan.o \
 			   prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \
-			   page_isolation.o $(mmu-y)
+			   page_isolation.o mm_init.o $(mmu-y)
 
 obj-$(CONFIG_PROC_PAGE_MONITOR) += pagewalk.o
 obj-$(CONFIG_BOUNCE)	+= bounce.o
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-clean/mm/mm_init.c linux-2.6.25-rc9-0010_mminit_debug_framework/mm/mm_init.c
--- linux-2.6.25-rc9-clean/mm/mm_init.c	2008-04-16 10:42:54.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/mm/mm_init.c	2008-04-16 14:44:19.000000000 +0100
@@ -0,0 +1,40 @@
+/*
+ * mm_init.c - Memory initialisation verification and debugging
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+
+int __initdata mminit_debug_level;
+
+#define MMINIT_BUF_LEN 256
+
+void __meminit mminit_debug_printk(unsigned int level, const char *prefix,
+			const char *fmt, ...)
+{
+	char s[MMINIT_BUF_LEN];
+	va_list args;
+	unsigned int len;
+
+	WARN_ON(!prefix);
+	if (level < mminit_debug_level) {
+		len = snprintf(s, MMINIT_BUF_LEN, KERN_INFO "mminit::%s ",
+								prefix);
+
+		va_start(args, fmt);
+		len += vsnprintf(&s[len], (MMINIT_BUF_LEN - len), fmt, args);
+		va_end(args);
+
+		printk(s);
+
+		WARN_ON(len < 5);
+		WARN_ON(len == MMINIT_BUF_LEN);
+	}
+}
+
+static __init int set_mminit_debug_level(char *str)
+{
+	get_option(&str, &mminit_debug_level);
+	return 0;
+}
+early_param("mminit_debug_level", set_mminit_debug_level);
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-clean/mm/page_alloc.c linux-2.6.25-rc9-0010_mminit_debug_framework/mm/page_alloc.c
--- linux-2.6.25-rc9-clean/mm/page_alloc.c	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/mm/page_alloc.c	2008-04-16 14:44:19.000000000 +0100
@@ -2958,7 +2958,8 @@ void __init sparse_memory_present_with_a
 void __init push_node_boundaries(unsigned int nid,
 		unsigned long start_pfn, unsigned long end_pfn)
 {
-	printk(KERN_DEBUG "Entering push_node_boundaries(%u, %lu, %lu)\n",
+	mminit_debug_printk(MMINIT_TRACE, "zoneboundary",
+			"Entering push_node_boundaries(%u, %lu, %lu)\n",
 			nid, start_pfn, end_pfn);
 
 	/* Initialise the boundary for this node if necessary */
@@ -2976,7 +2977,8 @@ void __init push_node_boundaries(unsigne
 static void __meminit account_node_boundary(unsigned int nid,
 		unsigned long *start_pfn, unsigned long *end_pfn)
 {
-	printk(KERN_DEBUG "Entering account_node_boundary(%u, %lu, %lu)\n",
+	mminit_debug_printk(MMINIT_TRACE, "zoneboundary",
+			"Entering account_node_boundary(%u, %lu, %lu)\n",
 			nid, *start_pfn, *end_pfn);
 
 	/* Return if boundary information has not been provided */
@@ -3350,8 +3352,8 @@ static void __paginginit free_area_init_
 		memmap_pages = (size * sizeof(struct page)) >> PAGE_SHIFT;
 		if (realsize >= memmap_pages) {
 			realsize -= memmap_pages;
-			printk(KERN_DEBUG
-				"  %s zone: %lu pages used for memmap\n",
+			mminit_debug_printk(MMINIT_TRACE, "memmap_init",
+				"%s zone: %lu pages used for memmap\n",
 				zone_names[j], memmap_pages);
 		} else
 			printk(KERN_WARNING
@@ -3361,7 +3363,8 @@ static void __paginginit free_area_init_
 		/* Account for reserved pages */
 		if (j == 0 && realsize > dma_reserve) {
 			realsize -= dma_reserve;
-			printk(KERN_DEBUG "  %s zone: %lu pages reserved\n",
+			mminit_debug_printk(MMINIT_TRACE, "memmap_init",
+					"%s zone: %lu pages reserved\n",
 					zone_names[0], dma_reserve);
 		}
 
@@ -3496,7 +3499,8 @@ void __init add_active_range(unsigned in
 {
 	int i;
 
-	printk(KERN_DEBUG "Entering add_active_range(%d, %lu, %lu) "
+	mminit_debug_printk(MMINIT_TRACE, "memory_register",
+			"Entering add_active_range(%d, %lu, %lu) "
 			  "%d entries of %d used\n",
 			  nid, start_pfn, end_pfn,
 			  nr_nodemap_entries, MAX_ACTIVE_REGIONS);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] Add a basic debugging framework for memory initialisation

[Ingo Molnar]

* Mel Gorman <mel@csn.ul.ie> wrote:

> +static __init int set_mminit_debug_level(char *str)
> +{
> +	get_option(&str, &mminit_debug_level);
> +	return 0;
> +}
> +early_param("mminit_debug_level", set_mminit_debug_level);

another small suggestion: could you please also add a Kconfig method of 
enabling it, dependent on KERNEL_DEBUG, default-off (for now). The best 
would be not a numeric switch but something that gets randomized by 
"make randconfig". I.e. an on/off switch kind of things.

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] Add a basic debugging framework for memory initialisation

[Mel Gorman]

On (16/04/08 16:04), Ingo Molnar didst pronounce:
> 
> * Mel Gorman <mel@csn.ul.ie> wrote:
> 
> > +static __init int set_mminit_debug_level(char *str)
> > +{
> > +	get_option(&str, &mminit_debug_level);
> > +	return 0;
> > +}
> > +early_param("mminit_debug_level", set_mminit_debug_level);
> 
> another small suggestion: could you please also add a Kconfig method of 
> enabling it, dependent on KERNEL_DEBUG, default-off (for now). The best 
> would be not a numeric switch but something that gets randomized by 
> "make randconfig". I.e. an on/off switch kind of things.
> 

Makes sense. I've this and your other suggestions incorporated. It'll be
part of V2.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 0/4] Verification and debugging of memory initialisation V2

[Mel Gorman]

Many changes are based on feedback from Ingo. Others are from off-list mails
that were not group-replied for some reason. A number of issues were rattled
out while testing on machines other than x86-32. V1 was pretty flaky and
barely a proof-of-concept but this version has successfully boot-tested on
x86-32, x86-64 and ppc64. It has been successfully cross-compiled on ARM
which does not support arch-independent zone-sizing.

Changelog since V1
  o Make memory initialisation verification a DEBUG option depending on
    DEBUG_KERNEL option. By default it will then to verify structures but
    tracing can be enabled via the command-line. Without the CONFIG option,
    checks will still be made on PFN ranges passed by the architecture-specific
    code and a warning printed once if a problem is encountered
  o Reshuffle the patches so that the zonelist printing is at the end of the
    patchset. This is because -mm requires a different patch to print zonelists
    and this allows the end patch to be temporarily dropped when testing against
    -mm
  o Rebase on top of Ingo's sparsemem fix for easier testing
  o WARN_ON_ONCE when PFNs from the architecture violate SPARSEMEM limitations.
    The warning should be "harmless" as the system will boot regardless but
    it acts as a reminder that bad input is being used.
  o Convert mminit_debug_printk() to a macro
  o Spelling mistake corrections
  o Proper use of KERN_CONT
  o Document mminit_debug_level=
  o Fix check on pageflags where the masks were not being shifted
  o The zone ID should should have used page_zonenum not page_zone_id
  o Iterate all zonelists correctly
  o Correct typo of SECTIONS_SHIFT

Boot initialisation has always been a bit of a mess with a number
of ugly points. While significant amounts of the initialisation
is architecture-independent, it trusts of the data received from the
architecture layer. This was a mistake in retrospect as it has resulted in
a number of difficult-to-diagnose bugs.

This patchset optionally adds some validation and tracing to memory
initialisation. It also introduces a few basic defencive measures and
depending on a boot parameter, will perform additional tests for errors
"that should never occur". I think this would have reduced debugging time
for some boot-related problems. 

One question. These patches generally print at KERN_INFO level on the
assumption if the user has compiled in the option, they are not expecting to
also have to set loglevel. However, it has been pointed out privately that this
may be confusing. Which level for printk would people find less surprising?

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/4] Add a basic debugging framework for memory initialisation

[Mel Gorman]

This patch creates a new file mm/mm_init.c which is conditionally
compiled to have almost all of the debugging and verification code to
avoid further polluting page_alloc.c. Ideally other mm initialisation code
will be moved here over time and the file partially compiled depending
on Kconfig. This patch introduces a simple mminit_debug_printk() macro
and an mminit_debug_level commmand-line parameter for setting the level of
tracing and verification that should be done.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 Documentation/kernel-parameters.txt |    8 ++++++++
 lib/Kconfig.debug                   |   12 ++++++++++++
 mm/Makefile                         |    1 +
 mm/internal.h                       |   26 ++++++++++++++++++++++++++
 mm/mm_init.c                        |   18 ++++++++++++++++++
 mm/page_alloc.c                     |   16 ++++++++++------
 6 files changed, 75 insertions(+), 6 deletions(-)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-ingo-fix-sparsemem/Documentation/kernel-parameters.txt linux-2.6.25-rc9-0010_mminit_debug_framework/Documentation/kernel-parameters.txt
--- linux-2.6.25-rc9-ingo-fix-sparsemem/Documentation/kernel-parameters.txt	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/Documentation/kernel-parameters.txt	2008-04-17 00:20:19.000000000 +0100
@@ -1148,6 +1148,14 @@ and is between 256 and 4096 characters. 
 
 	mga=		[HW,DRM]
 
+	mminit_debug_level=
+			[KNL] When CONFIG_DEBUG_MEMORY_INIT is set, this
+			parameter allows control of what level of debugging
+			and verification is done during memory initialisation.
+			A value of -1 disables the checks and a level of 4 will
+			enable tracing. By default basic verifications are made
+			when the Kconfig option is set
+
 	mousedev.tap_time=
 			[MOUSE] Maximum time between finger touching and
 			leaving touchpad surface for touch to be considered
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-ingo-fix-sparsemem/lib/Kconfig.debug linux-2.6.25-rc9-0010_mminit_debug_framework/lib/Kconfig.debug
--- linux-2.6.25-rc9-ingo-fix-sparsemem/lib/Kconfig.debug	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/lib/Kconfig.debug	2008-04-17 00:20:19.000000000 +0100
@@ -437,6 +437,18 @@ config DEBUG_VM
 
 	  If unsure, say N.
 
+config DEBUG_MEMORY_INIT
+	bool "Debug memory initialisation"
+	depends on DEBUG_KERNEL
+	help
+	  Enable this to turn on debug checks during memory initialisation. By
+	  default, sanity checks will be made on the memory model and
+	  information provided by the architecture. What level of checking
+	  made and verbosity during boot can be set with the
+	  mminit_debug_level= command-line option.
+
+	  If unsure, say N
+
 config DEBUG_LIST
 	bool "Debug linked list manipulation"
 	depends on DEBUG_KERNEL
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-ingo-fix-sparsemem/mm/internal.h linux-2.6.25-rc9-0010_mminit_debug_framework/mm/internal.h
--- linux-2.6.25-rc9-ingo-fix-sparsemem/mm/internal.h	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/mm/internal.h	2008-04-17 00:20:19.000000000 +0100
@@ -60,4 +60,30 @@ static inline unsigned long page_order(s
 #define __paginginit __init
 #endif
 
+/* Memory initialisation debug and verification */
+enum mminit_levels {
+	MMINIT_NORMAL = -1,
+	MMINIT_VERIFY,
+	MMINIT_TRACE
+};
+
+#ifdef CONFIG_DEBUG_MEMORY_INIT
+
+extern int mminit_debug_level;
+
+#define mminit_debug_printk(level, prefix, fmt, arg...) \
+do { \
+	if (level < mminit_debug_level) { \
+		printk(KERN_INFO "mminit::%s " fmt, prefix, ##arg); \
+	} \
+} while (0)
+
+#else
+
+static inline void mminit_debug_printk(unsigned int level, const char *prefix,
+				const char *fmt, ...)
+{
+}
+
+#endif /* CONFIG_DEBUG_MEMORY_INIT */
 #endif
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-ingo-fix-sparsemem/mm/Makefile linux-2.6.25-rc9-0010_mminit_debug_framework/mm/Makefile
--- linux-2.6.25-rc9-ingo-fix-sparsemem/mm/Makefile	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/mm/Makefile	2008-04-17 00:20:19.000000000 +0100
@@ -33,4 +33,5 @@ obj-$(CONFIG_MIGRATION) += migrate.o
 obj-$(CONFIG_SMP) += allocpercpu.o
 obj-$(CONFIG_QUICKLIST) += quicklist.o
 obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o
+obj-$(CONFIG_DEBUG_MEMORY_INIT) += mm_init.o
 
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-ingo-fix-sparsemem/mm/mm_init.c linux-2.6.25-rc9-0010_mminit_debug_framework/mm/mm_init.c
--- linux-2.6.25-rc9-ingo-fix-sparsemem/mm/mm_init.c	2008-04-16 10:42:54.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/mm/mm_init.c	2008-04-17 00:20:19.000000000 +0100
@@ -0,0 +1,18 @@
+/*
+ * mm_init.c - Memory initialisation verification and debugging
+ *
+ * Copyright 2008 IBM Corporation, 2008
+ * Author Mel Gorman <mel@csn.ul.ie>
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+
+int __initdata mminit_debug_level;
+
+static __init int set_mminit_debug_level(char *str)
+{
+	get_option(&str, &mminit_debug_level);
+	return 0;
+}
+early_param("mminit_debug_level", set_mminit_debug_level);
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-ingo-fix-sparsemem/mm/page_alloc.c linux-2.6.25-rc9-0010_mminit_debug_framework/mm/page_alloc.c
--- linux-2.6.25-rc9-ingo-fix-sparsemem/mm/page_alloc.c	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0010_mminit_debug_framework/mm/page_alloc.c	2008-04-17 00:20:19.000000000 +0100
@@ -2958,7 +2958,8 @@ void __init sparse_memory_present_with_a
 void __init push_node_boundaries(unsigned int nid,
 		unsigned long start_pfn, unsigned long end_pfn)
 {
-	printk(KERN_DEBUG "Entering push_node_boundaries(%u, %lu, %lu)\n",
+	mminit_debug_printk(MMINIT_TRACE, "zoneboundary",
+			"Entering push_node_boundaries(%u, %lu, %lu)\n",
 			nid, start_pfn, end_pfn);
 
 	/* Initialise the boundary for this node if necessary */
@@ -2976,7 +2977,8 @@ void __init push_node_boundaries(unsigne
 static void __meminit account_node_boundary(unsigned int nid,
 		unsigned long *start_pfn, unsigned long *end_pfn)
 {
-	printk(KERN_DEBUG "Entering account_node_boundary(%u, %lu, %lu)\n",
+	mminit_debug_printk(MMINIT_TRACE, "zoneboundary",
+			"Entering account_node_boundary(%u, %lu, %lu)\n",
 			nid, *start_pfn, *end_pfn);
 
 	/* Return if boundary information has not been provided */
@@ -3350,8 +3352,8 @@ static void __paginginit free_area_init_
 		memmap_pages = (size * sizeof(struct page)) >> PAGE_SHIFT;
 		if (realsize >= memmap_pages) {
 			realsize -= memmap_pages;
-			printk(KERN_DEBUG
-				"  %s zone: %lu pages used for memmap\n",
+			mminit_debug_printk(MMINIT_TRACE, "memmap_init",
+				"%s zone: %lu pages used for memmap\n",
 				zone_names[j], memmap_pages);
 		} else
 			printk(KERN_WARNING
@@ -3361,7 +3363,8 @@ static void __paginginit free_area_init_
 		/* Account for reserved pages */
 		if (j == 0 && realsize > dma_reserve) {
 			realsize -= dma_reserve;
-			printk(KERN_DEBUG "  %s zone: %lu pages reserved\n",
+			mminit_debug_printk(MMINIT_TRACE, "memmap_init",
+					"%s zone: %lu pages reserved\n",
 					zone_names[0], dma_reserve);
 		}
 
@@ -3496,7 +3499,8 @@ void __init add_active_range(unsigned in
 {
 	int i;
 
-	printk(KERN_DEBUG "Entering add_active_range(%d, %lu, %lu) "
+	mminit_debug_printk(MMINIT_TRACE, "memory_register",
+			"Entering add_active_range(%d, %lu, %lu) "
 			  "%d entries of %d used\n",
 			  nid, start_pfn, end_pfn,
 			  nr_nodemap_entries, MAX_ACTIVE_REGIONS);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/4] Verify the page links and memory model

[Mel Gorman]

This patch prints out information on how the page flags are being used and
verifies they are correct if mminit_debug_level is MMINIT_VERIFY or higher.
When the page flags are updated with section, node and zone information, an
additional check is made to ensure the values can be retrieved correctly. The
final check made with respect to pages is that pfn_to_page() and page_to_pfn()
are returning sensible values.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 mm/internal.h   |   12 +++++++++
 mm/mm_init.c    |   64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 mm/page_alloc.c |    6 ++++
 3 files changed, 82 insertions(+)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0010_mminit_debug_framework/mm/internal.h linux-2.6.25-rc9-0020_memmap_init_debug/mm/internal.h
--- linux-2.6.25-rc9-0010_mminit_debug_framework/mm/internal.h	2008-04-17 00:20:19.000000000 +0100
+++ linux-2.6.25-rc9-0020_memmap_init_debug/mm/internal.h	2008-04-17 00:20:33.000000000 +0100
@@ -78,6 +78,10 @@ do { \
 	} \
 } while (0)
 
+extern void mminit_verify_pageflags(void);
+extern void mminit_verify_page_links(struct page *page,
+		enum zone_type zone, unsigned long nid, unsigned long pfn);
+
 #else
 
 static inline void mminit_debug_printk(unsigned int level, const char *prefix,
@@ -85,5 +89,13 @@ static inline void mminit_debug_printk(u
 {
 }
 
+static inline void mminit_verify_pageflags(void)
+{
+}
+
+static inline void mminit_verify_page_links(struct page *page,
+		enum zone_type zone, unsigned long nid, unsigned long pfn)
+{
+}
 #endif /* CONFIG_DEBUG_MEMORY_INIT */
 #endif
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0010_mminit_debug_framework/mm/mm_init.c linux-2.6.25-rc9-0020_memmap_init_debug/mm/mm_init.c
--- linux-2.6.25-rc9-0010_mminit_debug_framework/mm/mm_init.c	2008-04-17 00:20:19.000000000 +0100
+++ linux-2.6.25-rc9-0020_memmap_init_debug/mm/mm_init.c	2008-04-17 00:20:33.000000000 +0100
@@ -7,9 +7,73 @@
  */
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include "internal.h"
 
 int __initdata mminit_debug_level;
 
+void __init mminit_verify_pageflags(void)
+{
+	unsigned long shift;
+	if (mminit_debug_level < MMINIT_VERIFY)
+		return;
+
+	mminit_debug_printk(MMINIT_TRACE, "pageflags_layout_widths",
+		"Section %d Node %d Zone %d Reserved %d\n",
+		SECTIONS_WIDTH,
+		NODES_WIDTH,
+		ZONES_WIDTH,
+		FLAGS_RESERVED);
+	mminit_debug_printk(MMINIT_TRACE, "pageflags_layout_shifts",
+		"Section %d Node %d Zone %d\n",
+#ifdef SECTIONS_SHIFT
+		SECTIONS_SHIFT,
+#else
+		0,
+#endif
+		NODES_SHIFT,
+		ZONES_SHIFT);
+	mminit_debug_printk(MMINIT_TRACE, "pageflags_layout_offsets",
+		"Section %d Node %d Zone %d\n",
+		SECTIONS_PGSHIFT,
+		NODES_PGSHIFT,
+		ZONES_PGSHIFT);
+	mminit_debug_printk(MMINIT_TRACE, "pageflags_layout_zoneid",
+		"Zone ID: %d -> %d\n",
+		ZONEID_PGOFF, ZONEID_PGOFF + ZONEID_SHIFT);
+#ifdef NODE_NOT_IN_PAGE_FLAGS
+	mminit_debug_printk(MMINIT_TRACE, "pageflags_layout_nodeflags",
+		"Node not in page flags");
+#endif
+
+	shift = 8 * sizeof(unsigned long);
+	if (SECTIONS_WIDTH) {
+		shift -= SECTIONS_WIDTH;
+		BUG_ON(shift != SECTIONS_PGSHIFT);
+	}
+	if (NODES_WIDTH) {
+		shift -= NODES_WIDTH;
+		BUG_ON(shift != NODES_PGSHIFT);
+	}
+	if (ZONES_WIDTH) {
+		shift -= ZONES_WIDTH;
+		BUG_ON(shift != ZONES_PGSHIFT);
+	}
+	BUG_ON((ZONES_MASK << ZONES_PGSHIFT) &
+			(NODES_MASK << NODES_PGSHIFT) &
+			(SECTIONS_MASK << SECTIONS_PGSHIFT));
+}
+
+void __meminit mminit_verify_page_links(struct page *page, enum zone_type zone,
+			unsigned long nid, unsigned long pfn)
+{
+	if (mminit_debug_level < MMINIT_VERIFY)
+		return;
+
+	BUG_ON(page_to_nid(page) != nid);
+	BUG_ON(page_zonenum(page) != zone);
+	BUG_ON(page_to_pfn(page) != pfn);
+}
+
 static __init int set_mminit_debug_level(char *str)
 {
 	get_option(&str, &mminit_debug_level);
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0010_mminit_debug_framework/mm/page_alloc.c linux-2.6.25-rc9-0020_memmap_init_debug/mm/page_alloc.c
--- linux-2.6.25-rc9-0010_mminit_debug_framework/mm/page_alloc.c	2008-04-17 00:20:19.000000000 +0100
+++ linux-2.6.25-rc9-0020_memmap_init_debug/mm/page_alloc.c	2008-04-17 00:20:33.000000000 +0100
@@ -2533,6 +2533,7 @@ void __meminit memmap_init_zone(unsigned
 		}
 		page = pfn_to_page(pfn);
 		set_page_links(page, zone, nid, pfn);
+		mminit_verify_page_links(page, zone, nid, pfn);
 		init_page_count(page);
 		reset_page_mapcount(page);
 		SetPageReserved(page);
@@ -2829,6 +2830,10 @@ __meminit int init_currently_empty_zone(
 
 	zone->zone_start_pfn = zone_start_pfn;
 
+	mminit_debug_printk(MMINIT_TRACE, "memmap_init",
+			"Initialising map node %d zone %d pfns %lu -> %lu\n",
+			pgdat->node_id, zone_idx(zone),
+			zone_start_pfn, (zone_start_pfn + size));
 	memmap_init(size, pgdat->node_id, zone_idx(zone), zone_start_pfn);
 
 	zone_init_free_lists(zone);
@@ -3896,6 +3901,7 @@ void __init free_area_init_nodes(unsigne
 						early_node_map[i].end_pfn);
 
 	/* Initialise every node */
+	mminit_verify_pageflags();
 	setup_nr_node_ids();
 	for_each_online_node(nid) {
 		pg_data_t *pgdat = NODE_DATA(nid);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/4] Make defencive checks around PFN values registered for memory usage

[Mel Gorman]

There are a number of different views to how much memory is currently
active. There is the arch-independent zone-sizing view, the bootmem allocator
and SPARSEMEMs view.  Architectures register this information at different
times and is not necessarily in sync particularly with view to some SPARSEMEM
limitations.

This patch introduces mminit_validate_physlimits() which is able to validate
and correct PFN ranges with respect to SPARSEMEM limitations. Ordinarily
they will be fixed silently but if mminit_debug_level is MMINIT_VERIFY or
higher, a message will be printed to dmesg.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 mm/bootmem.c    |    1 +
 mm/internal.h   |   12 ++++++++++++
 mm/page_alloc.c |    2 ++
 mm/sparse.c     |   37 +++++++++++++++++++++++++++++--------
 4 files changed, 44 insertions(+), 8 deletions(-)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0020_memmap_init_debug/mm/bootmem.c linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/bootmem.c
--- linux-2.6.25-rc9-0020_memmap_init_debug/mm/bootmem.c	2008-04-11 21:32:29.000000000 +0100
+++ linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/bootmem.c	2008-04-17 00:20:47.000000000 +0100
@@ -91,6 +91,7 @@ static unsigned long __init init_bootmem
 	bootmem_data_t *bdata = pgdat->bdata;
 	unsigned long mapsize;
 
+	mminit_validate_physlimits(&start, &end);
 	bdata->node_bootmem_map = phys_to_virt(PFN_PHYS(mapstart));
 	bdata->node_boot_start = PFN_PHYS(start);
 	bdata->node_low_pfn = end;
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0020_memmap_init_debug/mm/internal.h linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/internal.h
--- linux-2.6.25-rc9-0020_memmap_init_debug/mm/internal.h	2008-04-17 00:20:33.000000000 +0100
+++ linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/internal.h	2008-04-17 00:20:47.000000000 +0100
@@ -98,4 +98,16 @@ static inline void mminit_verify_page_li
 {
 }
 #endif /* CONFIG_DEBUG_MEMORY_INIT */
+
+/* mminit_validate_physlimits is independent of CONFIG_DEBUG_MEMORY_INIT */
+#if defined(CONFIG_SPARSEMEM)
+extern void mminit_validate_physlimits(unsigned long *start_pfn,
+				unsigned long *end_pfn);
+#else
+static inline void mminit_validate_physlimits(unsigned long *start_pfn,
+				unsigned long *end_pfn)
+{
+}
+#endif /* CONFIG_SPARSEMEM */
+
 #endif
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0020_memmap_init_debug/mm/page_alloc.c linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/page_alloc.c
--- linux-2.6.25-rc9-0020_memmap_init_debug/mm/page_alloc.c	2008-04-17 00:20:33.000000000 +0100
+++ linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/page_alloc.c	2008-04-17 00:20:47.000000000 +0100
@@ -3510,6 +3510,8 @@ void __init add_active_range(unsigned in
 			  nid, start_pfn, end_pfn,
 			  nr_nodemap_entries, MAX_ACTIVE_REGIONS);
 
+	mminit_validate_physlimits(&start_pfn, &end_pfn);
+
 	/* Merge with existing active regions if possible */
 	for (i = 0; i < nr_nodemap_entries; i++) {
 		if (early_node_map[i].nid != nid)
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0020_memmap_init_debug/mm/sparse.c linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/sparse.c
--- linux-2.6.25-rc9-0020_memmap_init_debug/mm/sparse.c	2008-04-16 15:47:34.000000000 +0100
+++ linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/sparse.c	2008-04-17 00:20:47.000000000 +0100
@@ -11,6 +11,7 @@
 #include <asm/dma.h>
 #include <asm/pgalloc.h>
 #include <asm/pgtable.h>
+#include "internal.h"
 
 /*
  * Permanent SPARSEMEM data:
@@ -146,22 +147,41 @@ static inline int sparse_early_nid(struc
 	return (section->section_mem_map >> SECTION_NID_SHIFT);
 }
 
-/* Record a memory area against a node. */
-void __init memory_present(int nid, unsigned long start, unsigned long end)
+/* Validate the physical addressing limitations of the model */
+void __meminit mminit_validate_physlimits(unsigned long *start_pfn,
+						unsigned long *end_pfn)
 {
-	unsigned long max_arch_pfn = 1UL << (MAX_PHYSMEM_BITS-PAGE_SHIFT);
-	unsigned long pfn;
+	unsigned long max_sparsemem_pfn = 1UL << (MAX_PHYSMEM_BITS-PAGE_SHIFT);
 
 	/*
 	 * Sanity checks - do not allow an architecture to pass
 	 * in larger pfns than the maximum scope of sparsemem:
 	 */
-	if (start >= max_arch_pfn)
-		return;
-	if (end >= max_arch_pfn)
-		end = max_arch_pfn;
+	if (*start_pfn > max_sparsemem_pfn) {
+		mminit_debug_printk(MMINIT_VERIFY, "pfnvalidation",
+			"Start of range %lu -> %lu exceeds SPARSEMEM max %lu\n",
+			*start_pfn, *end_pfn, max_sparsemem_pfn);
+		WARN_ON_ONCE(1);
+		*start_pfn = max_sparsemem_pfn;
+		*end_pfn = max_sparsemem_pfn;
+	}
+
+	if (*end_pfn > max_sparsemem_pfn) {
+		mminit_debug_printk(MMINIT_VERIFY, "pfnvalidation",
+			"End of range %lu -> %lu exceeds SPARSEMEM max %lu\n",
+			*start_pfn, *end_pfn, max_sparsemem_pfn);
+		WARN_ON_ONCE(1);
+		*end_pfn = max_sparsemem_pfn;
+	}
+}
+
+/* Record a memory area against a node. */
+void __init memory_present(int nid, unsigned long start, unsigned long end)
+{
+	unsigned long pfn;
 
 	start &= PAGE_SECTION_MASK;
+	mminit_validate_physlimits(&start, &end);
 	for (pfn = start; pfn < end; pfn += PAGES_PER_SECTION) {
 		unsigned long section = pfn_to_section_nr(pfn);
 		struct mem_section *ms;
@@ -186,6 +206,7 @@ unsigned long __init node_memmap_size_by
 	unsigned long pfn;
 	unsigned long nr_pages = 0;
 
+	mminit_validate_physlimits(&start_pfn, &end_pfn);
 	for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
 		if (nid != early_pfn_to_nid(pfn))
 			continue;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 4/4] Print out the zonelists on request for manual verification

[Mel Gorman]

This patch prints out the zonelists during boot for manual verification
by the user. This is useful for checking if the zonelists were somehow
corrupt during initialisation. Note that this patch will not work in -mm
due to differences in how zonelists are used.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 mm/internal.h   |    5 +++++
 mm/mm_init.c    |   40 ++++++++++++++++++++++++++++++++++++++++
 mm/page_alloc.c |    1 +
 3 files changed, 46 insertions(+)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/internal.h linux-2.6.25-rc9-0030_display_zonelist/mm/internal.h
--- linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/internal.h	2008-04-17 00:20:47.000000000 +0100
+++ linux-2.6.25-rc9-0030_display_zonelist/mm/internal.h	2008-04-17 00:21:07.000000000 +0100
@@ -81,6 +81,7 @@ do { \
 extern void mminit_verify_pageflags(void);
 extern void mminit_verify_page_links(struct page *page,
 		enum zone_type zone, unsigned long nid, unsigned long pfn);
+extern void mminit_verify_zonelist(void);
 
 #else
 
@@ -97,6 +98,10 @@ static inline void mminit_verify_page_li
 		enum zone_type zone, unsigned long nid, unsigned long pfn)
 {
 }
+
+static inline void mminit_verify_zonelist(void)
+{
+}
 #endif /* CONFIG_DEBUG_MEMORY_INIT */
 
 /* mminit_validate_physlimits is independent of CONFIG_DEBUG_MEMORY_INIT */
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/mm_init.c linux-2.6.25-rc9-0030_display_zonelist/mm/mm_init.c
--- linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/mm_init.c	2008-04-17 00:20:33.000000000 +0100
+++ linux-2.6.25-rc9-0030_display_zonelist/mm/mm_init.c	2008-04-17 00:21:07.000000000 +0100
@@ -11,6 +11,46 @@
 
 int __initdata mminit_debug_level;
 
+/* Note that the verification of correctness is required from the user */
+void mminit_verify_zonelist(void)
+{
+	int nid;
+
+	if (mminit_debug_level < MMINIT_VERIFY)
+		return;
+
+	for_each_online_node(nid) {
+		pg_data_t *pgdat = NODE_DATA(nid);
+		struct zone *zone;
+		struct zone **z;
+		int listid;
+
+		for (listid = 0; listid < MAX_ZONELISTS; listid++) {
+			zone = &pgdat->node_zones[listid % MAX_NR_ZONES];
+
+			if (!populated_zone(zone))
+				continue;
+
+			printk(KERN_INFO "mminit::zonelist %s %d:%s = ",
+				listid >= MAX_NR_ZONES ? "thisnode" : "general",
+				nid,
+				zone->name);
+			z = pgdat->node_zonelists[listid].zones;
+
+			while (*z != NULL) {
+#ifdef CONFIG_NUMA
+				printk(KERN_CONT "%d:%s ",
+						(*z)->node, (*z)->name);
+#else
+				printk(KERN_CONT "0:%s ", (*z)->name);
+#endif
+				z++;
+			}
+			printk(KERN_CONT "\n");
+		}
+	}
+}
+
 void __init mminit_verify_pageflags(void)
 {
 	unsigned long shift;
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/page_alloc.c linux-2.6.25-rc9-0030_display_zonelist/mm/page_alloc.c
--- linux-2.6.25-rc9-0025_defensive_pfn_checks/mm/page_alloc.c	2008-04-17 00:20:47.000000000 +0100
+++ linux-2.6.25-rc9-0030_display_zonelist/mm/page_alloc.c	2008-04-17 00:21:07.000000000 +0100
@@ -2353,6 +2353,7 @@ void build_all_zonelists(void)
 
 	if (system_state == SYSTEM_BOOTING) {
 		__build_all_zonelists(NULL);
+		mminit_verify_zonelist();
 		cpuset_init_current_mems_allowed();
 	} else {
 		/* we have to stop all cpus to guarantee there is no user

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] Add a basic debugging framework for memory initialisation

[Ingo Molnar]

* Mel Gorman <mel@csn.ul.ie> wrote:

> +config DEBUG_MEMORY_INIT
> +	bool "Debug memory initialisation"
> +	depends on DEBUG_KERNEL
> +	help
> +	  Enable this to turn on debug checks during memory initialisation. By
> +	  default, sanity checks will be made on the memory model and
> +	  information provided by the architecture. What level of checking
> +	  made and verbosity during boot can be set with the
> +	  mminit_debug_level= command-line option.
> +
> +	  If unsure, say N

should be "default y" - and perhaps only disable-able on 
CONFIG_EMBEDDED. We generally want such bugs to pop up as soon as 
possible, and the sanity checks should only go away if someone 
specifically aims for lowest system footprint.

the default loglevel for debug printouts might deserve another debug 
option - but the core checks should always be included, and _errors_ 
should always be printed out.

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] Add a basic debugging framework for memory initialisation

[Mel Gorman]

On (21/04/08 17:14), Ingo Molnar didst pronounce:
> 
> * Mel Gorman <mel@csn.ul.ie> wrote:
> 
> > +config DEBUG_MEMORY_INIT
> > +	bool "Debug memory initialisation"
> > +	depends on DEBUG_KERNEL
> > +	help
> > +	  Enable this to turn on debug checks during memory initialisation. By
> > +	  default, sanity checks will be made on the memory model and
> > +	  information provided by the architecture. What level of checking
> > +	  made and verbosity during boot can be set with the
> > +	  mminit_debug_level= command-line option.
> > +
> > +	  If unsure, say N
> 
> should be "default y" - and perhaps only disable-able on 
> CONFIG_EMBEDDED.

Ok, that would be something like the following?

       bool "Debug memory initialisation" if DEBUG_KERNEL && EMBEDDED
       depends on DEBUG_KERNEL
       default !EMBEDDED

This will slow up boot slightly on debug kernels as the additional checks
are made. It'll remain to be seen as to whether this is a problem for people
or not. I doubt it'll be noticed.

> We generally want such bugs to pop up as soon as 
> possible, and the sanity checks should only go away if someone 
> specifically aims for lowest system footprint.
> 

Seems fair and it's the second time this has been suggested (off-list reviewer
again). The only potential gotcha is if a sanity check is introduced that is
itself broken. It should be very obvious when this type of bug occurs though.

> the default loglevel for debug printouts might deserve another debug 
> option - but the core checks should always be included, and _errors_ 
> should always be printed out.
> 

I'll replace mminit_debug_level with mminit_loglevel that determines whether
information is printed at KERN_DEBUG or not. This matches what other similar
debug-frameworks are doing. I'll make sure errors always get printed.

Thanks

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/4] Add a basic debugging framework for memory initialisation

[Mel Gorman]

This patch creates a new file mm/mm_init.c which is conditionally compiled
to have almost all of the debugging and verification code to avoid further
polluting page_alloc.c. Ideally other mm initialisation code will be moved
here over time and the file partially compiled depending on Kconfig. This
patch introduces a simple mminit_debug_printk() macro and an mminit_loglevel
command-line parameter. Tracing and verification is always done but the
amount of information logged at KERN_DEBUG level can be controlled.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 Documentation/kernel-parameters.txt |    8 ++++++++
 lib/Kconfig.debug                   |   13 +++++++++++++
 mm/Makefile                         |    1 +
 mm/internal.h                       |   26 ++++++++++++++++++++++++++
 mm/mm_init.c                        |   18 ++++++++++++++++++
 mm/page_alloc.c                     |   16 ++++++++++------
 6 files changed, 76 insertions(+), 6 deletions(-)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/Documentation/kernel-parameters.txt linux-2.6.25-mm1-0010_mminit_debug_framework/Documentation/kernel-parameters.txt
--- linux-2.6.25-mm1-clean/Documentation/kernel-parameters.txt	2008-04-22 10:29:56.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/Documentation/kernel-parameters.txt	2008-04-22 17:47:13.000000000 +0100
@@ -1185,6 +1185,14 @@ and is between 256 and 4096 characters. 
 
 	mga=		[HW,DRM]
 
+	mminit_loglevel=
+			[KNL] When CONFIG_DEBUG_MEMORY_INIT is set, this
+			parameter allows control of the logging verbosity for
+			the additional memory initialisation checks. A value
+			of -1 disables mminit logging and a level of 2 will
+			log everything. Information is printed at KERN_DEBUG
+			so loglevel=9 may also need to be specified.
+
 	mousedev.tap_time=
 			[MOUSE] Maximum time between finger touching and
 			leaving touchpad surface for touch to be considered
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/lib/Kconfig.debug linux-2.6.25-mm1-0010_mminit_debug_framework/lib/Kconfig.debug
--- linux-2.6.25-mm1-clean/lib/Kconfig.debug	2008-04-22 10:30:04.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/lib/Kconfig.debug	2008-04-22 17:47:13.000000000 +0100
@@ -482,6 +482,19 @@ config DEBUG_WRITECOUNT
 
 	  If unsure, say N.
 
+config DEBUG_MEMORY_INIT
+	bool "Debug memory initialisation" if DEBUG_VM && EMBEDDED
+	depends on DEBUG_VM
+	default !EMBEDDED
+	help
+	  Enable this for additional checks during memory initialisation.
+	  The sanity checks verify aspects of the VM such as the memory model
+	  and other information provided by the architecture. Verbose
+	  information will be printed at KERN_DEBUG loglevel depending 
+	  on the mminit_loglevel= command-line option.
+
+	  If unsure, say N
+
 config DEBUG_LIST
 	bool "Debug linked list manipulation"
 	depends on DEBUG_KERNEL
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/mm/internal.h linux-2.6.25-mm1-0010_mminit_debug_framework/mm/internal.h
--- linux-2.6.25-mm1-clean/mm/internal.h	2008-04-22 10:30:04.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/mm/internal.h	2008-04-22 17:47:13.000000000 +0100
@@ -59,4 +59,30 @@ static inline unsigned long page_order(s
 #define __paginginit __init
 #endif
 
+/* Memory initialisation debug and verification */
+enum mminit_levels {
+	MMINIT_NORMAL = -1,
+	MMINIT_VERIFY,
+	MMINIT_TRACE
+};
+
+#ifdef CONFIG_DEBUG_MEMORY_INIT
+
+extern int mminit_loglevel;
+
+#define mminit_debug_printk(level, prefix, fmt, arg...) \
+do { \
+	if (level < mminit_loglevel) { \
+		printk(KERN_DEBUG "mminit:: " prefix " " fmt, ##arg); \
+	} \
+} while (0)
+
+#else
+
+static inline void mminit_debug_printk(unsigned int level, const char *prefix,
+				const char *fmt, ...)
+{
+}
+
+#endif /* CONFIG_DEBUG_MEMORY_INIT */
 #endif
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/mm/Makefile linux-2.6.25-mm1-0010_mminit_debug_framework/mm/Makefile
--- linux-2.6.25-mm1-clean/mm/Makefile	2008-04-22 10:30:04.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/mm/Makefile	2008-04-22 17:47:13.000000000 +0100
@@ -33,4 +33,5 @@ obj-$(CONFIG_MIGRATION) += migrate.o
 obj-$(CONFIG_SMP) += allocpercpu.o
 obj-$(CONFIG_QUICKLIST) += quicklist.o
 obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o
+obj-$(CONFIG_DEBUG_MEMORY_INIT) += mm_init.o
 
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/mm/mm_init.c linux-2.6.25-mm1-0010_mminit_debug_framework/mm/mm_init.c
--- linux-2.6.25-mm1-clean/mm/mm_init.c	2008-04-22 12:29:06.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/mm/mm_init.c	2008-04-22 17:47:13.000000000 +0100
@@ -0,0 +1,18 @@
+/*
+ * mm_init.c - Memory initialisation verification and debugging
+ *
+ * Copyright 2008 IBM Corporation, 2008
+ * Author Mel Gorman <mel@csn.ul.ie>
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+
+int __meminitdata mminit_loglevel;
+
+static __init int set_mminit_loglevel(char *str)
+{
+	get_option(&str, &mminit_loglevel);
+	return 0;
+}
+early_param("mminit_loglevel", set_mminit_loglevel);
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/mm/page_alloc.c linux-2.6.25-mm1-0010_mminit_debug_framework/mm/page_alloc.c
--- linux-2.6.25-mm1-clean/mm/page_alloc.c	2008-04-22 10:30:04.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/mm/page_alloc.c	2008-04-22 17:47:13.000000000 +0100
@@ -3068,7 +3068,8 @@ void __init sparse_memory_present_with_a
 void __init push_node_boundaries(unsigned int nid,
 		unsigned long start_pfn, unsigned long end_pfn)
 {
-	printk(KERN_DEBUG "Entering push_node_boundaries(%u, %lu, %lu)\n",
+	mminit_debug_printk(MMINIT_TRACE, "zoneboundary",
+			"Entering push_node_boundaries(%u, %lu, %lu)\n",
 			nid, start_pfn, end_pfn);
 
 	/* Initialise the boundary for this node if necessary */
@@ -3086,7 +3087,8 @@ void __init push_node_boundaries(unsigne
 static void __meminit account_node_boundary(unsigned int nid,
 		unsigned long *start_pfn, unsigned long *end_pfn)
 {
-	printk(KERN_DEBUG "Entering account_node_boundary(%u, %lu, %lu)\n",
+	mminit_debug_printk(MMINIT_TRACE, "zoneboundary",
+			"Entering account_node_boundary(%u, %lu, %lu)\n",
 			nid, *start_pfn, *end_pfn);
 
 	/* Return if boundary information has not been provided */
@@ -3460,8 +3462,8 @@ static void __paginginit free_area_init_
 		memmap_pages = (size * sizeof(struct page)) >> PAGE_SHIFT;
 		if (realsize >= memmap_pages) {
 			realsize -= memmap_pages;
-			printk(KERN_DEBUG
-				"  %s zone: %lu pages used for memmap\n",
+			mminit_debug_printk(MMINIT_TRACE, "memmap_init",
+				"%s zone: %lu pages used for memmap\n",
 				zone_names[j], memmap_pages);
 		} else
 			printk(KERN_WARNING
@@ -3471,7 +3473,8 @@ static void __paginginit free_area_init_
 		/* Account for reserved pages */
 		if (j == 0 && realsize > dma_reserve) {
 			realsize -= dma_reserve;
-			printk(KERN_DEBUG "  %s zone: %lu pages reserved\n",
+			mminit_debug_printk(MMINIT_TRACE, "memmap_init",
+					"%s zone: %lu pages reserved\n",
 					zone_names[0], dma_reserve);
 		}
 
@@ -3609,7 +3612,8 @@ void __init add_active_range(unsigned in
 {
 	int i;
 
-	printk(KERN_DEBUG "Entering add_active_range(%d, %lu, %lu) "
+	mminit_debug_printk(MMINIT_TRACE, "memory_register",
+			"Entering add_active_range(%d, %lu, %lu) "
 			  "%d entries of %d used\n",
 			  nid, start_pfn, end_pfn,
 			  nr_nodemap_entries, MAX_ACTIVE_REGIONS);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] Add a basic debugging framework for memory initialisation

[Andrew Morton]

> On Tue, 22 Apr 2008 19:31:53 +0100 (IST) Mel Gorman <mel@csn.ul.ie> wrote:
>
> This patch creates a new file mm/mm_init.c which is conditionally compiled
> to have almost all of the debugging and verification code to avoid further
> polluting page_alloc.c. Ideally other mm initialisation code will be moved
> here over time and the file partially compiled depending on Kconfig.

I was wondering why the file was misnamed ;)

I worry that

a) MM developers will forget to turn on the debug option (ask me about
   this) and the code in mm_init.c will break and 

b) The mm_init.c code is broken (or will break) on some architecture(s)
   and people who run that arch won't turn on the debug option either.

So hm.  I think that we should be more inclined to at least compile the
code even if we don't run it.  To catch compile-time breakage.

And it would be good if we could have a super-quick version of the checks
just so that more people at least partially run them.  Or something.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/4] Add a basic debugging framework for memory initialisation

[Mel Gorman]

On (25/04/08 23:10), Andrew Morton didst pronounce:
> > On Tue, 22 Apr 2008 19:31:53 +0100 (IST) Mel Gorman <mel@csn.ul.ie> wrote:
> >
> > This patch creates a new file mm/mm_init.c which is conditionally compiled
> > to have almost all of the debugging and verification code to avoid further
> > polluting page_alloc.c. Ideally other mm initialisation code will be moved
> > here over time and the file partially compiled depending on Kconfig.
> 
> I was wondering why the file was misnamed ;)
> 

:) I named it mm_debug.c at one point before figuring another attempt to
move the init code out of page_alloc.c might help make that file more
readable in the long term.

> I worry that
> 
> a) MM developers will forget to turn on the debug option (ask me about
>    this) and the code in mm_init.c will break and 
> 
> b) The mm_init.c code is broken (or will break) on some architecture(s)
>    and people who run that arch won't turn on the debug option either.
> 
> So hm.  I think that we should be more inclined to at least compile the
> code even if we don't run it.  To catch compile-time breakage.
> 

Ok, that seems fair as CONFIG_DEBUG_VM is probably not enabled a lot of the
time. I had slightly different concerns where it would compile fine, but the
verification code would be broken at runtime due to some change in the core
(zonelist handling for example) and not caught quickly. I will set this
to always build unless CONFIG_EMBEDDED in which case it iss optional. The
default logging level will be set to only print messages on errors. The
possibility will exist that a change later will cause compile-breakage on
CONFIG_EMBEDDED && !CONFIG_DEBUG_MEMORY_INIT.

Is this a reasonable approach?

> And it would be good if we could have a super-quick version of the checks
> just so that more people at least partially run them.  Or something.
> 

As it is, the checks should not cause noticable slowdown on boot. The
most common tests is the one run on each initialised struct page

+void __meminit mminit_verify_page_links(struct page *page, enum zone_type zone,
+                       unsigned long nid, unsigned long pfn)
+{
+       BUG_ON(page_to_nid(page) != nid);
+       BUG_ON(page_zonenum(page) != zone);
+       BUG_ON(page_to_pfn(page) != pfn);
+}

I doubt the overhead will be noticed.

If future expensive tests become, they could run conditionally depending
on the value of mminit_level=.

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/4] Add a basic debugging framework for memory initialisation

[Mel Gorman]

This patch adds additional debugging and verification code for memory
initialisation. Once enabled, the verification checks are always run and
when required additional debugging information may be outputted via a
mminit_loglevel= command-line parameter. The verification code is placed
in a new file mm/mm_init.c. Ideally other mm initialisation code will be
moved here over time.

Signed-off-by: Mel Gorman <mel@csn.ul.ie>
---

 Documentation/kernel-parameters.txt |    8 ++++++++
 lib/Kconfig.debug                   |   12 ++++++++++++
 mm/Makefile                         |    1 +
 mm/internal.h                       |   27 +++++++++++++++++++++++++++
 mm/mm_init.c                        |   18 ++++++++++++++++++
 mm/page_alloc.c                     |   16 ++++++++++------
 6 files changed, 76 insertions(+), 6 deletions(-)

diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/Documentation/kernel-parameters.txt linux-2.6.25-mm1-0010_mminit_debug_framework/Documentation/kernel-parameters.txt
--- linux-2.6.25-mm1-clean/Documentation/kernel-parameters.txt	2008-04-22 10:29:56.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/Documentation/kernel-parameters.txt	2008-04-28 14:39:59.000000000 +0100
@@ -1185,6 +1185,14 @@ and is between 256 and 4096 characters. 
 
 	mga=		[HW,DRM]
 
+	mminit_loglevel=
+			[KNL] When CONFIG_DEBUG_MEMORY_INIT is set, this
+			parameter allows control of the logging verbosity for
+			the additional memory initialisation checks. A value
+			of 0 disables mminit logging and a level of 4 will
+			log everything. Information is printed at KERN_DEBUG
+			so loglevel=8 may also need to be specified.
+
 	mousedev.tap_time=
 			[MOUSE] Maximum time between finger touching and
 			leaving touchpad surface for touch to be considered
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/lib/Kconfig.debug linux-2.6.25-mm1-0010_mminit_debug_framework/lib/Kconfig.debug
--- linux-2.6.25-mm1-clean/lib/Kconfig.debug	2008-04-22 10:30:04.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/lib/Kconfig.debug	2008-04-28 14:39:59.000000000 +0100
@@ -482,6 +482,18 @@ config DEBUG_WRITECOUNT
 
 	  If unsure, say N.
 
+config DEBUG_MEMORY_INIT
+	bool "Debug memory initialisation" if EMBEDDED
+	default !EMBEDDED
+	help
+	  Enable this for additional checks during memory initialisation.
+	  The sanity checks verify aspects of the VM such as the memory model
+	  and other information provided by the architecture. Verbose
+	  information will be printed at KERN_DEBUG loglevel depending 
+	  on the mminit_loglevel= command-line option.
+
+	  If unsure, say Y
+
 config DEBUG_LIST
 	bool "Debug linked list manipulation"
 	depends on DEBUG_KERNEL
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/mm/internal.h linux-2.6.25-mm1-0010_mminit_debug_framework/mm/internal.h
--- linux-2.6.25-mm1-clean/mm/internal.h	2008-04-22 10:30:04.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/mm/internal.h	2008-04-28 14:39:59.000000000 +0100
@@ -59,4 +59,31 @@ static inline unsigned long page_order(s
 #define __paginginit __init
 #endif
 
+/* Memory initialisation debug and verification */
+enum mminit_level {
+	MMINIT_WARNING,
+	MMINIT_VERIFY,
+	MMINIT_TRACE
+};
+
+#ifdef CONFIG_DEBUG_MEMORY_INIT
+
+extern int mminit_loglevel;
+
+#define mminit_dprintk(level, prefix, fmt, arg...) \
+do { \
+	if (level < mminit_loglevel) { \
+		printk(level <= MMINIT_WARNING ? KERN_WARNING : KERN_DEBUG \
+			"mminit:: " prefix " " fmt, ##arg); \
+	} \
+} while (0)
+
+#else
+
+static inline void mminit_dprintk(enum mminit_level level,
+				const char *prefix, const char *fmt, ...)
+{
+}
+
+#endif /* CONFIG_DEBUG_MEMORY_INIT */
 #endif
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/mm/Makefile linux-2.6.25-mm1-0010_mminit_debug_framework/mm/Makefile
--- linux-2.6.25-mm1-clean/mm/Makefile	2008-04-22 10:30:04.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/mm/Makefile	2008-04-28 14:39:59.000000000 +0100
@@ -33,4 +33,5 @@ obj-$(CONFIG_MIGRATION) += migrate.o
 obj-$(CONFIG_SMP) += allocpercpu.o
 obj-$(CONFIG_QUICKLIST) += quicklist.o
 obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o
+obj-$(CONFIG_DEBUG_MEMORY_INIT) += mm_init.o
 
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/mm/mm_init.c linux-2.6.25-mm1-0010_mminit_debug_framework/mm/mm_init.c
--- linux-2.6.25-mm1-clean/mm/mm_init.c	2008-04-22 12:29:06.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/mm/mm_init.c	2008-04-28 14:39:59.000000000 +0100
@@ -0,0 +1,18 @@
+/*
+ * mm_init.c - Memory initialisation verification and debugging
+ *
+ * Copyright 2008 IBM Corporation, 2008
+ * Author Mel Gorman <mel@csn.ul.ie>
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/init.h>
+
+int __meminitdata mminit_loglevel;
+
+static __init int set_mminit_loglevel(char *str)
+{
+	get_option(&str, &mminit_loglevel);
+	return 0;
+}
+early_param("mminit_loglevel", set_mminit_loglevel);
diff -rup -X /usr/src/patchset-0.6/bin//dontdiff linux-2.6.25-mm1-clean/mm/page_alloc.c linux-2.6.25-mm1-0010_mminit_debug_framework/mm/page_alloc.c
--- linux-2.6.25-mm1-clean/mm/page_alloc.c	2008-04-22 10:30:04.000000000 +0100
+++ linux-2.6.25-mm1-0010_mminit_debug_framework/mm/page_alloc.c	2008-04-28 14:39:59.000000000 +0100
@@ -3068,7 +3068,8 @@ void __init sparse_memory_present_with_a
 void __init push_node_boundaries(unsigned int nid,
 		unsigned long start_pfn, unsigned long end_pfn)
 {
-	printk(KERN_DEBUG "Entering push_node_boundaries(%u, %lu, %lu)\n",
+	mminit_dprintk(MMINIT_TRACE, "zoneboundary",
+			"Entering push_node_boundaries(%u, %lu, %lu)\n",
 			nid, start_pfn, end_pfn);
 
 	/* Initialise the boundary for this node if necessary */
@@ -3086,7 +3087,8 @@ void __init push_node_boundaries(unsigne
 static void __meminit account_node_boundary(unsigned int nid,
 		unsigned long *start_pfn, unsigned long *end_pfn)
 {
-	printk(KERN_DEBUG "Entering account_node_boundary(%u, %lu, %lu)\n",
+	mminit_dprintk(MMINIT_TRACE, "zoneboundary",
+			"Entering account_node_boundary(%u, %lu, %lu)\n",
 			nid, *start_pfn, *end_pfn);
 
 	/* Return if boundary information has not been provided */
@@ -3460,8 +3462,8 @@ static void __paginginit free_area_init_
 		memmap_pages = (size * sizeof(struct page)) >> PAGE_SHIFT;
 		if (realsize >= memmap_pages) {
 			realsize -= memmap_pages;
-			printk(KERN_DEBUG
-				"  %s zone: %lu pages used for memmap\n",
+			mminit_dprintk(MMINIT_TRACE, "memmap_init",
+				"%s zone: %lu pages used for memmap\n",
 				zone_names[j], memmap_pages);
 		} else
 			printk(KERN_WARNING
@@ -3471,7 +3473,8 @@ static void __paginginit free_area_init_
 		/* Account for reserved pages */
 		if (j == 0 && realsize > dma_reserve) {
 			realsize -= dma_reserve;
-			printk(KERN_DEBUG "  %s zone: %lu pages reserved\n",
+			mminit_dprintk(MMINIT_TRACE, "memmap_init",
+					"%s zone: %lu pages reserved\n",
 					zone_names[0], dma_reserve);
 		}
 
@@ -3609,7 +3612,8 @@ void __init add_active_range(unsigned in
 {
 	int i;
 
-	printk(KERN_DEBUG "Entering add_active_range(%d, %lu, %lu) "
+	mminit_dprintk(MMINIT_TRACE, "memory_register",
+			"Entering add_active_range(%d, %lu, %lu) "
 			  "%d entries of %d used\n",
 			  nid, start_pfn, end_pfn,
 			  nr_nodemap_entries, MAX_ACTIVE_REGIONS);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>