[PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Yinghai Lu]

try to insert_resource second time, by expand the resource...

for case: e820 reserved entry is partially overlapped with bar res...

hope it will never happen

v2: according to Linus, add insert_resource_expand_to_fit, and change
	__insert_resource to static without lock

v3: use reserve_region_with_split() instead to hand overlapping
	with test case by extend 0xe0000000 - 0xeffffff to 0xdd800000 -
	get
		e0000000-efffffff : PCI MMCONFIG 0
			 e0000000-efffffff : reserved
	in /proc/iomem
	get
		found conflict for reserved [dd800000, efffffff], try to reserve with split
		    __reserve_region_with_split: (PCI Bus #80) [dd000000, ddffffff], res: (reserved) [dd800000, efffffff]
		    __reserve_region_with_split: (PCI Bus #00) [de000000, dfffffff], res: (reserved) [de000000, efffffff]
		initcall pci_subsys_init+0x0/0x121 returned 0 after 381 msecs
	in dmesg

v4: take out __insert_resource and insert_resource_expand_to_fit : Linus already check in.
    use reserve_region_with_split at the first point
    use const char *name

v5: fix checking on overlapping

v6: only reserve common area in conflict
	so got sth in /proc/iomem
		d8000000-dfffffff : PCI Bus #00
		  dc000000-dfffffff : GART
		    dc000000-dfffffff : reserved
		e0000000-efffffff : PCI MMCONFIG 0
		  e0000000-efffffff : reserved
	when we have big range in e820
		00000000dc000000-00000000f0000000 (reserved)

v7: remove wrong removing of write_unlock(&resource_lock) in adjust_resource()

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

---
 arch/x86/kernel/e820.c |    2 -
 include/linux/ioport.h |    3 ++
 kernel/resource.c      |   68 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/x86/kernel/e820.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/e820.c
+++ linux-2.6/arch/x86/kernel/e820.c
@@ -1320,7 +1320,7 @@ void __init e820_reserve_resources_late(
 	res = e820_res;
 	for (i = 0; i < e820.nr_map; i++) {
 		if (!res->parent && res->end)
-			insert_resource(&iomem_resource, res);
+			reserve_region_with_split(&iomem_resource, res->start, res->end, res->name);
 		res++;
 	}
 }
Index: linux-2.6/include/linux/ioport.h
===================================================================
--- linux-2.6.orig/include/linux/ioport.h
+++ linux-2.6/include/linux/ioport.h
@@ -108,6 +108,9 @@ extern struct resource iomem_resource;
 
 extern int request_resource(struct resource *root, struct resource *new);
 extern int release_resource(struct resource *new);
+extern void reserve_region_with_split(struct resource *root,
+			     resource_size_t start, resource_size_t end,
+			     const char *name);
 extern int insert_resource(struct resource *parent, struct resource *new);
 extern void insert_resource_expand_to_fit(struct resource *root, struct resource *new);
 extern int allocate_resource(struct resource *root, struct resource *new,
Index: linux-2.6/kernel/resource.c
===================================================================
--- linux-2.6.orig/kernel/resource.c
+++ linux-2.6/kernel/resource.c
@@ -516,6 +516,74 @@ int adjust_resource(struct resource *res
 	return result;
 }
 
+static void __init __reserve_region_with_split(struct resource *root,
+		resource_size_t start, resource_size_t end,
+		const char *name)
+{
+	struct resource *parent = root;
+	struct resource *conflict;
+	struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);
+
+	if (!res)
+		return;
+
+	res->name = name;
+	res->start = start;
+	res->end = end;
+	res->flags = IORESOURCE_BUSY;
+
+	for (;;) {
+		conflict = __request_resource(parent, res);
+		if (!conflict)
+			break;
+		if (conflict != parent) {
+			parent = conflict;
+			if (!(conflict->flags & IORESOURCE_BUSY))
+				continue;
+		}
+
+		/* Uhhuh, that didn't work out.. */
+		kfree(res);
+		res = NULL;
+		break;
+	}
+
+	if (!res) {
+		printk(KERN_DEBUG "    __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
+			 conflict->name, conflict->start, conflict->end,
+			 name, start, end);
+
+		/* failed, split and try again */
+
+		/* conflict coverred whole area */
+		if (conflict->start <= start && conflict->end >= end)
+			return;
+
+		if (conflict->start > start)
+			__reserve_region_with_split(root, start, conflict->start-1, name);
+		if (!(conflict->flags & IORESOURCE_BUSY)) {
+			resource_size_t common_start, common_end;
+
+			common_start = max(conflict->start, start);
+			common_end = min(conflict->end, end);
+			if (common_start < common_end)
+				__reserve_region_with_split(root, common_start, common_end, name);
+		}
+		if (conflict->end < end)
+			__reserve_region_with_split(root, conflict->end+1, end, name);
+	}
+
+}
+
+void reserve_region_with_split(struct resource *root,
+		resource_size_t start, resource_size_t end,
+		const char *name)
+{
+	write_lock(&resource_lock);
+	__reserve_region_with_split(root, start, end, name);
+	write_unlock(&resource_lock);
+}
+
 EXPORT_SYMBOL(adjust_resource);
 
 /**

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Ingo Molnar]

* Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> v3: use reserve_region_with_split() instead to hand overlapping
> 	with test case by extend 0xe0000000 - 0xeffffff to 0xdd800000 -
> 	get
> 		e0000000-efffffff : PCI MMCONFIG 0
> 			 e0000000-efffffff : reserved
> 	in /proc/iomem
> 	get
> 		found conflict for reserved [dd800000, efffffff], try to reserve with split
> 		    __reserve_region_with_split: (PCI Bus #80) [dd000000, ddffffff], res: (reserved) [dd800000, efffffff]
> 		    __reserve_region_with_split: (PCI Bus #00) [de000000, dfffffff], res: (reserved) [de000000, efffffff]
> 		initcall pci_subsys_init+0x0/0x121 returned 0 after 381 msecs
> 	in dmesg
> 
> v4: take out __insert_resource and insert_resource_expand_to_fit : Linus already check in.
>     use reserve_region_with_split at the first point
>     use const char *name
> 
> v5: fix checking on overlapping
> 
> v6: only reserve common area in conflict
> 	so got sth in /proc/iomem
> 		d8000000-dfffffff : PCI Bus #00
> 		  dc000000-dfffffff : GART
> 		    dc000000-dfffffff : reserved
> 		e0000000-efffffff : PCI MMCONFIG 0
> 		  e0000000-efffffff : reserved
> 	when we have big range in e820
> 		00000000dc000000-00000000f0000000 (reserved)
> 
> v7: remove wrong removing of write_unlock(&resource_lock) in adjust_resource()

i've applied the generic commit below to tip/core/resources, and merged 
tip/core/resources into tip/x86/core, and added the remaining e820.c 
oneliner change (attached below as well).

thanks Yinghai!

	Ingo

-------------------->
>From 268364a0f48aee2f851f9d1ef8a6cda0f3039ef1 Mon Sep 17 00:00:00 2001
From: Yinghai Lu <yhlu.kernel@gmail.com>
Date: Thu, 4 Sep 2008 21:02:44 +0200
Subject: [PATCH] IO resources: add reserve_region_with_split()

add reserve_region_with_split() to not lose e820 reserved entries if
they overlap with existing IO regions:

with test case by extend 0xe0000000 - 0xeffffff to 0xdd800000 -
we get:
	e0000000-efffffff : PCI MMCONFIG 0
		 e0000000-efffffff : reserved

and in /proc/iomem we get:
	found conflict for reserved [dd800000, efffffff], try to reserve with split
	    __reserve_region_with_split: (PCI Bus #80) [dd000000, ddffffff], res: (reserved) [dd800000, efffffff]
	    __reserve_region_with_split: (PCI Bus #00) [de000000, dfffffff], res: (reserved) [de000000, efffffff]
	initcall pci_subsys_init+0x0/0x121 returned 0 after 381 msecs
in dmesg

various fixes and improvements suggested by Linus.

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/linux/ioport.h |    3 ++
 kernel/resource.c      |   68 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 0 deletions(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 8d3b7a9..fded376 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -108,6 +108,9 @@ extern struct resource iomem_resource;
 
 extern int request_resource(struct resource *root, struct resource *new);
 extern int release_resource(struct resource *new);
+extern void reserve_region_with_split(struct resource *root,
+			     resource_size_t start, resource_size_t end,
+			     const char *name);
 extern int insert_resource(struct resource *parent, struct resource *new);
 extern void insert_resource_expand_to_fit(struct resource *root, struct resource *new);
 extern int allocate_resource(struct resource *root, struct resource *new,
diff --git a/kernel/resource.c b/kernel/resource.c
index 03d796c..414d6fc 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -516,6 +516,74 @@ int adjust_resource(struct resource *res, resource_size_t start, resource_size_t
 	return result;
 }
 
+static void __init __reserve_region_with_split(struct resource *root,
+		resource_size_t start, resource_size_t end,
+		const char *name)
+{
+	struct resource *parent = root;
+	struct resource *conflict;
+	struct resource *res = kzalloc(sizeof(*res), GFP_KERNEL);
+
+	if (!res)
+		return;
+
+	res->name = name;
+	res->start = start;
+	res->end = end;
+	res->flags = IORESOURCE_BUSY;
+
+	for (;;) {
+		conflict = __request_resource(parent, res);
+		if (!conflict)
+			break;
+		if (conflict != parent) {
+			parent = conflict;
+			if (!(conflict->flags & IORESOURCE_BUSY))
+				continue;
+		}
+
+		/* Uhhuh, that didn't work out.. */
+		kfree(res);
+		res = NULL;
+		break;
+	}
+
+	if (!res) {
+		printk(KERN_DEBUG "    __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
+			 conflict->name, conflict->start, conflict->end,
+			 name, start, end);
+
+		/* failed, split and try again */
+
+		/* conflict coverred whole area */
+		if (conflict->start <= start && conflict->end >= end)
+			return;
+
+		if (conflict->start > start)
+			__reserve_region_with_split(root, start, conflict->start-1, name);
+		if (!(conflict->flags & IORESOURCE_BUSY)) {
+			resource_size_t common_start, common_end;
+
+			common_start = max(conflict->start, start);
+			common_end = min(conflict->end, end);
+			if (common_start < common_end)
+				__reserve_region_with_split(root, common_start, common_end, name);
+		}
+		if (conflict->end < end)
+			__reserve_region_with_split(root, conflict->end+1, end, name);
+	}
+
+}
+
+void reserve_region_with_split(struct resource *root,
+		resource_size_t start, resource_size_t end,
+		const char *name)
+{
+	write_lock(&resource_lock);
+	__reserve_region_with_split(root, start, end, name);
+	write_unlock(&resource_lock);
+}
+
 EXPORT_SYMBOL(adjust_resource);
 
 /**

>From fac8f1e4f99dff7a0c3a929f327d66f46de6fa21 Mon Sep 17 00:00:00 2001
From: Yinghai Lu <yhlu.kernel@gmail.com>
Date: Thu, 4 Sep 2008 20:59:22 +0200
Subject: [PATCH] x86: split e820 reserved entries record to late, v7

try to insert_resource second time, by expanding the resource...

for case: e820 reserved entry is partially overlapped with bar res...

hope it will never happen

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/e820.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index a7a7133..e24d1bc 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -1320,7 +1320,7 @@ void __init e820_reserve_resources_late(void)
 	res = e820_res;
 	for (i = 0; i < e820.nr_map; i++) {
 		if (!res->parent && res->end)
-			insert_resource(&iomem_resource, res);
+			reserve_region_with_split(&iomem_resource, res->start, res->end, res->name);
 		res++;
 	}
 }

>From ebd60cd64f8ab1170102c3ab072eb73042b7a33d Mon Sep 17 00:00:00 2001
From: Yinghai Lu <yhlu.kernel@gmail.com>
Date: Thu, 4 Sep 2008 21:04:32 +0200
Subject: [PATCH] x86: unify using pci_mmcfg_insert_resource

even with known_bridge insert them late too.

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/pci/mmconfig-shared.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/x86/pci/mmconfig-shared.c b/arch/x86/pci/mmconfig-shared.c
index d963576..654a223 100644
--- a/arch/x86/pci/mmconfig-shared.c
+++ b/arch/x86/pci/mmconfig-shared.c
@@ -209,7 +209,7 @@ static int __init pci_mmcfg_check_hostbridge(void)
 	return name != NULL;
 }
 
-static void __init pci_mmcfg_insert_resources(unsigned long resource_flags)
+static void __init pci_mmcfg_insert_resources(void)
 {
 #define PCI_MMCFG_RESOURCE_NAME_LEN 19
 	int i;
@@ -233,7 +233,7 @@ static void __init pci_mmcfg_insert_resources(unsigned long resource_flags)
 			 cfg->pci_segment);
 		res->start = cfg->address;
 		res->end = res->start + (num_buses << 20) - 1;
-		res->flags = IORESOURCE_MEM | resource_flags;
+		res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
 		insert_resource(&iomem_resource, res);
 		names += PCI_MMCFG_RESOURCE_NAME_LEN;
 	}
@@ -434,11 +434,9 @@ static void __init __pci_mmcfg_init(int early)
 	    (pci_mmcfg_config[0].address == 0))
 		return;
 
-	if (pci_mmcfg_arch_init()) {
-		if (known_bridge)
-			pci_mmcfg_insert_resources(IORESOURCE_BUSY);
+	if (pci_mmcfg_arch_init())
 		pci_probe = (pci_probe & ~PCI_PROBE_MASK) | PCI_PROBE_MMCONF;
-	} else {
+	else {
 		/*
 		 * Signal not to attempt to insert mmcfg resources because
 		 * the architecture mmcfg setup could not initialize.
@@ -475,7 +473,7 @@ static int __init pci_mmcfg_late_insert_resources(void)
 	 * marked so it won't cause request errors when __request_region is
 	 * called.
 	 */
-	pci_mmcfg_insert_resources(0);
+	pci_mmcfg_insert_resources();
 
 	return 0;
 }

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Andrew Morton]

On Thu, 4 Sep 2008 21:04:57 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> +		printk(KERN_DEBUG "    __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
> +			 conflict->name, conflict->start, conflict->end,
> +			 name, start, end);

start and end have type resource_size_t.  Such types CANNOT be printed
unless cast to a known type.

Because there is a %s following an incorrect %lld, the above code will
crash the machine.

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Yinghai Lu]

On Thu, Sep 4, 2008 at 12:16 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Thu, 4 Sep 2008 21:04:57 +0200
> Ingo Molnar <mingo@elte.hu> wrote:
>
>> +             printk(KERN_DEBUG "    __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
>> +                      conflict->name, conflict->start, conflict->end,
>> +                      name, start, end);
>
> start and end have type resource_size_t.  Such types CANNOT be printed
> unless cast to a known type.
>
> Because there is a %s following an incorrect %lld, the above code will
> crash the machine.

should just remove those lines.

YH

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Tony Luck]

On Thu, Sep 4, 2008 at 12:22 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> On Thu, Sep 4, 2008 at 12:16 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
>> On Thu, 4 Sep 2008 21:04:57 +0200
>> Ingo Molnar <mingo@elte.hu> wrote:
>>
>>> +             printk(KERN_DEBUG "    __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
>>> +                      conflict->name, conflict->start, conflict->end,
>>> +                      name, start, end);
>>
>> start and end have type resource_size_t.  Such types CANNOT be printed
>> unless cast to a known type.
>>
>> Because there is a %s following an incorrect %lld, the above code will
>> crash the machine.
>
> should just remove those lines.

This didn't happen.  These lines are still in the version that went into
Linus' tree over the weekend for 2.6.28.  On the ia64 build they spit
out a bunch of warnings:

kernel/resource.c:554: warning: format '%llx' expects type 'long long
unsigned int', but argument 3 has type 'resource_size_t'

Ditto for args 4, 6 and 7 on the same line.

-Tony

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

Tony Luck wrote:
> On Thu, Sep 4, 2008 at 12:22 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>> On Thu, Sep 4, 2008 at 12:16 PM, Andrew Morton
>> <akpm@linux-foundation.org> wrote:
>>> On Thu, 4 Sep 2008 21:04:57 +0200
>>> Ingo Molnar <mingo@elte.hu> wrote:
>>>
>>>> +             printk(KERN_DEBUG "    __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
>>>> +                      conflict->name, conflict->start, conflict->end,
>>>> +                      name, start, end);
>>> start and end have type resource_size_t.  Such types CANNOT be printed
>>> unless cast to a known type.
>>>
>>> Because there is a %s following an incorrect %lld, the above code will
>>> crash the machine.
>> should just remove those lines.
> 
> This didn't happen.  These lines are still in the version that went into
> Linus' tree over the weekend for 2.6.28.  On the ia64 build they spit
> out a bunch of warnings:
> 
> kernel/resource.c:554: warning: format '%llx' expects type 'long long
> unsigned int', but argument 3 has type 'resource_size_t'
> 
> Ditto for args 4, 6 and 7 on the same line.
> 

Here is a fix... currently running standard tests on it.

	-hpa


[-- Attachment #2: 0001-resource-fix-printk-of-resource_size_t.patch --]
[-- Type: text/x-patch, Size: 1317 bytes --]

>From 2d7c0377c3c98d2605704e67e64d1d19aa30ced3 Mon Sep 17 00:00:00 2001
From: H. Peter Anvin <hpa@zytor.com>
Date: Mon, 13 Oct 2008 14:11:03 -0700
Subject: [PATCH] resource: fix printk() of resource_size_t

Impact: crash on 32-bit platforms, warnings on 64-bit platforms

When printk'ing resource_size_t, we have to explicitly cast them to
unsigned long long in order to be safe on all platforms.  We must not use
%z since that applies to size_t, which is frequently not the same type
as resource_size_t.

Signed-off-by: H. Peter Anvin <hpa@zytor.com>
---
 kernel/resource.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 414d6fc..f747fb6 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -550,8 +550,12 @@ static void __init __reserve_region_with_split(struct resource *root,
 
 	if (!res) {
 		printk(KERN_DEBUG "    __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
-			 conflict->name, conflict->start, conflict->end,
-			 name, start, end);
+		       conflict->name,
+		       (unsigned long long)conflict->start,
+		       (unsigned long long)conflict->end,
+		       name,
+		       (unsigned long long)start,
+		       (unsigned long long)end);
 
 		/* failed, split and try again */
 
-- 
1.6.0.2

RE: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Luck, Tony]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 282 bytes --]

> Here is a fix... currently running standard tests on it.

That shuts up the compiler for the ia64 build.

Thanks

-Tony
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Linus Torvalds]

On Mon, 13 Oct 2008, H. Peter Anvin wrote:
> 
> Here is a fix... currently running standard tests on it.

Or we could do what Andrew suggested some time ago, and extend %p to do 
resource printing, like %pS and %pF.

TOTALLY UNTESTED! But something like this might allow

	printk(KERN_DEBUG "  reserve_region: (%s) %pR\n"
		res->name, res);

and if I did things right it should print

	reserve_region: (name) [xx-xx]

and maybe it's worth it. We certainly do seem to have a fair number of 
those irritating casts for resource printouts.

		Linus
---
 lib/vsprintf.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index c399bc1..dd62557 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -24,6 +24,7 @@
 #include <linux/kernel.h>
 #include <linux/kallsyms.h>
 #include <linux/uaccess.h>
+#include <linux/ioport.h>
 
 #include <asm/page.h>		/* for PAGE_SIZE */
 #include <asm/div64.h>
@@ -528,6 +529,21 @@ static char *symbol_string(char *buf, char *end, void *ptr, int field_width, int
 #endif
 }
 
+static char *resource_string(char *buf, char *end, struct resource *res, int field_width, int precision, int flags)
+{
+	char sym[4*sizeof(resource_size_t) + 4];
+	char *p = sym, *pend = sym + sizeof(sym);
+
+	*p++ = '[';
+	p = number(p, pend, res->start, 16, -1, -1, 0);
+	*p++ = '-';
+	p = number(p, pend, res->end, 16, -1, -1, 0);
+	*p++ = ']';
+	*p = 0;
+
+	return string(buf, end, sym, field_width, precision, flags);
+}
+
 /*
  * Show a '%p' thing.  A kernel extension is that the '%p' is followed
  * by an extra set of alphanumeric characters that are extended format
@@ -549,6 +565,8 @@ static char *pointer(const char *fmt, char *buf, char *end, void *ptr, int field
 		/* Fallthrough */
 	case 'S':
 		return symbol_string(buf, end, ptr, field_width, precision, flags);
+	case 'R':
+		return resource_string(buf, end, ptr, field_width, precision, flags);
 	}
 	flags |= SMALL;
 	if (field_width == -1) {

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[H. Peter Anvin]

Linus Torvalds wrote:
> 
> Or we could do what Andrew suggested some time ago, and extend %p to do 
> resource printing, like %pS and %pF.
> 
> TOTALLY UNTESTED! But something like this might allow
> 
> 	printk(KERN_DEBUG "  reserve_region: (%s) %pR\n"
> 		res->name, res);
> 
> and if I did things right it should print
> 
> 	reserve_region: (name) [xx-xx]
> 
> and maybe it's worth it. We certainly do seem to have a fair number of 
> those irritating casts for resource printouts.
> 

There is certainly a lot to be said for this.  Using higher-level 
interfaces keeps formatting consistent and gives us one place to change 
things if desired.

	-hpa

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Yinghai Lu]

On Mon, Oct 13, 2008 at 2:46 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
>
> On Mon, 13 Oct 2008, H. Peter Anvin wrote:
>>
>> Here is a fix... currently running standard tests on it.
>
> Or we could do what Andrew suggested some time ago, and extend %p to do
> resource printing, like %pS and %pF.
>
> TOTALLY UNTESTED! But something like this might allow
>
>        printk(KERN_DEBUG "  reserve_region: (%s) %pR\n"
>                res->name, res);
>
> and if I did things right it should print
>
>        reserve_region: (name) [xx-xx]
>
> and maybe it's worth it. We certainly do seem to have a fair number of
> those irritating casts for resource printouts.
>
>                Linus
> ---
>  lib/vsprintf.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index c399bc1..dd62557 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -24,6 +24,7 @@
>  #include <linux/kernel.h>
>  #include <linux/kallsyms.h>
>  #include <linux/uaccess.h>
> +#include <linux/ioport.h>
>
>  #include <asm/page.h>          /* for PAGE_SIZE */
>  #include <asm/div64.h>
> @@ -528,6 +529,21 @@ static char *symbol_string(char *buf, char *end, void *ptr, int field_width, int
>  #endif
>  }
>
> +static char *resource_string(char *buf, char *end, struct resource *res, int field_width, int precision, int flags)
> +{
> +       char sym[4*sizeof(resource_size_t) + 4];
> +       char *p = sym, *pend = sym + sizeof(sym);
> +
> +       *p++ = '[';
> +       p = number(p, pend, res->start, 16, -1, -1, 0);
> +       *p++ = '-';
> +       p = number(p, pend, res->end, 16, -1, -1, 0);
> +       *p++ = ']';
> +       *p = 0;
> +
> +       return string(buf, end, sym, field_width, precision, flags);

can we have
       printk(KERN_DEBUG "  reserve_region: (%s) %04pR\n"
               res->name, res);
or
       printk(KERN_DEBUG "  reserve_region: (%s) %08pR\n"
               res->name, res);

because io port resource will only 4 digi only.

YH

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> can we have
>        printk(KERN_DEBUG "  reserve_region: (%s) %04pR\n"
>                res->name, res);
> or
>        printk(KERN_DEBUG "  reserve_region: (%s) %08pR\n"
>                res->name, res);
> 
> because io port resource will only 4 digi only.
> 

That should be picked up from res->flags instead of being hardcoded in 
the format string.

	-hpa

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Yinghai Lu]

On Mon, Oct 13, 2008 at 1:32 PM, Tony Luck <tony.luck@intel.com> wrote:
> On Thu, Sep 4, 2008 at 12:22 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>> On Thu, Sep 4, 2008 at 12:16 PM, Andrew Morton
>> <akpm@linux-foundation.org> wrote:
>>> On Thu, 4 Sep 2008 21:04:57 +0200
>>> Ingo Molnar <mingo@elte.hu> wrote:
>>>
>>>> +             printk(KERN_DEBUG "    __reserve_region_with_split: (%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
>>>> +                      conflict->name, conflict->start, conflict->end,
>>>> +                      name, start, end);
>>>

Ingo, how come following patch getting lost?

commit 1cf44baad76b6f20f95ece397c6f643320aa44c9
Author: Ingo Molnar <mingo@elte.hu>
Date:   Thu Sep 4 21:26:06 2008 +0200

    IO resources: fix/remove printk

    Andrew Morton noticed that the printk in kernel/resource.c was buggy:

    | start and end have type resource_size_t.  Such types CANNOT be printed
    | unless cast to a known type.
    |
    | Because there is a %s following an incorrect %lld, the above code will
    | crash the machine.

    ... and it's probably quite unneeded as well, so remove it.

    Reported-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/kernel/resource.c b/kernel/resource.c
index 414d6fc..fc59dcc 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -549,13 +549,9 @@ static void __init
__reserve_region_with_split(struct resource *root,
        }

        if (!res) {
-               printk(KERN_DEBUG "    __reserve_region_with_split:
(%s) [%llx, %llx], res: (%s) [%llx, %llx]\n",
-                        conflict->name, conflict->start, conflict->end,
-                        name, start, end);
-
                /* failed, split and try again */

-               /* conflict coverred whole area */
+               /* conflict covered whole area */
                if (conflict->start <= start && conflict->end >= end)
                        return;

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Yinghai Lu]

On Thu, Sep 4, 2008 at 12:04 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
>> v7: remove wrong removing of write_unlock(&resource_lock) in adjust_resource()
>
> i've applied the generic commit below to tip/core/resources, and merged
> tip/core/resources into tip/x86/core, and added the remaining e820.c
> oneliner change (attached below as well).
>

wonder if need to put

  x86: split e820 reserved entries record to late v4
  IO resources: add reserve_region_with_split()
  x86: split e820 reserved entries record to late, v7

into 2.6.27 for fix hang on David's system.

YH

Re: [PATCH] x86: split e820 reserved entries record to late v4 - fix v7

[Ingo Molnar]

* Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> On Thu, Sep 4, 2008 at 12:04 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> >> v7: remove wrong removing of write_unlock(&resource_lock) in adjust_resource()
> >
> > i've applied the generic commit below to tip/core/resources, and merged
> > tip/core/resources into tip/x86/core, and added the remaining e820.c
> > oneliner change (attached below as well).
> >
> 
> wonder if need to put
> 
>   x86: split e820 reserved entries record to late v4
>   IO resources: add reserve_region_with_split()
>   x86: split e820 reserved entries record to late, v7
> 
> into 2.6.27 for fix hang on David's system.

too late i think. It hung on v2.6.26 too, right? So v2.6.28 would be 
more appropriate, and linux-next or tip/msater can be used as a 
practical kernel in the meanwhile.

	Ingo