[PATCH 1/3] corruption check: move the corruption checks into their own file

[PATCH 1/3] corruption check: move the corruption checks into their own file

[Arjan van de Ven]

>From 19058f850aaf82d8185f06b54582a3d8e1ca8d97 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Mon, 22 Sep 2008 09:35:06 -0700
Subject: [PATCH] corruption check: move the corruption checks into their own file

There'll be more checks over time (based on kerneloops data I have
several ideas already) and making the checks their own file rather
than setup.c makes sense in that light.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 arch/x86/kernel/Makefile |    1 +
 arch/x86/kernel/check.c  |  156 ++++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/setup.c  |  151 --------------------------------------------
 include/asm-x86/setup.h  |    4 +
 4 files changed, 161 insertions(+), 151 deletions(-)
 create mode 100644 arch/x86/kernel/check.c

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index c71722d..5d788ea 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -36,6 +36,7 @@ obj-y			+= bootflag.o e820.o
 obj-y			+= pci-dma.o quirks.o i8237.o topology.o kdebugfs.o
 obj-y			+= alternative.o i8253.o pci-nommu.o
 obj-y			+= tsc.o io_delay.o rtc.o
+obj-y			+= check.o
 
 obj-$(CONFIG_X86_TRAMPOLINE)	+= trampoline.o
 obj-y				+= process.o
diff --git a/arch/x86/kernel/check.c b/arch/x86/kernel/check.c
new file mode 100644
index 0000000..a7e80ed
--- /dev/null
+++ b/arch/x86/kernel/check.c
@@ -0,0 +1,156 @@
+#include <linux/module.h>
+#include <linux/sched.h>
+
+#include <asm/e820.h>
+#include <asm/proto.h>
+
+/*
+ * Some BIOSes seem to corrupt the low 64k of memory during events
+ * like suspend/resume and unplugging an HDMI cable.  Reserve all
+ * remaining free memory in that area and fill it with a distinct
+ * pattern.
+ */
+#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
+#define MAX_SCAN_AREAS	8
+
+static int __read_mostly memory_corruption_check = -1;
+
+static unsigned __read_mostly corruption_check_size = 64*1024;
+static unsigned __read_mostly corruption_check_period = 60; /* seconds */
+
+static struct e820entry scan_areas[MAX_SCAN_AREAS];
+static int num_scan_areas;
+
+
+static int set_corruption_check(char *arg)
+{
+	char *end;
+
+	memory_corruption_check = simple_strtol(arg, &end, 10);
+
+	return (*end == 0) ? 0 : -EINVAL;
+}
+early_param("memory_corruption_check", set_corruption_check);
+
+static int set_corruption_check_period(char *arg)
+{
+	char *end;
+
+	corruption_check_period = simple_strtoul(arg, &end, 10);
+
+	return (*end == 0) ? 0 : -EINVAL;
+}
+early_param("memory_corruption_check_period", set_corruption_check_period);
+
+static int set_corruption_check_size(char *arg)
+{
+	char *end;
+	unsigned size;
+
+	size = memparse(arg, &end);
+
+	if (*end == '\0')
+		corruption_check_size = size;
+
+	return (size == corruption_check_size) ? 0 : -EINVAL;
+}
+early_param("memory_corruption_check_size", set_corruption_check_size);
+
+
+void __init setup_bios_corruption_check(void)
+{
+	u64 addr = PAGE_SIZE;	/* assume first page is reserved anyway */
+
+	if (memory_corruption_check == -1) {
+		memory_corruption_check =
+#ifdef CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK
+			1
+#else
+			0
+#endif
+			;
+	}
+
+	if (corruption_check_size == 0)
+		memory_corruption_check = 0;
+
+	if (!memory_corruption_check)
+		return;
+
+	corruption_check_size = round_up(corruption_check_size, PAGE_SIZE);
+
+	while(addr < corruption_check_size && num_scan_areas < MAX_SCAN_AREAS) {
+		u64 size;
+		addr = find_e820_area_size(addr, &size, PAGE_SIZE);
+
+		if (addr == 0)
+			break;
+
+		if ((addr + size) > corruption_check_size)
+			size = corruption_check_size - addr;
+
+		if (size == 0)
+			break;
+
+		e820_update_range(addr, size, E820_RAM, E820_RESERVED);
+		scan_areas[num_scan_areas].addr = addr;
+		scan_areas[num_scan_areas].size = size;
+		num_scan_areas++;
+
+		/* Assume we've already mapped this early memory */
+		memset(__va(addr), 0, size);
+
+		addr += size;
+	}
+
+	printk(KERN_INFO "Scanning %d areas for low memory corruption\n",
+	       num_scan_areas);
+	update_e820();
+}
+
+static struct timer_list periodic_check_timer;
+
+void check_for_bios_corruption(void)
+{
+	int i;
+	int corruption = 0;
+
+	if (!memory_corruption_check)
+		return;
+
+	for(i = 0; i < num_scan_areas; i++) {
+		unsigned long *addr = __va(scan_areas[i].addr);
+		unsigned long size = scan_areas[i].size;
+
+		for(; size; addr++, size -= sizeof(unsigned long)) {
+			if (!*addr)
+				continue;
+			printk(KERN_ERR "Corrupted low memory at %p (%lx phys) = %08lx\n",
+			       addr, __pa(addr), *addr);
+			corruption = 1;
+			*addr = 0;
+		}
+	}
+
+	WARN(corruption, KERN_ERR "Memory corruption detected in low memory\n");
+}
+
+static void periodic_check_for_corruption(unsigned long data)
+{
+	check_for_bios_corruption();
+	mod_timer(&periodic_check_timer, round_jiffies(jiffies + corruption_check_period*HZ));
+}
+
+void start_periodic_check_for_corruption(void)
+{
+	if (!memory_corruption_check || corruption_check_period == 0)
+		return;
+
+	printk(KERN_INFO "Scanning for low memory corruption every %d seconds\n",
+	       corruption_check_period);
+
+	init_timer(&periodic_check_timer);
+	periodic_check_timer.function = &periodic_check_for_corruption;
+	periodic_check_for_corruption(0);
+}
+#endif
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 3d677b5..a243fe4 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -581,157 +581,6 @@ static struct x86_quirks default_x86_quirks __initdata;
 
 struct x86_quirks *x86_quirks __initdata = &default_x86_quirks;
 
-/*
- * Some BIOSes seem to corrupt the low 64k of memory during events
- * like suspend/resume and unplugging an HDMI cable.  Reserve all
- * remaining free memory in that area and fill it with a distinct
- * pattern.
- */
-#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
-#define MAX_SCAN_AREAS	8
-
-static int __read_mostly memory_corruption_check = -1;
-
-static unsigned __read_mostly corruption_check_size = 64*1024;
-static unsigned __read_mostly corruption_check_period = 60; /* seconds */
-
-static struct e820entry scan_areas[MAX_SCAN_AREAS];
-static int num_scan_areas;
-
-
-static int set_corruption_check(char *arg)
-{
-	char *end;
-
-	memory_corruption_check = simple_strtol(arg, &end, 10);
-
-	return (*end == 0) ? 0 : -EINVAL;
-}
-early_param("memory_corruption_check", set_corruption_check);
-
-static int set_corruption_check_period(char *arg)
-{
-	char *end;
-
-	corruption_check_period = simple_strtoul(arg, &end, 10);
-
-	return (*end == 0) ? 0 : -EINVAL;
-}
-early_param("memory_corruption_check_period", set_corruption_check_period);
-
-static int set_corruption_check_size(char *arg)
-{
-	char *end;
-	unsigned size;
-
-	size = memparse(arg, &end);
-
-	if (*end == '\0')
-		corruption_check_size = size;
-
-	return (size == corruption_check_size) ? 0 : -EINVAL;
-}
-early_param("memory_corruption_check_size", set_corruption_check_size);
-
-
-static void __init setup_bios_corruption_check(void)
-{
-	u64 addr = PAGE_SIZE;	/* assume first page is reserved anyway */
-
-	if (memory_corruption_check == -1) {
-		memory_corruption_check =
-#ifdef CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK
-			1
-#else
-			0
-#endif
-			;
-	}
-
-	if (corruption_check_size == 0)
-		memory_corruption_check = 0;
-
-	if (!memory_corruption_check)
-		return;
-
-	corruption_check_size = round_up(corruption_check_size, PAGE_SIZE);
-
-	while(addr < corruption_check_size && num_scan_areas < MAX_SCAN_AREAS) {
-		u64 size;
-		addr = find_e820_area_size(addr, &size, PAGE_SIZE);
-
-		if (addr == 0)
-			break;
-
-		if ((addr + size) > corruption_check_size)
-			size = corruption_check_size - addr;
-
-		if (size == 0)
-			break;
-
-		e820_update_range(addr, size, E820_RAM, E820_RESERVED);
-		scan_areas[num_scan_areas].addr = addr;
-		scan_areas[num_scan_areas].size = size;
-		num_scan_areas++;
-
-		/* Assume we've already mapped this early memory */
-		memset(__va(addr), 0, size);
-
-		addr += size;
-	}
-
-	printk(KERN_INFO "Scanning %d areas for low memory corruption\n",
-	       num_scan_areas);
-	update_e820();
-}
-
-static struct timer_list periodic_check_timer;
-
-void check_for_bios_corruption(void)
-{
-	int i;
-	int corruption = 0;
-
-	if (!memory_corruption_check)
-		return;
-
-	for(i = 0; i < num_scan_areas; i++) {
-		unsigned long *addr = __va(scan_areas[i].addr);
-		unsigned long size = scan_areas[i].size;
-
-		for(; size; addr++, size -= sizeof(unsigned long)) {
-			if (!*addr)
-				continue;
-			printk(KERN_ERR "Corrupted low memory at %p (%lx phys) = %08lx\n",
-			       addr, __pa(addr), *addr);
-			corruption = 1;
-			*addr = 0;
-		}
-	}
-
-	WARN(corruption, KERN_ERR "Memory corruption detected in low memory\n");
-}
-
-static void periodic_check_for_corruption(unsigned long data)
-{
-	check_for_bios_corruption();
-	mod_timer(&periodic_check_timer, round_jiffies(jiffies + corruption_check_period*HZ));
-}
-
-void start_periodic_check_for_corruption(void)
-{
-	if (!memory_corruption_check || corruption_check_period == 0)
-		return;
-
-	printk(KERN_INFO "Scanning for low memory corruption every %d seconds\n",
-	       corruption_check_period);
-
-	init_timer(&periodic_check_timer);
-	periodic_check_timer.function = &periodic_check_for_corruption;
-	periodic_check_for_corruption(0);
-}
-#endif
-
 static int __init dmi_low_memory_corruption(const struct dmi_system_id *d)
 {
 	printk(KERN_NOTICE
diff --git a/include/asm-x86/setup.h b/include/asm-x86/setup.h
index 11b6cc1..5c55fc1 100644
--- a/include/asm-x86/setup.h
+++ b/include/asm-x86/setup.h
@@ -8,6 +8,10 @@
 /* Interrupt control for vSMPowered x86_64 systems */
 void vsmp_init(void);
 
+
+void setup_bios_corruption_check(void);
+
+
 #ifdef CONFIG_X86_VISWS
 extern void visws_early_detect(void);
 extern int is_visws_box(void);
-- 
1.5.5.1


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

[PATCH 2/3] corruption check: run the corruption checks from a work queue

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Date: Mon, 22 Sep 2008 13:42:15 -0700
Subject: [PATCH] corruption check: run the corruption checks from a work queue

the corruption checks are better off run from a work queue; there's nothing
time critical about them and this way the amount of interrupt-context work
is reduced.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 arch/x86/kernel/check.c |   21 +++++++++++++++++----
 arch/x86/mm/init_32.c   |    2 --
 arch/x86/mm/init_64.c   |    2 --
 include/linux/kernel.h  |    2 +-
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/check.c b/arch/x86/kernel/check.c
index a7e80ed..b22733f 100644
--- a/arch/x86/kernel/check.c
+++ b/arch/x86/kernel/check.c
@@ -1,5 +1,6 @@
 #include <linux/module.h>
 #include <linux/sched.h>
+#include <linux/kthread.h>
 
 #include <asm/e820.h>
 #include <asm/proto.h>
@@ -135,22 +136,34 @@ void check_for_bios_corruption(void)
 	WARN(corruption, KERN_ERR "Memory corruption detected in low memory\n");
 }
 
-static void periodic_check_for_corruption(unsigned long data)
+static void check_corruption(struct work_struct *dummy)
 {
 	check_for_bios_corruption();
+}
+
+static void periodic_check_for_corruption(unsigned long data)
+{
+	static DECLARE_WORK(corruptioncheck_work, check_corruption);
+	schedule_work(&corruptioncheck_work);
 	mod_timer(&periodic_check_timer, round_jiffies(jiffies + corruption_check_period*HZ));
 }
 
-void start_periodic_check_for_corruption(void)
+
+
+int start_periodic_check_for_corruption(void)
 {
 	if (!memory_corruption_check || corruption_check_period == 0)
-		return;
+		return 0;
 
 	printk(KERN_INFO "Scanning for low memory corruption every %d seconds\n",
 	       corruption_check_period);
 
 	init_timer(&periodic_check_timer);
 	periodic_check_timer.function = &periodic_check_for_corruption;
-	periodic_check_for_corruption(0);
+	mod_timer(&periodic_check_timer,
+			round_jiffies(jiffies + corruption_check_period*HZ));
+	return 0;
 }
+
+module_init(start_periodic_check_for_corruption);
 #endif
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index b4c3a92..16ea9d0 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -905,8 +905,6 @@ void __init mem_init(void)
 	int codesize, reservedpages, datasize, initsize;
 	int tmp;
 
-	start_periodic_check_for_corruption();
-
 #ifdef CONFIG_FLATMEM
 	BUG_ON(!mem_map);
 #endif
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index b565721..b65492e 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -822,8 +822,6 @@ void __init mem_init(void)
 {
 	long codesize, reservedpages, datasize, initsize;
 
-	start_periodic_check_for_corruption();
-
 	pci_iommu_alloc();
 
 	/* clear_bss() already clear the empty_zero_page */
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index a5afee2..01085d2 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -260,7 +260,7 @@ extern int root_mountflags;
  * able to scatter it around anywhere in the kernel.
  */
 void check_for_bios_corruption(void);
-void start_periodic_check_for_corruption(void);
+int start_periodic_check_for_corruption(void);
 #else
 static inline void check_for_bios_corruption(void)
 {
-- 
1.5.5.1




-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

[PATCH 3/3] corruption check: fix various checkpatch warnings

[Arjan van de Ven]

>From 9e49c38637f335d5c91a0f1c4deea7ce2c20aa34 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Tue, 23 Sep 2008 11:09:10 -0700
Subject: [PATCH] corruption check: fix various checkpatch warnings

Cleanups as suggested by Randy / checkpatch.pl

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 arch/x86/kernel/check.c |    8 ++++----
 include/linux/kernel.h  |    5 -----
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/check.c b/arch/x86/kernel/check.c
index b22733f..c59bfe6 100644
--- a/arch/x86/kernel/check.c
+++ b/arch/x86/kernel/check.c
@@ -80,7 +80,7 @@ void __init setup_bios_corruption_check(void)
 
 	corruption_check_size = round_up(corruption_check_size, PAGE_SIZE);
 
-	while(addr < corruption_check_size && num_scan_areas < MAX_SCAN_AREAS) {
+	while (addr < corruption_check_size && num_scan_areas < MAX_SCAN_AREAS) {
 		u64 size;
 		addr = find_e820_area_size(addr, &size, PAGE_SIZE);
 
@@ -119,11 +119,11 @@ void check_for_bios_corruption(void)
 	if (!memory_corruption_check)
 		return;
 
-	for(i = 0; i < num_scan_areas; i++) {
+	for (i = 0; i < num_scan_areas; i++) {
 		unsigned long *addr = __va(scan_areas[i].addr);
 		unsigned long size = scan_areas[i].size;
 
-		for(; size; addr++, size -= sizeof(unsigned long)) {
+		for (; size; addr++, size -= sizeof(unsigned long)) {
 			if (!*addr)
 				continue;
 			printk(KERN_ERR "Corrupted low memory at %p (%lx phys) = %08lx\n",
@@ -150,7 +150,7 @@ static void periodic_check_for_corruption(unsigned long data)
 
 
 
-int start_periodic_check_for_corruption(void)
+static int start_periodic_check_for_corruption(void)
 {
 	if (!memory_corruption_check || corruption_check_period == 0)
 		return 0;
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 01085d2..b39c7ef 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -260,15 +260,10 @@ extern int root_mountflags;
  * able to scatter it around anywhere in the kernel.
  */
 void check_for_bios_corruption(void);
-int start_periodic_check_for_corruption(void);
 #else
 static inline void check_for_bios_corruption(void)
 {
 }
-
-static inline void start_periodic_check_for_corruption(void)
-{
-}
 #endif
 
 /* Values used for system_state */
-- 
1.5.5.1




-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH 1/3] corruption check: move the corruption checks into their own file

[Andrew Morton]

On Tue, 23 Sep 2008 21:53:35 -0700 Arjan van de Ven <arjan@infradead.org> wrote:

> 
> >From 19058f850aaf82d8185f06b54582a3d8e1ca8d97 Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Mon, 22 Sep 2008 09:35:06 -0700
> Subject: [PATCH] corruption check: move the corruption checks into their own file
> 
> There'll be more checks over time (based on kerneloops data I have
> several ideas already) and making the checks their own file rather
> than setup.c makes sense in that light.
> 
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> ---
>  arch/x86/kernel/Makefile |    1 +
>  arch/x86/kernel/check.c  |  156 ++++++++++++++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/setup.c  |  151 --------------------------------------------

So this is long-standing code.  Let's pretend otherwise ;)

>  include/asm-x86/setup.h  |    4 +
>  4 files changed, 161 insertions(+), 151 deletions(-)
>  create mode 100644 arch/x86/kernel/check.c
> 
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index c71722d..5d788ea 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -36,6 +36,7 @@ obj-y			+= bootflag.o e820.o
>  obj-y			+= pci-dma.o quirks.o i8237.o topology.o kdebugfs.o
>  obj-y			+= alternative.o i8253.o pci-nommu.o
>  obj-y			+= tsc.o io_delay.o rtc.o
> +obj-y			+= check.o
>  
>  obj-$(CONFIG_X86_TRAMPOLINE)	+= trampoline.o
>  obj-y				+= process.o
> diff --git a/arch/x86/kernel/check.c b/arch/x86/kernel/check.c
> new file mode 100644
> index 0000000..a7e80ed
> --- /dev/null
> +++ b/arch/x86/kernel/check.c
> @@ -0,0 +1,156 @@
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +
> +#include <asm/e820.h>
> +#include <asm/proto.h>
> +
> +/*
> + * Some BIOSes seem to corrupt the low 64k of memory during events
> + * like suspend/resume and unplugging an HDMI cable.  Reserve all
> + * remaining free memory in that area and fill it with a distinct
> + * pattern.
> + */
> +#ifdef CONFIG_X86_CHECK_BIOS_CORRUPTION
> +#define MAX_SCAN_AREAS	8
> +
> +static int __read_mostly memory_corruption_check = -1;
> +
> +static unsigned __read_mostly corruption_check_size = 64*1024;
> +static unsigned __read_mostly corruption_check_period = 60; /* seconds */
> +
> +static struct e820entry scan_areas[MAX_SCAN_AREAS];
> +static int num_scan_areas;
> +
> +
> +static int set_corruption_check(char *arg)
> +{
> +	char *end;
> +
> +	memory_corruption_check = simple_strtol(arg, &end, 10);
> +
> +	return (*end == 0) ? 0 : -EINVAL;
> +}
> +early_param("memory_corruption_check", set_corruption_check);

__init?

> +static int set_corruption_check_period(char *arg)
> +{
> +	char *end;
> +
> +	corruption_check_period = simple_strtoul(arg, &end, 10);
> +
> +	return (*end == 0) ? 0 : -EINVAL;
> +}
> +early_param("memory_corruption_check_period", set_corruption_check_period);

__init?

> +static int set_corruption_check_size(char *arg)
> +{
> +	char *end;
> +	unsigned size;
> +
> +	size = memparse(arg, &end);
> +
> +	if (*end == '\0')
> +		corruption_check_size = size;
> +
> +	return (size == corruption_check_size) ? 0 : -EINVAL;
> +}
> +early_param("memory_corruption_check_size", set_corruption_check_size);

__init?

> +
> +void __init setup_bios_corruption_check(void)
> +{
> +	u64 addr = PAGE_SIZE;	/* assume first page is reserved anyway */
> +
> +	if (memory_corruption_check == -1) {
> +		memory_corruption_check =
> +#ifdef CONFIG_X86_BOOTPARAM_MEMORY_CORRUPTION_CHECK
> +			1
> +#else
> +			0
> +#endif
> +			;
> +	}
> +
> +	if (corruption_check_size == 0)
> +		memory_corruption_check = 0;
> +
> +	if (!memory_corruption_check)
> +		return;
> +
> +	corruption_check_size = round_up(corruption_check_size, PAGE_SIZE);
> +
> +	while(addr < corruption_check_size && num_scan_areas < MAX_SCAN_AREAS) {
> +		u64 size;
> +		addr = find_e820_area_size(addr, &size, PAGE_SIZE);
> +
> +		if (addr == 0)
> +			break;
> +
> +		if ((addr + size) > corruption_check_size)
> +			size = corruption_check_size - addr;
> +
> +		if (size == 0)
> +			break;
> +
> +		e820_update_range(addr, size, E820_RAM, E820_RESERVED);
> +		scan_areas[num_scan_areas].addr = addr;
> +		scan_areas[num_scan_areas].size = size;
> +		num_scan_areas++;
> +
> +		/* Assume we've already mapped this early memory */
> +		memset(__va(addr), 0, size);
> +
> +		addr += size;
> +	}
> +
> +	printk(KERN_INFO "Scanning %d areas for low memory corruption\n",
> +	       num_scan_areas);
> +	update_e820();
> +}
> +
> +static struct timer_list periodic_check_timer;

Could use DEFINE_TIMER() here.

> +void check_for_bios_corruption(void)
> +{
> +	int i;
> +	int corruption = 0;
> +
> +	if (!memory_corruption_check)
> +		return;
> +
> +	for(i = 0; i < num_scan_areas; i++) {
> +		unsigned long *addr = __va(scan_areas[i].addr);
> +		unsigned long size = scan_areas[i].size;
> +
> +		for(; size; addr++, size -= sizeof(unsigned long)) {
> +			if (!*addr)
> +				continue;
> +			printk(KERN_ERR "Corrupted low memory at %p (%lx phys) = %08lx\n",
> +			       addr, __pa(addr), *addr);
> +			corruption = 1;
> +			*addr = 0;
> +		}
> +	}
> +
> +	WARN(corruption, KERN_ERR "Memory corruption detected in low memory\n");
> +}
> +
> +static void periodic_check_for_corruption(unsigned long data)
> +{
> +	check_for_bios_corruption();
> +	mod_timer(&periodic_check_timer, round_jiffies(jiffies + corruption_check_period*HZ));
> +}
> +
> +void start_periodic_check_for_corruption(void)
> +{
> +	if (!memory_corruption_check || corruption_check_period == 0)
> +		return;
> +
> +	printk(KERN_INFO "Scanning for low memory corruption every %d seconds\n",
> +	       corruption_check_period);
> +
> +	init_timer(&periodic_check_timer);
> +	periodic_check_timer.function = &periodic_check_for_corruption;

and zap those.

> +	periodic_check_for_corruption(0);
> +}
> +#endif

Re: [PATCH 2/3] corruption check: run the corruption checks from a work queue

[Andrew Morton]

On Tue, 23 Sep 2008 21:54:13 -0700 Arjan van de Ven <arjan@infradead.org> wrote:

> 
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Mon, 22 Sep 2008 13:42:15 -0700
> Subject: [PATCH] corruption check: run the corruption checks from a work queue
> 
> the corruption checks are better off run from a work queue; there's nothing
> time critical about them and this way the amount of interrupt-context work
> is reduced.
> 
> ...
>
> +int start_periodic_check_for_corruption(void)
>  {
>  	if (!memory_corruption_check || corruption_check_period == 0)
> -		return;
> +		return 0;
>  
>  	printk(KERN_INFO "Scanning for low memory corruption every %d seconds\n",
>  	       corruption_check_period);
>  
>  	init_timer(&periodic_check_timer);
>  	periodic_check_timer.function = &periodic_check_for_corruption;
> -	periodic_check_for_corruption(0);
> +	mod_timer(&periodic_check_timer,
> +			round_jiffies(jiffies + corruption_check_period*HZ));
> +	return 0;
>  }

Could use schedule_delayed_work() and zap the timer altogether?

Re: [PATCH 1/3] corruption check: move the corruption checks into their own file

[Arjan van de Ven]

On Tue, 23 Sep 2008 22:53:06 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:


> __init?

I got flamed already for changing code while it's just being moved ....

I'll look at your cleanup suggestions as another separate patch
please ;(

Re: [PATCH 1/3] corruption check: move the corruption checks into their own file

[Randy Dunlap]

On Wed, 24 Sep 2008 10:16:08 -0700 Arjan van de Ven wrote:

> On Tue, 23 Sep 2008 22:53:06 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> 
> > __init?
> 
> I got flamed already for changing code while it's just being moved ....

You mean what Ingo said?  I don't see any flames in what he said.
Maybe you got flames by private email??

> I'll look at your cleanup suggestions as another separate patch
> please ;(

whatever.

---
~Randy

Re: [PATCH 1/3] corruption check: move the corruption checks into their own file

[Ingo Molnar]

* Arjan van de Ven <arjan@infradead.org> wrote:

> I'll look at your cleanup suggestions as another separate patch please 

yes, that's natural. Anything that isnt super-important to backmerge 
should go into a separate patch.

	Ingo