[PATCH 0/2] printk: Allocate log buffer as early as possible

[PATCH 0/2] printk: Allocate log buffer as early as possible

[Mike Travis]

On larger systems, information in the kernel log is lost because
there is so much early text printed, that it overflows the static
log buffer before the log_buf_len kernel parameter can be processed,
and a bigger log buffer allocated.

Distros are relunctant to increase memory usage by increasing the
size of the static log buffer, so minimize the problem by allocating
the new log buffer as early as possible.

-- 

[PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Mike Travis]

[-- Attachment #1: add-no-memblock --]
[-- Type: text/plain, Size: 1013 bytes --]

	Add an error return if CONFIG_HAVE_MEMBLOCK is not set instead
	of having to add #ifdef CONFIG_HAVE_MEMBLOCK around blocks of
	code calling that function.

Authored-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Mike Travis <travis@sgi.com>
---
 include/linux/memblock.h |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

--- linux.orig/include/linux/memblock.h
+++ linux/include/linux/memblock.h
@@ -2,6 +2,8 @@
 #define _LINUX_MEMBLOCK_H
 #ifdef __KERNEL__
 
+#define MEMBLOCK_ERROR	0
+
 #ifdef CONFIG_HAVE_MEMBLOCK
 /*
  * Logical memory blocks.
@@ -20,7 +22,6 @@
 #include <asm/memblock.h>
 
 #define INIT_MEMBLOCK_REGIONS	128
-#define MEMBLOCK_ERROR		0
 
 struct memblock_region {
 	phys_addr_t base;
@@ -160,6 +161,12 @@ static inline unsigned long memblock_reg
 #define __initdata_memblock
 #endif
 
+#else
+static inline phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t align)
+{
+	return MEMBLOCK_ERROR;
+}
+
 #endif /* CONFIG_HAVE_MEMBLOCK */
 
 #endif /* __KERNEL__ */

-- 

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Andrew Morton]

On Mon, 25 Apr 2011 13:11:37 -0500
Mike Travis <travis@sgi.com> wrote:

> 	Add an error return if CONFIG_HAVE_MEMBLOCK is not set instead
> 	of having to add #ifdef CONFIG_HAVE_MEMBLOCK around blocks of
> 	code calling that function.
> 
> Authored-by: Yinghai Lu <yinghai@kernel.org>
> Signed-off-by: Mike Travis <travis@sgi.com>

There is no such thing as "Authored-by:".  If this patch was written by
yinghai then it must be tagged as From:him at the top of the changelog
and preferably has his signed-off-by: at the end.

Please clarify?

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Mike Travis]

Andrew Morton wrote:
> On Mon, 25 Apr 2011 13:11:37 -0500
> Mike Travis <travis@sgi.com> wrote:
> 
>> 	Add an error return if CONFIG_HAVE_MEMBLOCK is not set instead
>> 	of having to add #ifdef CONFIG_HAVE_MEMBLOCK around blocks of
>> 	code calling that function.
>>
>> Authored-by: Yinghai Lu <yinghai@kernel.org>
>> Signed-off-by: Mike Travis <travis@sgi.com>
> 
> There is no such thing as "Authored-by:".  If this patch was written by
> yinghai then it must be tagged as From:him at the top of the changelog
> and preferably has his signed-off-by: at the end.
> 
> Please clarify?

Yes, you have it correct.  I had added the From: line but when I receive
the email, it's removed.

Why not have an "Authored-by"?  That would eliminate the sendmail program
from screwing it up?

Thanks,
Mike

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> 
> 
> Andrew Morton wrote:
> >On Mon, 25 Apr 2011 13:11:37 -0500
> >Mike Travis <travis@sgi.com> wrote:
> >
> >>	Add an error return if CONFIG_HAVE_MEMBLOCK is not set instead
> >>	of having to add #ifdef CONFIG_HAVE_MEMBLOCK around blocks of
> >>	code calling that function.
> >>
> >>Authored-by: Yinghai Lu <yinghai@kernel.org>
> >>Signed-off-by: Mike Travis <travis@sgi.com>
> >
> >There is no such thing as "Authored-by:".  If this patch was written by
> >yinghai then it must be tagged as From:him at the top of the changelog
> >and preferably has his signed-off-by: at the end.
> >
> >Please clarify?
> 
> Yes, you have it correct.  I had added the From: line but when I receive
> the email, it's removed.
> 
> Why not have an "Authored-by"?  That would eliminate the sendmail program
> from screwing it up?

"From:" headers are properly recognized by Git and both git log and git 
annotate will show the right authorship. Authored-by does not get propagated.

Also, sendmail does not screw up From: headers that are in the body of the 
email - forwarding emails is one of the oldest things that can be done to 
emails.

Thanks,

	Ingo

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Valdis.Kletnieks]

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

On Wed, 27 Apr 2011 08:33:18 +0200, Ingo Molnar said:
> Also, sendmail does not screw up From: headers that are in the body of the 
> email - forwarding emails is one of the oldest things that can be done to 
> emails.

What some Sendmail configurations *do* eat is lines that start with 'From ' (no
colon). Those will get escaped as '>From ...'  to prevent them from looking
like a separator line for what's known as 'mbox' format - which is basically
the same info as in the Return-Path: header plus a timestamp. They look like:

>From hostmaster@prairie.net Fri Apr 19 15:50:15 2002

(If the above line has a >From, you have the issue.  Yes, it could be smarter
and do a full parse for a mbox header rather than just '^From '.  And yes, I
had to go digging to find an old archived mbox format file to snarf the example
from.


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

[PATCH 2/2] printk: Allocate kernel log buffer earlier v2

[Mike Travis]

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

	On larger systems, because of the numerous ACPI, Bootmem
	and EFI messages, the static log buffer overflows before the
	larger one specified by the log_buf_len param is allocated.
	Minimize the overflow by allocating the new log buffer as
	soon as possible.

	On kernels without memblock, a later call to setup_log_buf
	from kernel/init.c is the fallback.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 arch/x86/kernel/setup.c |    2 +
 include/linux/printk.h  |    4 ++
 init/main.c             |    1 
 kernel/printk.c         |   91 +++++++++++++++++++++++++++++++-----------------
 4 files changed, 67 insertions(+), 31 deletions(-)

--- linux.orig/arch/x86/kernel/setup.c
+++ linux/arch/x86/kernel/setup.c
@@ -975,6 +975,8 @@ void __init setup_arch(char **cmdline_p)
 	if (init_ohci1394_dma_early)
 		init_ohci1394_dma_on_all_controllers();
 #endif
+	/* Allocate bigger log buffer */
+	setup_log_buf(1);
 
 	reserve_initrd();
 
--- linux.orig/include/linux/printk.h
+++ linux/include/linux/printk.h
@@ -1,6 +1,8 @@
 #ifndef __KERNEL_PRINTK__
 #define __KERNEL_PRINTK__
 
+#include <linux/init.h>
+
 extern const char linux_banner[];
 extern const char linux_proc_banner[];
 
@@ -89,6 +91,8 @@ int no_printk(const char *fmt, ...)
 extern asmlinkage __attribute__ ((format (printf, 1, 2)))
 void early_printk(const char *fmt, ...);
 
+void __init setup_log_buf(int early);
+
 extern int printk_needs_cpu(int cpu);
 extern void printk_tick(void);
 
--- linux.orig/init/main.c
+++ linux/init/main.c
@@ -592,6 +592,7 @@ asmlinkage void __init start_kernel(void
 	 * These use large bootmem allocations and must precede
 	 * kmem_cache_init()
 	 */
+	setup_log_buf(0);
 	pidhash_init();
 	vfs_caches_init_early();
 	sort_main_extable();
--- linux.orig/kernel/printk.c
+++ linux/kernel/printk.c
@@ -31,6 +31,7 @@
 #include <linux/smp.h>
 #include <linux/security.h>
 #include <linux/bootmem.h>
+#include <linux/memblock.h>
 #include <linux/syscalls.h>
 #include <linux/kexec.h>
 #include <linux/kdb.h>
@@ -162,46 +163,74 @@ void log_buf_kexec_setup(void)
 }
 #endif
 
+/* requested log_buf_len from kernel cmdline */
+static unsigned long __initdata new_log_buf_len;
+
+/* save requested log_buf_len since it's too early to process it */
 static int __init log_buf_len_setup(char *str)
 {
 	unsigned size = memparse(str, &str);
-	unsigned long flags;
 
 	if (size)
 		size = roundup_pow_of_two(size);
-	if (size > log_buf_len) {
-		unsigned start, dest_idx, offset;
-		char *new_log_buf;
-
-		new_log_buf = alloc_bootmem(size);
-		if (!new_log_buf) {
-			printk(KERN_WARNING "log_buf_len: allocation failed\n");
-			goto out;
-		}
-
-		spin_lock_irqsave(&logbuf_lock, flags);
-		log_buf_len = size;
-		log_buf = new_log_buf;
-
-		offset = start = min(con_start, log_start);
-		dest_idx = 0;
-		while (start != log_end) {
-			log_buf[dest_idx] = __log_buf[start & (__LOG_BUF_LEN - 1)];
-			start++;
-			dest_idx++;
-		}
-		log_start -= offset;
-		con_start -= offset;
-		log_end -= offset;
-		spin_unlock_irqrestore(&logbuf_lock, flags);
+	if (size > log_buf_len)
+		new_log_buf_len = size;
 
-		printk(KERN_NOTICE "log_buf_len: %d\n", log_buf_len);
-	}
-out:
-	return 1;
+	return 0;
 }
+early_param("log_buf_len", log_buf_len_setup);
 
-__setup("log_buf_len=", log_buf_len_setup);
+void __init setup_log_buf(int early)
+{
+	unsigned long flags;
+	unsigned start, dest_idx, offset;
+	char *new_log_buf;
+	int free;
+
+	if (!new_log_buf_len)
+		return;
+
+	if (early) {
+		unsigned long mem;
+
+		mem = memblock_alloc(new_log_buf_len, PAGE_SIZE);
+		if (mem == MEMBLOCK_ERROR)
+			return;
+		new_log_buf = __va(mem);
+	}
+	else
+		new_log_buf = alloc_bootmem_nopanic(new_log_buf_len);
+
+	if (unlikely(!new_log_buf)) {
+		pr_err("log_buf_len: %ld bytes not available\n",
+			new_log_buf_len);
+		return;
+	}
+
+	spin_lock_irqsave(&logbuf_lock, flags);
+	log_buf_len = new_log_buf_len;
+	log_buf = new_log_buf;
+	new_log_buf_len = 0;
+	free = __LOG_BUF_LEN - log_end;
+
+	offset = start = min(con_start, log_start);
+	dest_idx = 0;
+	while (start != log_end) {
+		unsigned log_idx_mask = start & (__LOG_BUF_LEN - 1);
+
+		log_buf[dest_idx] = __log_buf[log_idx_mask];
+		start++;
+		dest_idx++;
+	}
+	log_start -= offset;
+	con_start -= offset;
+	log_end -= offset;
+	spin_unlock_irqrestore(&logbuf_lock, flags);
+
+	pr_info("log_buf_len: %d\n", log_buf_len);
+	pr_info("early log buf free: %d(%d%%)\n",
+		free, (free * 100) / __LOG_BUF_LEN);
+}
 
 #ifdef CONFIG_BOOT_PRINTK_DELAY
 

-- 

Re: [PATCH 0/2] printk: Allocate log buffer as early as possible

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> 
> On larger systems, information in the kernel log is lost because
> there is so much early text printed, that it overflows the static
> log buffer before the log_buf_len kernel parameter can be processed,
> and a bigger log buffer allocated.
> 
> Distros are relunctant to increase memory usage by increasing the
> size of the static log buffer, so minimize the problem by allocating
> the new log buffer as early as possible.

I think this looks sane in principle and will solve quite a few 'the log buffer 
got too large' problems which previously resulted in somewhat complex looking 
printk-reduction patches. Andrew, if you agree with the patches, wanna carry 
them?

Thanks,

	Ingo

[PATCH 0/4] init: Shrink early messages to prevent overflowing the kernel log buffer

[Mike Travis]

On larger systems, information in the kernel log is lost because
there is so much early text printed, that it overflows the static
log buffer before the log_buf_len kernel parameter can be processed,
and a bigger log buffer allocated.

Distros are relunctant to increase memory usage by increasing the
size of the static log buffer, so minimize the problem by allocating
the new log buffer as early as possible.  Unfortunately allocating
early does not recover all the early messages, so we also need to
reduce the amount of characters those early messages generate.

Here are the stats testing on a system with 248 nodes, 606 EFI
mem ranges, 1984 cores

after get_log_buff_early:  (17% overflow)

	[    0.000000] early log_buf free: -45723/262183(-17%)
	[    0.000000] first line: : mem339: type=7, attr=0xf, range=[0x00000e6000000000-0x00000e6fff000000) (6552

Here I enabled some cores that were disabled so now the system
has 248 nodes, 606 EFI mem ranges, 2368 cores.

after minimize-time-zero-msgs:  (5% overflow)

[0] early log_buf free: -15184/262172(-5%)
[0] first line: [0x000000007226e000-0x0000000072271000) (0MB) <6>[0] EFI: mem67: type=3, attr=0

after minimize-srat-msgs.v2: (28% free)

[0] early log_buf free: 73556/188616(28%)
[0] first line: <6>[0] Initializing cgroup subsys cpuset <6>[0] Initializing cgroup subsys cpu

Some previous stats from testing these changes on our current lab
UV systems.  (Both of these systems lost all of the e820 and EFI
memmap ranges before the changes.)

System X:
	8,793,945,145,344 bytes of system memory
	256 nodes
	599 EFI Mem ranges
	4096 cpu_ids
	43% of static log buffer unused

System Y:
	11,779,115,188,224 bytes of system memory
	492 Nodes
	976 EFI Mem ranges
	1968 cpu_ids
	17% of static log buffer unused

The last stat is how close the static log buffer came
to overflowing.  While these resources are fairly close
to today's max limits, there is not a lot of head room
for growth.

An alternative for the future might be to create a larger
static log buffer in the __initdata section, and then
always allocate a dynamically sized log buffer to replace
it.  This would also allow shrinking the log buffer for
memory tight situations.  But it would add complexity to
the code.

-- 

[PATCH 1/4] printk: Allocate kernel log buffer earlier

[Mike Travis]

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

On larger systems, because of the numerous ACPI, Bootmem and EFI
messages, the static log buffer overflows before the larger one
specified by the log_buf_len param is allocated.  Minimize the
potential for overflow by allocating the new log buffer as soon
as possible.

We do this by changing the log_buf_len from an early_param to a
_setup param.  But _setup params are processed before the
alloc_bootmem is available, so this function will now just save
the requested log buf len.  The real work routine (setup_log_buf)
is called after bootmem is available.

Signed-off-by: Mike Travis <travis@sgi.com>
Reviewed-by: Jack Steiner <steiner@sgi.com>
Reviewed-by: Robin Holt <holt@sgi.com>
---
 arch/x86/kernel/setup.c |    5 +++
 include/linux/printk.h  |    4 ++
 init/main.c             |    1 
 kernel/printk.c         |   75 ++++++++++++++++++++++++++++--------------------
 4 files changed, 54 insertions(+), 31 deletions(-)

--- linux.orig/arch/x86/kernel/setup.c
+++ linux/arch/x86/kernel/setup.c
@@ -1007,6 +1007,11 @@ void __init setup_arch(char **cmdline_p)
 	memblock_find_dma_reserve();
 	dma32_reserve_bootmem();
 
+	/*
+	 * Allocate bigger log buffer as early as possible
+	 */
+	setup_log_buf();
+
 #ifdef CONFIG_KVM_CLOCK
 	kvmclock_init();
 #endif
--- linux.orig/include/linux/printk.h
+++ linux/include/linux/printk.h
@@ -1,6 +1,8 @@
 #ifndef __KERNEL_PRINTK__
 #define __KERNEL_PRINTK__
 
+#include <linux/init.h>
+
 extern const char linux_banner[];
 extern const char linux_proc_banner[];
 
@@ -89,6 +91,8 @@ int no_printk(const char *fmt, ...)
 extern asmlinkage __attribute__ ((format (printf, 1, 2)))
 void early_printk(const char *fmt, ...);
 
+void __init setup_log_buf(void);
+
 extern int printk_needs_cpu(int cpu);
 extern void printk_tick(void);
 
--- linux.orig/init/main.c
+++ linux/init/main.c
@@ -592,6 +592,7 @@ asmlinkage void __init start_kernel(void
 	 * These use large bootmem allocations and must precede
 	 * kmem_cache_init()
 	 */
+	setup_log_buf();
 	pidhash_init();
 	vfs_caches_init_early();
 	sort_main_extable();
--- linux.orig/kernel/printk.c
+++ linux/kernel/printk.c
@@ -162,46 +162,59 @@ void log_buf_kexec_setup(void)
 }
 #endif
 
+/* requested log_buf_len from kernel cmdline */
+static unsigned long __initdata new_log_buf_len;
+
+/* save requested log_buf_len since it's too early to process it */
 static int __init log_buf_len_setup(char *str)
 {
 	unsigned size = memparse(str, &str);
-	unsigned long flags;
 
 	if (size)
 		size = roundup_pow_of_two(size);
-	if (size > log_buf_len) {
-		unsigned start, dest_idx, offset;
-		char *new_log_buf;
-
-		new_log_buf = alloc_bootmem(size);
-		if (!new_log_buf) {
-			printk(KERN_WARNING "log_buf_len: allocation failed\n");
-			goto out;
-		}
-
-		spin_lock_irqsave(&logbuf_lock, flags);
-		log_buf_len = size;
-		log_buf = new_log_buf;
-
-		offset = start = min(con_start, log_start);
-		dest_idx = 0;
-		while (start != log_end) {
-			log_buf[dest_idx] = __log_buf[start & (__LOG_BUF_LEN - 1)];
-			start++;
-			dest_idx++;
-		}
-		log_start -= offset;
-		con_start -= offset;
-		log_end -= offset;
-		spin_unlock_irqrestore(&logbuf_lock, flags);
+	if (size > log_buf_len)
+		new_log_buf_len = size;
 
-		printk(KERN_NOTICE "log_buf_len: %d\n", log_buf_len);
-	}
-out:
-	return 1;
+	return 0;
 }
+early_param("log_buf_len", log_buf_len_setup);
 
-__setup("log_buf_len=", log_buf_len_setup);
+void __init setup_log_buf(void)
+{
+	unsigned long flags;
+	unsigned start, dest_idx, offset;
+	char *new_log_buf;
+	int free;
+
+	if (!new_log_buf_len)
+		return;
+
+	new_log_buf = alloc_bootmem(new_log_buf_len);
+
+	spin_lock_irqsave(&logbuf_lock, flags);
+	log_buf_len = new_log_buf_len;
+	log_buf = new_log_buf;
+	new_log_buf_len = 0;
+	free = __LOG_BUF_LEN - log_end;
+
+	offset = start = min(con_start, log_start);
+	dest_idx = 0;
+	while (start != log_end) {
+		unsigned log_idx_mask = start & (__LOG_BUF_LEN - 1);
+
+		log_buf[dest_idx] = __log_buf[log_idx_mask];
+		start++;
+		dest_idx++;
+	}
+	log_start -= offset;
+	con_start -= offset;
+	log_end -= offset;
+	spin_unlock_irqrestore(&logbuf_lock, flags);
+
+	pr_info("log_buf_len: %d\n", log_buf_len);
+	pr_info("early log buf free: %d(%d%%)\n",
+		free, (free * 100) / __LOG_BUF_LEN);
+}
 
 #ifdef CONFIG_BOOT_PRINTK_DELAY
 

-- 

Re: [PATCH 1/4] printk: Allocate kernel log buffer earlier

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> On larger systems, because of the numerous ACPI, Bootmem and EFI
> messages, the static log buffer overflows before the larger one
> specified by the log_buf_len param is allocated.  Minimize the
> potential for overflow by allocating the new log buffer as soon
> as possible.
> 
> We do this by changing the log_buf_len from an early_param to a
> _setup param.  But _setup params are processed before the
> alloc_bootmem is available, so this function will now just save
> the requested log buf len.  The real work routine (setup_log_buf)
> is called after bootmem is available.
> 
> Signed-off-by: Mike Travis <travis@sgi.com>
> Reviewed-by: Jack Steiner <steiner@sgi.com>
> Reviewed-by: Robin Holt <holt@sgi.com>
> ---
>  arch/x86/kernel/setup.c |    5 +++
>  include/linux/printk.h  |    4 ++
>  init/main.c             |    1 
>  kernel/printk.c         |   75 ++++++++++++++++++++++++++++--------------------
>  4 files changed, 54 insertions(+), 31 deletions(-)

Well, the modern allocation method is memblock - available on all major 
architectures.

You could avoid all this ugly workaround of bootmem limitations by moving the 
allocation to memblock_alloc() and desupporting the log_buf_len= boot parameter
on non-memblock architectures.

kernel log buffer size can be configured via the .config so they will not be left 
without larger buffers.

Doing this should also have the advantage of getting all the early x86 messages into 
the larger buffer already, reducing the pressure to apply some of the other patches 
in your series.

Thanks,

	Ingo

Re: [PATCH 1/4] printk: Allocate kernel log buffer earlier

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> You could avoid all this ugly workaround of bootmem limitations by moving the 
> allocation to memblock_alloc() and desupporting the log_buf_len= boot parameter on 
> non-memblock architectures.

memblock_alloc() could return -ENOSYS on architectures that do not implement it - 
thus enabling such optional features without ugly #ifdef CONFIG_HAVE_MEMBLOCK 
conditionals.

Thanks,

	Ingo

Re: [PATCH 1/4] printk: Allocate kernel log buffer earlier

[Yinghai Lu]

On 02/27/2011 04:15 AM, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
>> You could avoid all this ugly workaround of bootmem limitations by moving the 
>> allocation to memblock_alloc() and desupporting the log_buf_len= boot parameter on 
>> non-memblock architectures.
> 
> memblock_alloc() could return -ENOSYS on architectures that do not implement it - 
> thus enabling such optional features without ugly #ifdef CONFIG_HAVE_MEMBLOCK 
> conditionals.

Mike,

please check updated patch...

with the memblock change, you don't need to change acpi SRAT handling etc any more.

new buffer will be near high mem under 4g.

Thanks

Yinghai

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index a47fe00..69b8995 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -974,6 +974,11 @@ void __init setup_arch(char **cmdline_p)
 	memblock.current_limit = get_max_mapped();
 
 	/*
+	 * Allocate bigger log buffer as early as possible
+	 */
+	setup_log_buf();
+
+	/*
 	 * NOTE: On x86-32, only from this point on, fixmaps are ready for use.
 	 */
 
diff --git a/include/linux/memblock.h b/include/linux/memblock.h
index 62a10c2..c3ade22 100644
--- a/include/linux/memblock.h
+++ b/include/linux/memblock.h
@@ -2,6 +2,8 @@
 #define _LINUX_MEMBLOCK_H
 #ifdef __KERNEL__
 
+#define MEMBLOCK_ERROR		0
+
 #ifdef CONFIG_HAVE_MEMBLOCK
 /*
  * Logical memory blocks.
@@ -20,7 +22,6 @@
 #include <asm/memblock.h>
 
 #define INIT_MEMBLOCK_REGIONS	128
-#define MEMBLOCK_ERROR		0
 
 struct memblock_region {
 	phys_addr_t base;
@@ -160,6 +161,12 @@ static inline unsigned long memblock_region_reserved_end_pfn(const struct memblo
 #define __initdata_memblock
 #endif
 
+#else
+static inline phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t align)
+{
+	return MEMBLOCK_ERROR;
+}
+
 #endif /* CONFIG_HAVE_MEMBLOCK */
 
 #endif /* __KERNEL__ */
diff --git a/include/linux/printk.h b/include/linux/printk.h
index ee048e7..fd266a8 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -1,6 +1,8 @@
 #ifndef __KERNEL_PRINTK__
 #define __KERNEL_PRINTK__
 
+#include <linux/init.h>
+
 extern const char linux_banner[];
 extern const char linux_proc_banner[];
 
@@ -89,6 +91,8 @@ int no_printk(const char *fmt, ...)
 extern asmlinkage __attribute__ ((format (printf, 1, 2)))
 void early_printk(const char *fmt, ...);
 
+void __init setup_log_buf(void);
+
 extern int printk_needs_cpu(int cpu);
 extern void printk_tick(void);
 
diff --git a/init/main.c b/init/main.c
index 33c37c3..2085bb3 100644
--- a/init/main.c
+++ b/init/main.c
@@ -592,6 +592,7 @@ asmlinkage void __init start_kernel(void)
 	 * These use large bootmem allocations and must precede
 	 * kmem_cache_init()
 	 */
+	setup_log_buf();
 	pidhash_init();
 	vfs_caches_init_early();
 	sort_main_extable();
diff --git a/kernel/printk.c b/kernel/printk.c
index 3623152..14fa4d0 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -31,6 +31,7 @@
 #include <linux/smp.h>
 #include <linux/security.h>
 #include <linux/bootmem.h>
+#include <linux/memblock.h>
 #include <linux/syscalls.h>
 #include <linux/kexec.h>
 #include <linux/kdb.h>
@@ -162,46 +163,64 @@ void log_buf_kexec_setup(void)
 }
 #endif
 
+/* requested log_buf_len from kernel cmdline */
+static unsigned long __initdata new_log_buf_len;
+
+/* save requested log_buf_len since it's too early to process it */
 static int __init log_buf_len_setup(char *str)
 {
 	unsigned size = memparse(str, &str);
-	unsigned long flags;
 
 	if (size)
 		size = roundup_pow_of_two(size);
-	if (size > log_buf_len) {
-		unsigned start, dest_idx, offset;
-		char *new_log_buf;
+	if (size > log_buf_len)
+		new_log_buf_len = size;
 
-		new_log_buf = alloc_bootmem(size);
-		if (!new_log_buf) {
-			printk(KERN_WARNING "log_buf_len: allocation failed\n");
-			goto out;
-		}
+	return 0;
+}
+early_param("log_buf_len", log_buf_len_setup);
 
-		spin_lock_irqsave(&logbuf_lock, flags);
-		log_buf_len = size;
-		log_buf = new_log_buf;
-
-		offset = start = min(con_start, log_start);
-		dest_idx = 0;
-		while (start != log_end) {
-			log_buf[dest_idx] = __log_buf[start & (__LOG_BUF_LEN - 1)];
-			start++;
-			dest_idx++;
-		}
-		log_start -= offset;
-		con_start -= offset;
-		log_end -= offset;
-		spin_unlock_irqrestore(&logbuf_lock, flags);
+void __init setup_log_buf(void)
+{
+	unsigned long flags;
+	unsigned start, dest_idx, offset;
+	char *new_log_buf;
+	phys_addr_t new_addr;
+	int free;
+
+	if (!new_log_buf_len)
+		return;
 
-		printk(KERN_NOTICE "log_buf_len: %d\n", log_buf_len);
+	new_addr = memblock_alloc(new_log_buf_len, PAGE_SIZE);
+	if (new_addr != MEMBLOCK_ERROR)
+		new_log_buf = __va(new_addr);
+	else
+		new_log_buf = alloc_bootmem(new_log_buf_len);
+
+	spin_lock_irqsave(&logbuf_lock, flags);
+	log_buf_len = new_log_buf_len;
+	log_buf = new_log_buf;
+	new_log_buf_len = 0;
+	free = __LOG_BUF_LEN - log_end;
+
+	offset = start = min(con_start, log_start);
+	dest_idx = 0;
+	while (start != log_end) {
+		unsigned log_idx_mask = start & (__LOG_BUF_LEN - 1);
+
+		log_buf[dest_idx] = __log_buf[log_idx_mask];
+		start++;
+		dest_idx++;
 	}
-out:
-	return 1;
-}
+	log_start -= offset;
+	con_start -= offset;
+	log_end -= offset;
+	spin_unlock_irqrestore(&logbuf_lock, flags);
 
-__setup("log_buf_len=", log_buf_len_setup);
+	pr_info("log_buf_len: %d\n", log_buf_len);
+	pr_info("early log buf free: %d(%d%%)\n",
+		free, (free * 100) / __LOG_BUF_LEN);
+}
 
 #ifdef CONFIG_BOOT_PRINTK_DELAY
 

Re: [PATCH 1/4] printk: Allocate kernel log buffer earlier

[Mike Travis]

Yinghai Lu wrote:
> On 02/27/2011 04:15 AM, Ingo Molnar wrote:
>> * Ingo Molnar <mingo@elte.hu> wrote:
>>
>>> You could avoid all this ugly workaround of bootmem limitations by moving the 
>>> allocation to memblock_alloc() and desupporting the log_buf_len= boot parameter on 
>>> non-memblock architectures.
>> memblock_alloc() could return -ENOSYS on architectures that do not implement it - 
>> thus enabling such optional features without ugly #ifdef CONFIG_HAVE_MEMBLOCK 
>> conditionals.
> 
> Mike,
> 
> please check updated patch...
> 
> with the memblock change, you don't need to change acpi SRAT handling etc any more.

I had to debug a weird ACPI -> Node mapping last week and the
"improved" SRAT messages helped that considerably.  It was
far easier to spot which Node didn't have the correct assignments.
I'd submit that patch even without needing fewer (like 512 lines
max instead of 4096 lines max) bytes in the log buffer.

> 
> new buffer will be near high mem under 4g.

Is this really that important?  The patchset is ridiculously
simple as it is.  Do I really need to spend more time on it?
I've already done 4 revisions of it and responded to all
objections.

I have a far more important patchset that I've been trying to
prepare that actually fixes broken code, instead of trying to
reduce a few thousand bytes in the log buffer.

And everyone that has experienced it here on our lab systems
say they like the improvements (even the ones that I had to
toss because of "that's not the way it was before" kind of
objections.)

Let's move on to far more important problems.

Thanks,
Mike


> 
> Thanks
> 
> Yinghai
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index a47fe00..69b8995 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -974,6 +974,11 @@ void __init setup_arch(char **cmdline_p)
>  	memblock.current_limit = get_max_mapped();
>  
>  	/*
> +	 * Allocate bigger log buffer as early as possible
> +	 */
> +	setup_log_buf();
> +
> +	/*
>  	 * NOTE: On x86-32, only from this point on, fixmaps are ready for use.
>  	 */
>  
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 62a10c2..c3ade22 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -2,6 +2,8 @@
>  #define _LINUX_MEMBLOCK_H
>  #ifdef __KERNEL__
>  
> +#define MEMBLOCK_ERROR		0
> +
>  #ifdef CONFIG_HAVE_MEMBLOCK
>  /*
>   * Logical memory blocks.
> @@ -20,7 +22,6 @@
>  #include <asm/memblock.h>
>  
>  #define INIT_MEMBLOCK_REGIONS	128
> -#define MEMBLOCK_ERROR		0
>  
>  struct memblock_region {
>  	phys_addr_t base;
> @@ -160,6 +161,12 @@ static inline unsigned long memblock_region_reserved_end_pfn(const struct memblo
>  #define __initdata_memblock
>  #endif
>  
> +#else
> +static inline phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t align)
> +{
> +	return MEMBLOCK_ERROR;
> +}
> +
>  #endif /* CONFIG_HAVE_MEMBLOCK */
>  
>  #endif /* __KERNEL__ */
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index ee048e7..fd266a8 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -1,6 +1,8 @@
>  #ifndef __KERNEL_PRINTK__
>  #define __KERNEL_PRINTK__
>  
> +#include <linux/init.h>
> +
>  extern const char linux_banner[];
>  extern const char linux_proc_banner[];
>  
> @@ -89,6 +91,8 @@ int no_printk(const char *fmt, ...)
>  extern asmlinkage __attribute__ ((format (printf, 1, 2)))
>  void early_printk(const char *fmt, ...);
>  
> +void __init setup_log_buf(void);
> +
>  extern int printk_needs_cpu(int cpu);
>  extern void printk_tick(void);
>  
> diff --git a/init/main.c b/init/main.c
> index 33c37c3..2085bb3 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -592,6 +592,7 @@ asmlinkage void __init start_kernel(void)
>  	 * These use large bootmem allocations and must precede
>  	 * kmem_cache_init()
>  	 */
> +	setup_log_buf();
>  	pidhash_init();
>  	vfs_caches_init_early();
>  	sort_main_extable();
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 3623152..14fa4d0 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -31,6 +31,7 @@
>  #include <linux/smp.h>
>  #include <linux/security.h>
>  #include <linux/bootmem.h>
> +#include <linux/memblock.h>
>  #include <linux/syscalls.h>
>  #include <linux/kexec.h>
>  #include <linux/kdb.h>
> @@ -162,46 +163,64 @@ void log_buf_kexec_setup(void)
>  }
>  #endif
>  
> +/* requested log_buf_len from kernel cmdline */
> +static unsigned long __initdata new_log_buf_len;
> +
> +/* save requested log_buf_len since it's too early to process it */
>  static int __init log_buf_len_setup(char *str)
>  {
>  	unsigned size = memparse(str, &str);
> -	unsigned long flags;
>  
>  	if (size)
>  		size = roundup_pow_of_two(size);
> -	if (size > log_buf_len) {
> -		unsigned start, dest_idx, offset;
> -		char *new_log_buf;
> +	if (size > log_buf_len)
> +		new_log_buf_len = size;
>  
> -		new_log_buf = alloc_bootmem(size);
> -		if (!new_log_buf) {
> -			printk(KERN_WARNING "log_buf_len: allocation failed\n");
> -			goto out;
> -		}
> +	return 0;
> +}
> +early_param("log_buf_len", log_buf_len_setup);
>  
> -		spin_lock_irqsave(&logbuf_lock, flags);
> -		log_buf_len = size;
> -		log_buf = new_log_buf;
> -
> -		offset = start = min(con_start, log_start);
> -		dest_idx = 0;
> -		while (start != log_end) {
> -			log_buf[dest_idx] = __log_buf[start & (__LOG_BUF_LEN - 1)];
> -			start++;
> -			dest_idx++;
> -		}
> -		log_start -= offset;
> -		con_start -= offset;
> -		log_end -= offset;
> -		spin_unlock_irqrestore(&logbuf_lock, flags);
> +void __init setup_log_buf(void)
> +{
> +	unsigned long flags;
> +	unsigned start, dest_idx, offset;
> +	char *new_log_buf;
> +	phys_addr_t new_addr;
> +	int free;
> +
> +	if (!new_log_buf_len)
> +		return;
>  
> -		printk(KERN_NOTICE "log_buf_len: %d\n", log_buf_len);
> +	new_addr = memblock_alloc(new_log_buf_len, PAGE_SIZE);
> +	if (new_addr != MEMBLOCK_ERROR)
> +		new_log_buf = __va(new_addr);
> +	else
> +		new_log_buf = alloc_bootmem(new_log_buf_len);
> +
> +	spin_lock_irqsave(&logbuf_lock, flags);
> +	log_buf_len = new_log_buf_len;
> +	log_buf = new_log_buf;
> +	new_log_buf_len = 0;
> +	free = __LOG_BUF_LEN - log_end;
> +
> +	offset = start = min(con_start, log_start);
> +	dest_idx = 0;
> +	while (start != log_end) {
> +		unsigned log_idx_mask = start & (__LOG_BUF_LEN - 1);
> +
> +		log_buf[dest_idx] = __log_buf[log_idx_mask];
> +		start++;
> +		dest_idx++;
>  	}
> -out:
> -	return 1;
> -}
> +	log_start -= offset;
> +	con_start -= offset;
> +	log_end -= offset;
> +	spin_unlock_irqrestore(&logbuf_lock, flags);
>  
> -__setup("log_buf_len=", log_buf_len_setup);
> +	pr_info("log_buf_len: %d\n", log_buf_len);
> +	pr_info("early log buf free: %d(%d%%)\n",
> +		free, (free * 100) / __LOG_BUF_LEN);
> +}
>  
>  #ifdef CONFIG_BOOT_PRINTK_DELAY
>  
> 

Re: [PATCH 1/4] printk: Allocate kernel log buffer earlier

[Yinghai Lu]

On Mon, Feb 28, 2011 at 11:23 AM, Mike Travis <travis@sgi.com> wrote:
>
>
> Yinghai Lu wrote:
>>
>> On 02/27/2011 04:15 AM, Ingo Molnar wrote:
>>>
>>> * Ingo Molnar <mingo@elte.hu> wrote:
>>>
>>>> You could avoid all this ugly workaround of bootmem limitations by
>>>> moving the allocation to memblock_alloc() and desupporting the log_buf_len=
>>>> boot parameter on non-memblock architectures.
>>>
>>> memblock_alloc() could return -ENOSYS on architectures that do not
>>> implement it - thus enabling such optional features without ugly #ifdef
>>> CONFIG_HAVE_MEMBLOCK conditionals.
>>
>> Mike,
>>
>> please check updated patch...
>>
>> with the memblock change, you don't need to change acpi SRAT handling etc
>> any more.
>
> I had to debug a weird ACPI -> Node mapping last week and the
> "improved" SRAT messages helped that considerably.  It was
> far easier to spot which Node didn't have the correct assignments.
> I'd submit that patch even without needing fewer (like 512 lines
> max instead of 4096 lines max) bytes in the log buffer.

Your current change to ACPI srat is not complete yet.

you only handle x2apic entries.

According to ACPI 4.0 spec, We should have mixed entries with apic
entries and x2apic entries.
apic entries are for apic id < 255.
x2apic entries are for apic id > 255.

Yinghai

Re: [PATCH 1/4] printk: Allocate kernel log buffer earlier

[Mike Travis]

Yinghai Lu wrote:
> On Mon, Feb 28, 2011 at 11:23 AM, Mike Travis <travis@sgi.com> wrote:
>>
>> Yinghai Lu wrote:
>>> On 02/27/2011 04:15 AM, Ingo Molnar wrote:
>>>> * Ingo Molnar <mingo@elte.hu> wrote:
>>>>
>>>>> You could avoid all this ugly workaround of bootmem limitations by
>>>>> moving the allocation to memblock_alloc() and desupporting the log_buf_len=
>>>>> boot parameter on non-memblock architectures.
>>>> memblock_alloc() could return -ENOSYS on architectures that do not
>>>> implement it - thus enabling such optional features without ugly #ifdef
>>>> CONFIG_HAVE_MEMBLOCK conditionals.
>>> Mike,
>>>
>>> please check updated patch...
>>>
>>> with the memblock change, you don't need to change acpi SRAT handling etc
>>> any more.
>> I had to debug a weird ACPI -> Node mapping last week and the
>> "improved" SRAT messages helped that considerably.  It was
>> far easier to spot which Node didn't have the correct assignments.
>> I'd submit that patch even without needing fewer (like 512 lines
>> max instead of 4096 lines max) bytes in the log buffer.
> 
> Your current change to ACPI srat is not complete yet.
> 
> you only handle x2apic entries.
> 
> According to ACPI 4.0 spec, We should have mixed entries with apic
> entries and x2apic entries.
> apic entries are for apic id < 255.
> x2apic entries are for apic id > 255.
> 
> Yinghai

Are you sure you can run both "legacy" and "x2" apic modes in
the same SSI under the Intel or AMD rules?

(And it's highly probable that you cannot overflow the log
buffer with less than 256 CPU's.)

Thanks,
Mike

Re: [PATCH 1/4] printk: Allocate kernel log buffer earlier

[Yinghai Lu]

On Mon, Feb 28, 2011 at 12:02 PM, Mike Travis <travis@sgi.com> wrote:
>
>
> Yinghai Lu wrote:
>>
>> On Mon, Feb 28, 2011 at 11:23 AM, Mike Travis <travis@sgi.com> wrote:
>>>
>>> Yinghai Lu wrote:
>>>>
>>>> On 02/27/2011 04:15 AM, Ingo Molnar wrote:
>>>>>
>>>>> * Ingo Molnar <mingo@elte.hu> wrote:
>>>>>
>>>>>> You could avoid all this ugly workaround of bootmem limitations by
>>>>>> moving the allocation to memblock_alloc() and desupporting the
>>>>>> log_buf_len=
>>>>>> boot parameter on non-memblock architectures.
>>>>>
>>>>> memblock_alloc() could return -ENOSYS on architectures that do not
>>>>> implement it - thus enabling such optional features without ugly #ifdef
>>>>> CONFIG_HAVE_MEMBLOCK conditionals.
>>>>
>>>> Mike,
>>>>
>>>> please check updated patch...
>>>>
>>>> with the memblock change, you don't need to change acpi SRAT handling
>>>> etc
>>>> any more.
>>>
>>> I had to debug a weird ACPI -> Node mapping last week and the
>>> "improved" SRAT messages helped that considerably.  It was
>>> far easier to spot which Node didn't have the correct assignments.
>>> I'd submit that patch even without needing fewer (like 512 lines
>>> max instead of 4096 lines max) bytes in the log buffer.
>>
>> Your current change to ACPI srat is not complete yet.
>>
>> you only handle x2apic entries.
>>
>> According to ACPI 4.0 spec, We should have mixed entries with apic
>> entries and x2apic entries.
>> apic entries are for apic id < 255.
>> x2apic entries are for apic id > 255.
>>
>> Yinghai
>
> Are you sure you can run both "legacy" and "x2" apic modes in
> the same SSI under the Intel or AMD rules?
>
No.

According to ACPI 4.0 Spec:
even the CPUs are with x2apic mode (aka pre-enabled in BIOS),
if their apic id < 255, BIOS still need to use xapic entries for them.

Yinghai

[PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Mike Travis]

Subject: memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set
Author: Yinghai Lu <yinghai@kernel.org>

	Add an error return if CONFIG_HAVE_MEMBLOCK is not set instead
	of having to add #ifdef CONFIG_HAVE_MEMBLOCK around blocks of
	code calling that function.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 include/linux/memblock.h |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

--- linux.orig/include/linux/memblock.h
+++ linux/include/linux/memblock.h
@@ -2,6 +2,8 @@
 #define _LINUX_MEMBLOCK_H
 #ifdef __KERNEL__
 
+#define MEMBLOCK_ERROR	0
+
 #ifdef CONFIG_HAVE_MEMBLOCK
 /*
  * Logical memory blocks.
@@ -20,7 +22,6 @@
 #include <asm/memblock.h>
 
 #define INIT_MEMBLOCK_REGIONS	128
-#define MEMBLOCK_ERROR		0
 
 struct memblock_region {
 	phys_addr_t base;
@@ -160,6 +161,12 @@ static inline unsigned long memblock_reg
 #define __initdata_memblock
 #endif
 
+#else
+static inline phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t align)
+{
+	return MEMBLOCK_ERROR;
+}
+
 #endif /* CONFIG_HAVE_MEMBLOCK */
 
 #endif /* __KERNEL__ */

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Yinghai Lu]

On Wed, Mar 30, 2011 at 5:41 PM, Mike Travis <travis@sgi.com> wrote:
> Subject: memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set
> Author: Yinghai Lu <yinghai@kernel.org>
>
>        Add an error return if CONFIG_HAVE_MEMBLOCK is not set instead
>        of having to add #ifdef CONFIG_HAVE_MEMBLOCK around blocks of
>        code calling that function.
>
> Signed-off-by: Mike Travis <travis@sgi.com>
> ---
> include/linux/memblock.h |    9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> --- linux.orig/include/linux/memblock.h
> +++ linux/include/linux/memblock.h
> @@ -2,6 +2,8 @@
> #define _LINUX_MEMBLOCK_H
> #ifdef __KERNEL__
>
> +#define MEMBLOCK_ERROR 0
> +
> #ifdef CONFIG_HAVE_MEMBLOCK
> /*
>  * Logical memory blocks.
> @@ -20,7 +22,6 @@
> #include <asm/memblock.h>
>
> #define INIT_MEMBLOCK_REGIONS   128
> -#define MEMBLOCK_ERROR         0
>
> struct memblock_region {
>        phys_addr_t base;
> @@ -160,6 +161,12 @@ static inline unsigned long memblock_reg
> #define __initdata_memblock
> #endif
>
> +#else
> +static inline phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t
> align)
> +{
> +       return MEMBLOCK_ERROR;
> +}
> +
> #endif /* CONFIG_HAVE_MEMBLOCK */
>
> #endif /* __KERNEL__ */
>

setup_log_buf will pass function pointer, So this one is not needed, right?

Thanks

Yinghai

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Mike Travis]

Yinghai Lu wrote:
> On Wed, Mar 30, 2011 at 5:41 PM, Mike Travis <travis@sgi.com> wrote:
>> Subject: memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set
>> Author: Yinghai Lu <yinghai@kernel.org>
>>
>>        Add an error return if CONFIG_HAVE_MEMBLOCK is not set instead
>>        of having to add #ifdef CONFIG_HAVE_MEMBLOCK around blocks of
>>        code calling that function.
>>
>> Signed-off-by: Mike Travis <travis@sgi.com>
>> ---
>> include/linux/memblock.h |    9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> --- linux.orig/include/linux/memblock.h
>> +++ linux/include/linux/memblock.h
>> @@ -2,6 +2,8 @@
>> #define _LINUX_MEMBLOCK_H
>> #ifdef __KERNEL__
>>
>> +#define MEMBLOCK_ERROR 0
>> +
>> #ifdef CONFIG_HAVE_MEMBLOCK
>> /*
>>  * Logical memory blocks.
>> @@ -20,7 +22,6 @@
>> #include <asm/memblock.h>
>>
>> #define INIT_MEMBLOCK_REGIONS   128
>> -#define MEMBLOCK_ERROR         0
>>
>> struct memblock_region {
>>        phys_addr_t base;
>> @@ -160,6 +161,12 @@ static inline unsigned long memblock_reg
>> #define __initdata_memblock
>> #endif
>>
>> +#else
>> +static inline phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t
>> align)
>> +{
>> +       return MEMBLOCK_ERROR;
>> +}
>> +
>> #endif /* CONFIG_HAVE_MEMBLOCK */
>>
>> #endif /* __KERNEL__ */
>>
> 
> setup_log_buf will pass function pointer, So this one is not needed, right?

The other function would need the #ifdef CONFIG_HAVE_MEMBLOCK before calling
memblock_alloc which I thought was the point of this patch?  Note we still
have the last fallback of using alloc_boot_mem in kernel/init.c.

Thanks,
Mike

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Yinghai Lu]

On Thu, Mar 31, 2011 at 8:23 AM, Mike Travis <travis@sgi.com> wrote:
>
>>
>> setup_log_buf will pass function pointer, So this one is not needed,
>> right?
>
> The other function would need the #ifdef CONFIG_HAVE_MEMBLOCK before calling
> memblock_alloc which I thought was the point of this patch?  Note we still
> have the last fallback of using alloc_boot_mem in kernel/init.c.

before that patch, it already use bootmem allocation.

So should be ok to drop this one.

Thanks

Yinghai

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Mike Travis]

Was there any further objections to these patches?

Mike Travis wrote:
> Subject: memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set
> Author: Yinghai Lu <yinghai@kernel.org>
> 
>     Add an error return if CONFIG_HAVE_MEMBLOCK is not set instead
>     of having to add #ifdef CONFIG_HAVE_MEMBLOCK around blocks of
>     code calling that function.
> 
> Signed-off-by: Mike Travis <travis@sgi.com>
> ---
> include/linux/memblock.h |    9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
> 
> --- linux.orig/include/linux/memblock.h
> +++ linux/include/linux/memblock.h
> @@ -2,6 +2,8 @@
> #define _LINUX_MEMBLOCK_H
> #ifdef __KERNEL__
> 
> +#define MEMBLOCK_ERROR    0
> +
> #ifdef CONFIG_HAVE_MEMBLOCK
> /*
>  * Logical memory blocks.
> @@ -20,7 +22,6 @@
> #include <asm/memblock.h>
> 
> #define INIT_MEMBLOCK_REGIONS    128
> -#define MEMBLOCK_ERROR        0
> 
> struct memblock_region {
>     phys_addr_t base;
> @@ -160,6 +161,12 @@ static inline unsigned long memblock_reg
> #define __initdata_memblock
> #endif
> 
> +#else
> +static inline phys_addr_t memblock_alloc(phys_addr_t size, phys_addr_t 
> align)
> +{
> +    return MEMBLOCK_ERROR;
> +}
> +
> #endif /* CONFIG_HAVE_MEMBLOCK */
> 
> #endif /* __KERNEL__ */
> 
> 

Re: [PATCH 1/2] memblock: add error return when CONFIG_HAVE_MEMBLOCK is not set

[Ingo Molnar]

* Mike Travis <travis@sgi.com> wrote:

> Was there any further objections to these patches?

No big objections that i remember - so assuming review feedback has been 
addressed please send the latest and greatest in a new thread, this one is 
getting a bit deep :-)

Thanks,

	Ingo