linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] memblock, x86: fix big numa system booting
@ 2014-01-24 19:11 Yinghai Lu
  2014-01-24 19:11 ` [PATCH 2/3] x86: Revert wrong memblock current limit setting Yinghai Lu
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Yinghai Lu @ 2014-01-24 19:11 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Ingo Molnar, H. Peter Anvin
  Cc: Dave Hansen, Santosh Shilimkar, linux-kernel, Yinghai Lu

Big numa system boot get broken while switch API from bootmem to
memblock_virt.

Revert the offending patch, and also address swiotlb regression.

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 2/3] x86: Revert wrong memblock current limit setting.
  2014-01-24 19:11 [PATCH 0/3] memblock, x86: fix big numa system booting Yinghai Lu
@ 2014-01-24 19:11 ` Yinghai Lu
  2014-01-24 19:11 ` [PATCH 3/3] memblock: Don't align size silent in memblock_virt_alloc() Yinghai Lu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 25+ messages in thread
From: Yinghai Lu @ 2014-01-24 19:11 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Ingo Molnar, H. Peter Anvin
  Cc: Dave Hansen, Santosh Shilimkar, linux-kernel, Yinghai Lu

Dave reported big numa system booting is broken.

It turns out
| commit 5b6e529521d35e1bcaa0fe43456d1bbb335cae5d
| Author: Santosh Shilimkar <santosh.shilimkar@ti.com>
| Date:   Tue Jan 21 15:50:03 2014 -0800
|
|    x86: memblock: set current limit to max low memory address

set limit to low wrongly.

max_low_pfn_mapped is different from max_pfn_mapped.
max_low_pfn_mapped is always under 4G.

That will memblock_alloc_nid all go under 4G.

Revert that offending patch.

Reported-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/include/asm/page_types.h |    4 ++--
 arch/x86/kernel/setup.c           |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Index: linux-2.6/arch/x86/include/asm/page_types.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/page_types.h
+++ linux-2.6/arch/x86/include/asm/page_types.h
@@ -51,9 +51,9 @@ extern int devmem_is_allowed(unsigned lo
 extern unsigned long max_low_pfn_mapped;
 extern unsigned long max_pfn_mapped;
 
-static inline phys_addr_t get_max_low_mapped(void)
+static inline phys_addr_t get_max_mapped(void)
 {
-	return (phys_addr_t)max_low_pfn_mapped << PAGE_SHIFT;
+	return (phys_addr_t)max_pfn_mapped << PAGE_SHIFT;
 }
 
 bool pfn_range_is_mapped(unsigned long start_pfn, unsigned long end_pfn);
Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c
@@ -1173,7 +1173,7 @@ void __init setup_arch(char **cmdline_p)
 
 	setup_real_mode();
 
-	memblock_set_current_limit(get_max_low_mapped());
+	memblock_set_current_limit(get_max_mapped());
 	dma_contiguous_reserve(0);
 
 	/*

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 3/3] memblock: Don't align size silent in memblock_virt_alloc()
  2014-01-24 19:11 [PATCH 0/3] memblock, x86: fix big numa system booting Yinghai Lu
  2014-01-24 19:11 ` [PATCH 2/3] x86: Revert wrong memblock current limit setting Yinghai Lu
@ 2014-01-24 19:11 ` Yinghai Lu
  2014-01-24 19:11 ` [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low() Yinghai Lu
  2014-01-24 19:33 ` [PATCH 0/3] memblock, x86: fix big numa system booting Santosh Shilimkar
  3 siblings, 0 replies; 25+ messages in thread
From: Yinghai Lu @ 2014-01-24 19:11 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Ingo Molnar, H. Peter Anvin
  Cc: Dave Hansen, Santosh Shilimkar, linux-kernel, Yinghai Lu

In original __alloc_memory_core_early() for bootmem wrapper, we do not
align size silently.

We should not do that, as later free with old size will leave some
range not freed.

It's obvious that code is copied from memblock_base_nid(), and that code
is wrong for the same reason.

Also remove that in memblock_alloc_base.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 mm/memblock.c |    6 ------
 1 file changed, 6 deletions(-)

Index: linux-2.6/mm/memblock.c
===================================================================
--- linux-2.6.orig/mm/memblock.c
+++ linux-2.6/mm/memblock.c
@@ -981,9 +981,6 @@ static phys_addr_t __init memblock_alloc
 	if (!align)
 		align = SMP_CACHE_BYTES;
 
-	/* align @size to avoid excessive fragmentation on reserved array */
-	size = round_up(size, align);
-
 	found = memblock_find_in_range_node(size, align, 0, max_addr, nid);
 	if (found && !memblock_reserve(found, size))
 		return found;
@@ -1077,9 +1074,6 @@ static void * __init memblock_virt_alloc
 	if (!align)
 		align = SMP_CACHE_BYTES;
 
-	/* align @size to avoid excessive fragmentation on reserved array */
-	size = round_up(size, align);
-
 again:
 	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
 					    nid);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-24 19:11 [PATCH 0/3] memblock, x86: fix big numa system booting Yinghai Lu
  2014-01-24 19:11 ` [PATCH 2/3] x86: Revert wrong memblock current limit setting Yinghai Lu
  2014-01-24 19:11 ` [PATCH 3/3] memblock: Don't align size silent in memblock_virt_alloc() Yinghai Lu
@ 2014-01-24 19:11 ` Yinghai Lu
  2014-01-24 19:25   ` Konrad Rzeszutek Wilk
  2014-01-28  8:02   ` Olof Johansson
  2014-01-24 19:33 ` [PATCH 0/3] memblock, x86: fix big numa system booting Santosh Shilimkar
  3 siblings, 2 replies; 25+ messages in thread
From: Yinghai Lu @ 2014-01-24 19:11 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds, Ingo Molnar, H. Peter Anvin
  Cc: Dave Hansen, Santosh Shilimkar, linux-kernel, Yinghai Lu,
	Russell King, Konrad Rzeszutek Wilk

The new memblock_virt APIs are used to replaced old bootmem API.

We need to allocate page below 4G for swiotlb.

That should fix regression on Andrew's system that is using swiotlb.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
 arch/arm/kernel/setup.c |    2 +-
 include/linux/bootmem.h |   37 +++++++++++++++++++++++++++++++++++++
 lib/swiotlb.c           |    4 ++--
 3 files changed, 40 insertions(+), 3 deletions(-)

Index: linux-2.6/include/linux/bootmem.h
===================================================================
--- linux-2.6.orig/include/linux/bootmem.h
+++ linux-2.6/include/linux/bootmem.h
@@ -175,6 +175,27 @@ static inline void * __init memblock_vir
 						    NUMA_NO_NODE);
 }
 
+#ifndef ARCH_LOW_ADDRESS_LIMIT
+#define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
+#endif
+
+static inline void * __init memblock_virt_alloc_low(
+					phys_addr_t size, phys_addr_t align)
+{
+	return memblock_virt_alloc_try_nid(size, align,
+						   BOOTMEM_LOW_LIMIT,
+						   ARCH_LOW_ADDRESS_LIMIT,
+						   NUMA_NO_NODE);
+}
+static inline void * __init memblock_virt_alloc_low_nopanic(
+					phys_addr_t size, phys_addr_t align)
+{
+	return memblock_virt_alloc_try_nid_nopanic(size, align,
+						   BOOTMEM_LOW_LIMIT,
+						   ARCH_LOW_ADDRESS_LIMIT,
+						   NUMA_NO_NODE);
+}
+
 static inline void * __init memblock_virt_alloc_from_nopanic(
 		phys_addr_t size, phys_addr_t align, phys_addr_t min_addr)
 {
@@ -238,6 +259,22 @@ static inline void * __init memblock_vir
 	return __alloc_bootmem_nopanic(size, align, BOOTMEM_LOW_LIMIT);
 }
 
+static inline void * __init memblock_virt_alloc_low(
+					phys_addr_t size, phys_addr_t align)
+{
+	if (!align)
+		align = SMP_CACHE_BYTES;
+	return __alloc_bootmem_low(size, align, BOOTMEM_LOW_LIMIT);
+}
+
+static inline void * __init memblock_virt_alloc_low_nopanic(
+					phys_addr_t size, phys_addr_t align)
+{
+	if (!align)
+		align = SMP_CACHE_BYTES;
+	return __alloc_bootmem_low_nopanic(size, align, BOOTMEM_LOW_LIMIT);
+}
+
 static inline void * __init memblock_virt_alloc_from_nopanic(
 		phys_addr_t size, phys_addr_t align, phys_addr_t min_addr)
 {
Index: linux-2.6/lib/swiotlb.c
===================================================================
--- linux-2.6.orig/lib/swiotlb.c
+++ linux-2.6/lib/swiotlb.c
@@ -172,7 +172,7 @@ int __init swiotlb_init_with_tbl(char *t
 	/*
 	 * Get the overflow emergency buffer
 	 */
-	v_overflow_buffer = memblock_virt_alloc_nopanic(
+	v_overflow_buffer = memblock_virt_alloc_low_nopanic(
 						PAGE_ALIGN(io_tlb_overflow),
 						PAGE_SIZE);
 	if (!v_overflow_buffer)
@@ -220,7 +220,7 @@ swiotlb_init(int verbose)
 	bytes = io_tlb_nslabs << IO_TLB_SHIFT;
 
 	/* Get IO TLB memory from the low pages */
-	vstart = memblock_virt_alloc_nopanic(PAGE_ALIGN(bytes), PAGE_SIZE);
+	vstart = memblock_virt_alloc_low_nopanic(PAGE_ALIGN(bytes), PAGE_SIZE);
 	if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, verbose))
 		return;
 
Index: linux-2.6/arch/arm/kernel/setup.c
===================================================================
--- linux-2.6.orig/arch/arm/kernel/setup.c
+++ linux-2.6/arch/arm/kernel/setup.c
@@ -717,7 +717,7 @@ static void __init request_standard_reso
 	kernel_data.end     = virt_to_phys(_end - 1);
 
 	for_each_memblock(memory, region) {
-		res = memblock_virt_alloc(sizeof(*res), 0);
+		res = memblock_virt_alloc_low(sizeof(*res), 0);
 		res->name  = "System RAM";
 		res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
 		res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-24 19:11 ` [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low() Yinghai Lu
@ 2014-01-24 19:25   ` Konrad Rzeszutek Wilk
  2014-01-24 19:30     ` Santosh Shilimkar
  2014-01-24 19:34     ` Yinghai Lu
  2014-01-28  8:02   ` Olof Johansson
  1 sibling, 2 replies; 25+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-01-24 19:25 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Andrew Morton, Linus Torvalds, Ingo Molnar, H. Peter Anvin,
	Dave Hansen, Santosh Shilimkar, linux-kernel, Russell King

On Fri, Jan 24, 2014 at 11:11:10AM -0800, Yinghai Lu wrote:
> The new memblock_virt APIs are used to replaced old bootmem API.
> 
> We need to allocate page below 4G for swiotlb.
> 
> That should fix regression on Andrew's system that is using swiotlb.

Please include the title of the patch that caused the regression.
I presume it is "mm/lib/swiotlb: Use memblock apis for early memory allocations"

Interestingly enough when I asked about it:

https://lkml.org/lkml/2013/11/9/280


	>> v_overflow_buffer = memblock_virt_alloc_align_nopanic(
	>> +						PAGE_ALIGN(io_tlb_overflow),
	>> +						PAGE_SIZE);
	> 
	> Does this guarantee that the pages will be allocated below 4GB?
	> 
	Yes. The memblock layer still allocates memory from lowmem. As I
	mentioned, there is no change in the behavior than what is today
	apart from just the interface change.

How did that happend? Was there another patch in the series that altered
such assumption?

> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> ---
>  arch/arm/kernel/setup.c |    2 +-
>  include/linux/bootmem.h |   37 +++++++++++++++++++++++++++++++++++++
>  lib/swiotlb.c           |    4 ++--
>  3 files changed, 40 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/include/linux/bootmem.h
> ===================================================================
> --- linux-2.6.orig/include/linux/bootmem.h
> +++ linux-2.6/include/linux/bootmem.h
> @@ -175,6 +175,27 @@ static inline void * __init memblock_vir
>  						    NUMA_NO_NODE);
>  }
>  
> +#ifndef ARCH_LOW_ADDRESS_LIMIT
> +#define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
> +#endif
> +
> +static inline void * __init memblock_virt_alloc_low(
> +					phys_addr_t size, phys_addr_t align)
> +{
> +	return memblock_virt_alloc_try_nid(size, align,
> +						   BOOTMEM_LOW_LIMIT,
> +						   ARCH_LOW_ADDRESS_LIMIT,
> +						   NUMA_NO_NODE);
> +}
> +static inline void * __init memblock_virt_alloc_low_nopanic(
> +					phys_addr_t size, phys_addr_t align)
> +{
> +	return memblock_virt_alloc_try_nid_nopanic(size, align,
> +						   BOOTMEM_LOW_LIMIT,
> +						   ARCH_LOW_ADDRESS_LIMIT,
> +						   NUMA_NO_NODE);
> +}
> +
>  static inline void * __init memblock_virt_alloc_from_nopanic(
>  		phys_addr_t size, phys_addr_t align, phys_addr_t min_addr)
>  {
> @@ -238,6 +259,22 @@ static inline void * __init memblock_vir
>  	return __alloc_bootmem_nopanic(size, align, BOOTMEM_LOW_LIMIT);
>  }
>  
> +static inline void * __init memblock_virt_alloc_low(
> +					phys_addr_t size, phys_addr_t align)
> +{
> +	if (!align)
> +		align = SMP_CACHE_BYTES;
> +	return __alloc_bootmem_low(size, align, BOOTMEM_LOW_LIMIT);
> +}
> +
> +static inline void * __init memblock_virt_alloc_low_nopanic(
> +					phys_addr_t size, phys_addr_t align)
> +{
> +	if (!align)
> +		align = SMP_CACHE_BYTES;
> +	return __alloc_bootmem_low_nopanic(size, align, BOOTMEM_LOW_LIMIT);
> +}
> +
>  static inline void * __init memblock_virt_alloc_from_nopanic(
>  		phys_addr_t size, phys_addr_t align, phys_addr_t min_addr)
>  {
> Index: linux-2.6/lib/swiotlb.c
> ===================================================================
> --- linux-2.6.orig/lib/swiotlb.c
> +++ linux-2.6/lib/swiotlb.c
> @@ -172,7 +172,7 @@ int __init swiotlb_init_with_tbl(char *t
>  	/*
>  	 * Get the overflow emergency buffer
>  	 */
> -	v_overflow_buffer = memblock_virt_alloc_nopanic(
> +	v_overflow_buffer = memblock_virt_alloc_low_nopanic(
>  						PAGE_ALIGN(io_tlb_overflow),
>  						PAGE_SIZE);
>  	if (!v_overflow_buffer)
> @@ -220,7 +220,7 @@ swiotlb_init(int verbose)
>  	bytes = io_tlb_nslabs << IO_TLB_SHIFT;
>  
>  	/* Get IO TLB memory from the low pages */
> -	vstart = memblock_virt_alloc_nopanic(PAGE_ALIGN(bytes), PAGE_SIZE);
> +	vstart = memblock_virt_alloc_low_nopanic(PAGE_ALIGN(bytes), PAGE_SIZE);
>  	if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, verbose))
>  		return;
>  
> Index: linux-2.6/arch/arm/kernel/setup.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/kernel/setup.c
> +++ linux-2.6/arch/arm/kernel/setup.c
> @@ -717,7 +717,7 @@ static void __init request_standard_reso
>  	kernel_data.end     = virt_to_phys(_end - 1);
>  
>  	for_each_memblock(memory, region) {
> -		res = memblock_virt_alloc(sizeof(*res), 0);
> +		res = memblock_virt_alloc_low(sizeof(*res), 0);
>  		res->name  = "System RAM";
>  		res->start = __pfn_to_phys(memblock_region_memory_base_pfn(region));
>  		res->end = __pfn_to_phys(memblock_region_memory_end_pfn(region)) - 1;

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-24 19:25   ` Konrad Rzeszutek Wilk
@ 2014-01-24 19:30     ` Santosh Shilimkar
  2014-01-24 19:34     ` Yinghai Lu
  1 sibling, 0 replies; 25+ messages in thread
From: Santosh Shilimkar @ 2014-01-24 19:30 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Yinghai Lu, Andrew Morton, Linus Torvalds, Ingo Molnar,
	H. Peter Anvin, Dave Hansen, linux-kernel, Russell King

On Friday 24 January 2014 02:25 PM, Konrad Rzeszutek Wilk wrote:
> On Fri, Jan 24, 2014 at 11:11:10AM -0800, Yinghai Lu wrote:
>> The new memblock_virt APIs are used to replaced old bootmem API.
>>
>> We need to allocate page below 4G for swiotlb.
>>
>> That should fix regression on Andrew's system that is using swiotlb.
> 
> Please include the title of the patch that caused the regression.
> I presume it is "mm/lib/swiotlb: Use memblock apis for early memory allocations"
> 
> Interestingly enough when I asked about it:
> 
> https://lkml.org/lkml/2013/11/9/280
> 
> 
> 	>> v_overflow_buffer = memblock_virt_alloc_align_nopanic(
> 	>> +						PAGE_ALIGN(io_tlb_overflow),
> 	>> +						PAGE_SIZE);
> 	> 
> 	> Does this guarantee that the pages will be allocated below 4GB?
> 	> 
> 	Yes. The memblock layer still allocates memory from lowmem. As I
> 	mentioned, there is no change in the behavior than what is today
> 	apart from just the interface change.
> 
> How did that happend? Was there another patch in the series that altered
> such assumption?
> 
Actually it didn't. It was the misunderstanding on my side about the
low_mem_max_addr being under 4GB, which is not always true especially
for 64-bit systems which have no addressing limitations. 

Regards,
Santosh


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 0/3] memblock, x86: fix big numa system booting
  2014-01-24 19:11 [PATCH 0/3] memblock, x86: fix big numa system booting Yinghai Lu
                   ` (2 preceding siblings ...)
  2014-01-24 19:11 ` [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low() Yinghai Lu
@ 2014-01-24 19:33 ` Santosh Shilimkar
  2014-01-24 20:17   ` Andrew Morton
  3 siblings, 1 reply; 25+ messages in thread
From: Santosh Shilimkar @ 2014-01-24 19:33 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Andrew Morton, Linus Torvalds, Ingo Molnar, H. Peter Anvin,
	Dave Hansen, linux-kernel

Yinghai,

On Friday 24 January 2014 02:11 PM, Yinghai Lu wrote:
> Big numa system boot get broken while switch API from bootmem to
> memblock_virt.
> 
> Revert the offending patch, and also address swiotlb regression.
> 
Thanks a lot for fixes and help to narrow down and fix these
regressions. For all the patches in the series,

Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-24 19:25   ` Konrad Rzeszutek Wilk
  2014-01-24 19:30     ` Santosh Shilimkar
@ 2014-01-24 19:34     ` Yinghai Lu
  1 sibling, 0 replies; 25+ messages in thread
From: Yinghai Lu @ 2014-01-24 19:34 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Andrew Morton, Linus Torvalds, Ingo Molnar, H. Peter Anvin,
	Dave Hansen, Santosh Shilimkar, Linux Kernel Mailing List,
	Russell King

On Fri, Jan 24, 2014 at 11:25 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Fri, Jan 24, 2014 at 11:11:10AM -0800, Yinghai Lu wrote:
>> The new memblock_virt APIs are used to replaced old bootmem API.
>>
>> We need to allocate page below 4G for swiotlb.
>>
>> That should fix regression on Andrew's system that is using swiotlb.
>
> Please include the title of the patch that caused the regression.
> I presume it is "mm/lib/swiotlb: Use memblock apis for early memory allocations"
>
> Interestingly enough when I asked about it:
>
> https://lkml.org/lkml/2013/11/9/280
>
>
>         >> v_overflow_buffer = memblock_virt_alloc_align_nopanic(
>         >> +                                            PAGE_ALIGN(io_tlb_overflow),
>         >> +                                            PAGE_SIZE);
>         >
>         > Does this guarantee that the pages will be allocated below 4GB?
>         >
>         Yes. The memblock layer still allocates memory from lowmem. As I
>         mentioned, there is no change in the behavior than what is today
>         apart from just the interface change.
>
> How did that happend? Was there another patch in the series that altered
> such assumption?
>

Yes, that is one.

He chose to set memblock.current_limit to max_low_mapped. (that is under 4g).

but it broke big numa system as all boot mem is under 4G and system
with lots of memory will need to
have big chunk for vmemmap .

Before that patch, we need to add another API to make sure those
swiotlb under 4G.

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 0/3] memblock, x86: fix big numa system booting
  2014-01-24 19:33 ` [PATCH 0/3] memblock, x86: fix big numa system booting Santosh Shilimkar
@ 2014-01-24 20:17   ` Andrew Morton
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Morton @ 2014-01-24 20:17 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Yinghai Lu, Linus Torvalds, Ingo Molnar, H. Peter Anvin,
	Dave Hansen, linux-kernel

On Fri, 24 Jan 2014 14:33:19 -0500 Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:

> Yinghai,
> 
> On Friday 24 January 2014 02:11 PM, Yinghai Lu wrote:
> > Big numa system boot get broken while switch API from bootmem to
> > memblock_virt.
> > 
> > Revert the offending patch, and also address swiotlb regression.
> > 
> Thanks a lot for fixes and help to narrow down and fix these
> regressions. For all the patches in the series,
> 
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

That patchset continues to boot happily on my swiotlb test box.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-24 19:11 ` [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low() Yinghai Lu
  2014-01-24 19:25   ` Konrad Rzeszutek Wilk
@ 2014-01-28  8:02   ` Olof Johansson
  2014-01-28 15:30     ` Kevin Hilman
  1 sibling, 1 reply; 25+ messages in thread
From: Olof Johansson @ 2014-01-28  8:02 UTC (permalink / raw)
  To: Yinghai Lu, Linus Torvalds
  Cc: Andrew Morton, Ingo Molnar, H. Peter Anvin, Dave Hansen,
	Santosh Shilimkar, linux-kernel, Russell King,
	Konrad Rzeszutek Wilk

Hi,

On Fri, Jan 24, 2014 at 11:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> The new memblock_virt APIs are used to replaced old bootmem API.
>
> We need to allocate page below 4G for swiotlb.
>
> That should fix regression on Andrew's system that is using swiotlb.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

This seems to have been merged by Linus tonight as ad6492b80f, and it
had fallout on ARM systems (boot failures with no console output on
all but 5 of my machine/config combos).

Seems like it didn't have a chance to sit in -next, which is somewhat
understandable given that it's considered a bugfix and it indeed fixed
the bug it was meant to.

i'm out of time to debug this tonight (I noticed the failures as I was
heading to bed and figured I'd at least bisect them), so I wouldn't
mind seeing a revert of the ARM side change of ad6492b80f until it's
been sorted out so we keep bisectabilty intact for the rest of the
kernel.


Thanks,

-Olof

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28  8:02   ` Olof Johansson
@ 2014-01-28 15:30     ` Kevin Hilman
  2014-01-28 17:12       ` Yinghai Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Kevin Hilman @ 2014-01-28 15:30 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Yinghai Lu, Linus Torvalds, Andrew Morton, Ingo Molnar,
	H. Peter Anvin, Dave Hansen, Santosh Shilimkar, linux-kernel,
	Russell King, Konrad Rzeszutek Wilk

On Tue, Jan 28, 2014 at 12:02 AM, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
> On Fri, Jan 24, 2014 at 11:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> The new memblock_virt APIs are used to replaced old bootmem API.
>>
>> We need to allocate page below 4G for swiotlb.
>>
>> That should fix regression on Andrew's system that is using swiotlb.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> This seems to have been merged by Linus tonight as ad6492b80f, and it
> had fallout on ARM systems (boot failures with no console output on
> all but 5 of my machine/config combos).
>
> Seems like it didn't have a chance to sit in -next, which is somewhat
> understandable given that it's considered a bugfix and it indeed fixed
> the bug it was meant to.
>
> i'm out of time to debug this tonight (I noticed the failures as I was
> heading to bed and figured I'd at least bisect them), so I wouldn't
> mind seeing a revert of the ARM side change of ad6492b80f until it's
> been sorted out so we keep bisectabilty intact for the rest of the
> kernel.

Like Olof, I noticed multiple boot failures on various ARM boards.
I've confirmed that reverting the arch/arm part of this patch makes
them all happily booting again.

Kevin

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 15:30     ` Kevin Hilman
@ 2014-01-28 17:12       ` Yinghai Lu
  2014-01-28 17:18         ` Russell King - ARM Linux
  2014-01-28 17:23         ` Santosh Shilimkar
  0 siblings, 2 replies; 25+ messages in thread
From: Yinghai Lu @ 2014-01-28 17:12 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Olof Johansson, Linus Torvalds, Andrew Morton, Ingo Molnar,
	H. Peter Anvin, Dave Hansen, Santosh Shilimkar, linux-kernel,
	Russell King, Konrad Rzeszutek Wilk

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

On Tue, Jan 28, 2014 at 7:30 AM, Kevin Hilman <khilman@linaro.org> wrote:
> On Tue, Jan 28, 2014 at 12:02 AM, Olof Johansson <olof@lixom.net> wrote:
>> Hi,
>>
>> On Fri, Jan 24, 2014 at 11:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> The new memblock_virt APIs are used to replaced old bootmem API.
>>>
>>> We need to allocate page below 4G for swiotlb.
>>>
>>> That should fix regression on Andrew's system that is using swiotlb.
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> Cc: Russell King <linux@arm.linux.org.uk>
>>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>
>> This seems to have been merged by Linus tonight as ad6492b80f, and it
>> had fallout on ARM systems (boot failures with no console output on
>> all but 5 of my machine/config combos).
>>
>> Seems like it didn't have a chance to sit in -next, which is somewhat
>> understandable given that it's considered a bugfix and it indeed fixed
>> the bug it was meant to.
>>
>> i'm out of time to debug this tonight (I noticed the failures as I was
>> heading to bed and figured I'd at least bisect them), so I wouldn't
>> mind seeing a revert of the ARM side change of ad6492b80f until it's
>> been sorted out so we keep bisectabilty intact for the rest of the
>> kernel.
>
> Like Olof, I noticed multiple boot failures on various ARM boards.
> I've confirmed that reverting the arch/arm part of this patch makes
> them all happily booting again.

please try attached patch.

Index: linux-2.6/include/linux/bootmem.h
===================================================================
--- linux-2.6.orig/include/linux/bootmem.h
+++ linux-2.6/include/linux/bootmem.h
@@ -179,6 +179,9 @@ static inline void * __init memblock_vir
                                                    NUMA_NO_NODE);
 }

+/* Take arch's ARCH_LOW_ADDRESS_LIMIT at first*/
+#include <asm/processor.h>
+
 #ifndef ARCH_LOW_ADDRESS_LIMIT
 #define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
 #endif

[-- Attachment #2: fix_arm_low.patch --]
[-- Type: text/x-patch, Size: 522 bytes --]

---
 include/linux/bootmem.h |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-2.6/include/linux/bootmem.h
===================================================================
--- linux-2.6.orig/include/linux/bootmem.h
+++ linux-2.6/include/linux/bootmem.h
@@ -179,6 +179,9 @@ static inline void * __init memblock_vir
 						    NUMA_NO_NODE);
 }
 
+/* Take arch's ARCH_LOW_ADDRESS_LIMIT at first*/
+#include <asm/processor.h>
+
 #ifndef ARCH_LOW_ADDRESS_LIMIT
 #define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
 #endif

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 17:12       ` Yinghai Lu
@ 2014-01-28 17:18         ` Russell King - ARM Linux
  2014-01-28 17:23           ` Santosh Shilimkar
  2014-01-28 19:43           ` Yinghai Lu
  2014-01-28 17:23         ` Santosh Shilimkar
  1 sibling, 2 replies; 25+ messages in thread
From: Russell King - ARM Linux @ 2014-01-28 17:18 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Kevin Hilman, Olof Johansson, Linus Torvalds, Andrew Morton,
	Ingo Molnar, H. Peter Anvin, Dave Hansen, Santosh Shilimkar,
	linux-kernel, Konrad Rzeszutek Wilk

On Tue, Jan 28, 2014 at 09:12:27AM -0800, Yinghai Lu wrote:
> On Tue, Jan 28, 2014 at 7:30 AM, Kevin Hilman <khilman@linaro.org> wrote:
> > Like Olof, I noticed multiple boot failures on various ARM boards.
> > I've confirmed that reverting the arch/arm part of this patch makes
> > them all happily booting again.
> 
> please try attached patch.

Maybe I'm missing something, but there is no ARCH_LOW_ADDRESS_LIMIT
defined in the ARM header files, so I don't see how adding that
additional include changes anything.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 17:12       ` Yinghai Lu
  2014-01-28 17:18         ` Russell King - ARM Linux
@ 2014-01-28 17:23         ` Santosh Shilimkar
  2014-01-28 18:22           ` Russell King - ARM Linux
  1 sibling, 1 reply; 25+ messages in thread
From: Santosh Shilimkar @ 2014-01-28 17:23 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Kevin Hilman, Olof Johansson, Linus Torvalds, Andrew Morton,
	Ingo Molnar, H. Peter Anvin, Dave Hansen, linux-kernel,
	Russell King, Konrad Rzeszutek Wilk

On Tuesday 28 January 2014 12:12 PM, Yinghai Lu wrote:
> On Tue, Jan 28, 2014 at 7:30 AM, Kevin Hilman <khilman@linaro.org> wrote:
>> > On Tue, Jan 28, 2014 at 12:02 AM, Olof Johansson <olof@lixom.net> wrote:
>>> >> Hi,
>>> >>
>>> >> On Fri, Jan 24, 2014 at 11:11 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> >>> The new memblock_virt APIs are used to replaced old bootmem API.
>>>> >>>
>>>> >>> We need to allocate page below 4G for swiotlb.
>>>> >>>
>>>> >>> That should fix regression on Andrew's system that is using swiotlb.
>>>> >>>
>>>> >>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>> >>> Cc: Russell King <linux@arm.linux.org.uk>
>>>> >>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>> >>
>>> >> This seems to have been merged by Linus tonight as ad6492b80f, and it
>>> >> had fallout on ARM systems (boot failures with no console output on
>>> >> all but 5 of my machine/config combos).
>>> >>
>>> >> Seems like it didn't have a chance to sit in -next, which is somewhat
>>> >> understandable given that it's considered a bugfix and it indeed fixed
>>> >> the bug it was meant to.
>>> >>
>>> >> i'm out of time to debug this tonight (I noticed the failures as I was
>>> >> heading to bed and figured I'd at least bisect them), so I wouldn't
>>> >> mind seeing a revert of the ARM side change of ad6492b80f until it's
>>> >> been sorted out so we keep bisectabilty intact for the rest of the
>>> >> kernel.
>> >
>> > Like Olof, I noticed multiple boot failures on various ARM boards.
>> > I've confirmed that reverting the arch/arm part of this patch makes
>> > them all happily booting again.
> please try attached patch.
> 
> Index: linux-2.6/include/linux/bootmem.h
> ===================================================================
> --- linux-2.6.orig/include/linux/bootmem.h
> +++ linux-2.6/include/linux/bootmem.h
> @@ -179,6 +179,9 @@ static inline void * __init memblock_vir
>                                                     NUMA_NO_NODE);
>  }
> 
> +/* Take arch's ARCH_LOW_ADDRESS_LIMIT at first*/
> +#include <asm/processor.h>
> +
>  #ifndef ARCH_LOW_ADDRESS_LIMIT
>  #define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
>  #endif

This won't help mostly since the ARM 32 arch don't set ARCH_LOW_ADDRESS_LIMIT.
Sorry i couldn't respond to the thread earlier because of travel and
don't have access to my board to try out the patches.
The issue is mostly because on ARM the allocations needs to be limited lowmem and
that has been ensured using BOOTMEM_ALLOC_ACCESSIBLE in bootmem case and
MEMBLOCK_ALLOC_ACCESSIBLE in memblock case.

Even though ARM change done to use alloc_low() is correct, it has undesired
side effect of allocating memory from highmem because of non-use of
ARCH_LOW_ADDRESS_LIMIT. Setting ARCH_LOW_ADDRESS_LIMIT to MEMBLOCK_ALLOC_ACCESSIBLE
in ARM case also doesn't seem right.

Unless you have better idea, backing out the arch/arm change from the
subject patch seems to be most safe.

Regards,
Ssantosh






^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 17:18         ` Russell King - ARM Linux
@ 2014-01-28 17:23           ` Santosh Shilimkar
  2014-01-28 19:43           ` Yinghai Lu
  1 sibling, 0 replies; 25+ messages in thread
From: Santosh Shilimkar @ 2014-01-28 17:23 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Yinghai Lu, Kevin Hilman, Olof Johansson, Linus Torvalds,
	Andrew Morton, Ingo Molnar, H. Peter Anvin, Dave Hansen,
	linux-kernel, Konrad Rzeszutek Wilk

On Tuesday 28 January 2014 12:18 PM, Russell King - ARM Linux wrote:
> On Tue, Jan 28, 2014 at 09:12:27AM -0800, Yinghai Lu wrote:
>> On Tue, Jan 28, 2014 at 7:30 AM, Kevin Hilman <khilman@linaro.org> wrote:
>>> Like Olof, I noticed multiple boot failures on various ARM boards.
>>> I've confirmed that reverting the arch/arm part of this patch makes
>>> them all happily booting again.
>>
>> please try attached patch.
> 
> Maybe I'm missing something, but there is no ARCH_LOW_ADDRESS_LIMIT
> defined in the ARM header files, so I don't see how adding that
> additional include changes anything.
> 
Our emails crossed... You are right. It won't help.

Regards,
Santosh

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 17:23         ` Santosh Shilimkar
@ 2014-01-28 18:22           ` Russell King - ARM Linux
  2014-01-28 18:36             ` Santosh Shilimkar
  2014-01-28 20:17             ` Linus Torvalds
  0 siblings, 2 replies; 25+ messages in thread
From: Russell King - ARM Linux @ 2014-01-28 18:22 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Yinghai Lu, Kevin Hilman, Olof Johansson, Linus Torvalds,
	Andrew Morton, Ingo Molnar, H. Peter Anvin, Dave Hansen,
	linux-kernel, Konrad Rzeszutek Wilk

On Tue, Jan 28, 2014 at 12:23:02PM -0500, Santosh Shilimkar wrote:
> On Tuesday 28 January 2014 12:12 PM, Yinghai Lu wrote:
> > Index: linux-2.6/include/linux/bootmem.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/bootmem.h
> > +++ linux-2.6/include/linux/bootmem.h
> > @@ -179,6 +179,9 @@ static inline void * __init memblock_vir
> >                                                     NUMA_NO_NODE);
> >  }
> > 
> > +/* Take arch's ARCH_LOW_ADDRESS_LIMIT at first*/
> > +#include <asm/processor.h>
> > +
> >  #ifndef ARCH_LOW_ADDRESS_LIMIT
> >  #define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
> >  #endif
> 
> This won't help mostly since the ARM 32 arch don't set ARCH_LOW_ADDRESS_LIMIT.
> Sorry i couldn't respond to the thread earlier because of travel and
> don't have access to my board to try out the patches.

Let's think about this for a moment, shall we...

What does memblock_alloc_virt*() return?  It returns a virtual address.

How is that virtual address obtained?  ptr = phys_to_virt(alloc);

What is the valid address range for passing into phys_to_virt() ?  Only
lowmem addresses.

Hence, having ARCH_LOW_ADDRESS_LIMIT set to 4GB-1 by default seems to be
completely rediculous - and presumably this also fails on x86_32 if it
returns memory up at 4GB.

So... yes, I think reverting the arch/arm part of this patch is the right
solution, whether the rest of it should be reverted is something I can't
comment on.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 18:22           ` Russell King - ARM Linux
@ 2014-01-28 18:36             ` Santosh Shilimkar
  2014-01-28 18:56               ` Konrad Rzeszutek Wilk
  2014-01-28 20:17             ` Linus Torvalds
  1 sibling, 1 reply; 25+ messages in thread
From: Santosh Shilimkar @ 2014-01-28 18:36 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Yinghai Lu, Kevin Hilman, Olof Johansson, Linus Torvalds,
	Andrew Morton, Ingo Molnar, H. Peter Anvin, Dave Hansen,
	linux-kernel, Konrad Rzeszutek Wilk, Strashko, Grygorii

+ Gryagorii,
On Tuesday 28 January 2014 01:22 PM, Russell King - ARM Linux wrote:
> On Tue, Jan 28, 2014 at 12:23:02PM -0500, Santosh Shilimkar wrote:
>> On Tuesday 28 January 2014 12:12 PM, Yinghai Lu wrote:
>>> Index: linux-2.6/include/linux/bootmem.h
>>> ===================================================================
>>> --- linux-2.6.orig/include/linux/bootmem.h
>>> +++ linux-2.6/include/linux/bootmem.h
>>> @@ -179,6 +179,9 @@ static inline void * __init memblock_vir
>>>                                                     NUMA_NO_NODE);
>>>  }
>>>
>>> +/* Take arch's ARCH_LOW_ADDRESS_LIMIT at first*/
>>> +#include <asm/processor.h>
>>> +
>>>  #ifndef ARCH_LOW_ADDRESS_LIMIT
>>>  #define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
>>>  #endif
>>
>> This won't help mostly since the ARM 32 arch don't set ARCH_LOW_ADDRESS_LIMIT.
>> Sorry i couldn't respond to the thread earlier because of travel and
>> don't have access to my board to try out the patches.
> 
> Let's think about this for a moment, shall we...
> 
> What does memblock_alloc_virt*() return?  It returns a virtual address.
> 
> How is that virtual address obtained?  ptr = phys_to_virt(alloc);
> 
> What is the valid address range for passing into phys_to_virt() ?  Only
> lowmem addresses.
> 
> Hence, having ARCH_LOW_ADDRESS_LIMIT set to 4GB-1 by default seems to be
> completely rediculous - and presumably this also fails on x86_32 if it
> returns memory up at 4GB.
> 
> So... yes, I think reverting the arch/arm part of this patch is the right
> solution, whether the rest of it should be reverted is something I can't
> comment on.
> 
Grygorri mentioned an alternate to update the memblock_find_in_range_node() so
that it takes into account the limit.

Regards,
Santosh 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 18:36             ` Santosh Shilimkar
@ 2014-01-28 18:56               ` Konrad Rzeszutek Wilk
  2014-01-28 20:16                 ` Strashko, Grygorii
  0 siblings, 1 reply; 25+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-01-28 18:56 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Russell King - ARM Linux, Yinghai Lu, Kevin Hilman,
	Olof Johansson, Linus Torvalds, Andrew Morton, Ingo Molnar,
	H. Peter Anvin, Dave Hansen, linux-kernel, Strashko, Grygorii,
	xen-devel

On Tue, Jan 28, 2014 at 01:36:28PM -0500, Santosh Shilimkar wrote:
> + Gryagorii,
> On Tuesday 28 January 2014 01:22 PM, Russell King - ARM Linux wrote:
> > On Tue, Jan 28, 2014 at 12:23:02PM -0500, Santosh Shilimkar wrote:
> >> On Tuesday 28 January 2014 12:12 PM, Yinghai Lu wrote:
> >>> Index: linux-2.6/include/linux/bootmem.h
> >>> ===================================================================
> >>> --- linux-2.6.orig/include/linux/bootmem.h
> >>> +++ linux-2.6/include/linux/bootmem.h
> >>> @@ -179,6 +179,9 @@ static inline void * __init memblock_vir
> >>>                                                     NUMA_NO_NODE);
> >>>  }
> >>>
> >>> +/* Take arch's ARCH_LOW_ADDRESS_LIMIT at first*/
> >>> +#include <asm/processor.h>
> >>> +
> >>>  #ifndef ARCH_LOW_ADDRESS_LIMIT
> >>>  #define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
> >>>  #endif
> >>
> >> This won't help mostly since the ARM 32 arch don't set ARCH_LOW_ADDRESS_LIMIT.
> >> Sorry i couldn't respond to the thread earlier because of travel and
> >> don't have access to my board to try out the patches.
> > 
> > Let's think about this for a moment, shall we...
> > 
> > What does memblock_alloc_virt*() return?  It returns a virtual address.
> > 
> > How is that virtual address obtained?  ptr = phys_to_virt(alloc);
> > 
> > What is the valid address range for passing into phys_to_virt() ?  Only
> > lowmem addresses.
> > 
> > Hence, having ARCH_LOW_ADDRESS_LIMIT set to 4GB-1 by default seems to be
> > completely rediculous - and presumably this also fails on x86_32 if it
> > returns memory up at 4GB.
> > 
> > So... yes, I think reverting the arch/arm part of this patch is the right
> > solution, whether the rest of it should be reverted is something I can't
> > comment on.
> > 
> Grygorri mentioned an alternate to update the memblock_find_in_range_node() so
> that it takes into account the limit.

This patch breaks also Xen and 32-bit guests (see
http://lists.xen.org/archives/html/xen-devel/2014-01/msg02476.html)

Reverting it fixes it.

> 
> Regards,
> Santosh 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 17:18         ` Russell King - ARM Linux
  2014-01-28 17:23           ` Santosh Shilimkar
@ 2014-01-28 19:43           ` Yinghai Lu
  2014-01-28 19:47             ` Russell King - ARM Linux
  1 sibling, 1 reply; 25+ messages in thread
From: Yinghai Lu @ 2014-01-28 19:43 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Kevin Hilman, Olof Johansson, Linus Torvalds, Andrew Morton,
	Ingo Molnar, H. Peter Anvin, Dave Hansen, Santosh Shilimkar,
	linux-kernel, Konrad Rzeszutek Wilk

On Tue, Jan 28, 2014 at 9:18 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Jan 28, 2014 at 09:12:27AM -0800, Yinghai Lu wrote:
>> On Tue, Jan 28, 2014 at 7:30 AM, Kevin Hilman <khilman@linaro.org> wrote:
>> > Like Olof, I noticed multiple boot failures on various ARM boards.
>> > I've confirmed that reverting the arch/arm part of this patch makes
>> > them all happily booting again.
>>
>> please try attached patch.
>
> Maybe I'm missing something, but there is no ARCH_LOW_ADDRESS_LIMIT
> defined in the ARM header files, so I don't see how adding that
> additional include changes anything.

there is one for arm64 and s390.

arch/arm64/include/asm/processor.h:#define ARCH_LOW_ADDRESS_LIMIT       PHYS_MAS
arch/s390/include/asm/processor.h:#define ARCH_LOW_ADDRESS_LIMIT        0x7fffff

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 19:43           ` Yinghai Lu
@ 2014-01-28 19:47             ` Russell King - ARM Linux
  2014-01-28 19:55               ` Yinghai Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Russell King - ARM Linux @ 2014-01-28 19:47 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Kevin Hilman, Olof Johansson, Linus Torvalds, Andrew Morton,
	Ingo Molnar, H. Peter Anvin, Dave Hansen, Santosh Shilimkar,
	linux-kernel, Konrad Rzeszutek Wilk

On Tue, Jan 28, 2014 at 11:43:02AM -0800, Yinghai Lu wrote:
> On Tue, Jan 28, 2014 at 9:18 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Jan 28, 2014 at 09:12:27AM -0800, Yinghai Lu wrote:
> >> On Tue, Jan 28, 2014 at 7:30 AM, Kevin Hilman <khilman@linaro.org> wrote:
> >> > Like Olof, I noticed multiple boot failures on various ARM boards.
> >> > I've confirmed that reverting the arch/arm part of this patch makes
> >> > them all happily booting again.
> >>
> >> please try attached patch.
> >
> > Maybe I'm missing something, but there is no ARCH_LOW_ADDRESS_LIMIT
> > defined in the ARM header files, so I don't see how adding that
> > additional include changes anything.
> 
> there is one for arm64 and s390.
> 
> arch/arm64/include/asm/processor.h:#define ARCH_LOW_ADDRESS_LIMIT       PHYS_MAS
> arch/s390/include/asm/processor.h:#define ARCH_LOW_ADDRESS_LIMIT        0x7fffff

Forgive me being difficult, but how exactly does your patch come anywhere
close to sortting out the reported regression by Kevin and Olof, and now
myself which is on ARM?

Your patch adds asm/processor.h to a header file, to allow architectures
to override ARCH_LOW_ADDRESS_LIMIT, but there is /no/ override for ARM,
so your patch has _zero_ effect there - we still end up with this being
0xffffffff, and we still end up with the thing being unable to boot.

Maybe you could describe what this ARCH_LOW_ADDRESS_LIMIT is, and what
it should be set to - I'm less than clear on how to set this given that
we have a multitude of different physical memory layouts - where memory
can start at almost any physical address.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 19:47             ` Russell King - ARM Linux
@ 2014-01-28 19:55               ` Yinghai Lu
  2014-01-28 20:17                 ` Yinghai Lu
  0 siblings, 1 reply; 25+ messages in thread
From: Yinghai Lu @ 2014-01-28 19:55 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Kevin Hilman, Olof Johansson, Linus Torvalds, Andrew Morton,
	Ingo Molnar, H. Peter Anvin, Dave Hansen, Santosh Shilimkar,
	linux-kernel, Konrad Rzeszutek Wilk

On Tue, Jan 28, 2014 at 11:47 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Jan 28, 2014 at 11:43:02AM -0800, Yinghai Lu wrote:
>> On Tue, Jan 28, 2014 at 9:18 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Tue, Jan 28, 2014 at 09:12:27AM -0800, Yinghai Lu wrote:
>> >> On Tue, Jan 28, 2014 at 7:30 AM, Kevin Hilman <khilman@linaro.org> wrote:
>> >> > Like Olof, I noticed multiple boot failures on various ARM boards.
>> >> > I've confirmed that reverting the arch/arm part of this patch makes
>> >> > them all happily booting again.
>> >>
>> >> please try attached patch.
>> >
>> > Maybe I'm missing something, but there is no ARCH_LOW_ADDRESS_LIMIT
>> > defined in the ARM header files, so I don't see how adding that
>> > additional include changes anything.
>>
>> there is one for arm64 and s390.
>>
>> arch/arm64/include/asm/processor.h:#define ARCH_LOW_ADDRESS_LIMIT       PHYS_MAS
>> arch/s390/include/asm/processor.h:#define ARCH_LOW_ADDRESS_LIMIT        0x7fffff
>
> Forgive me being difficult, but how exactly does your patch come anywhere
> close to sortting out the reported regression by Kevin and Olof, and now
> myself which is on ARM?
>
> Your patch adds asm/processor.h to a header file, to allow architectures
> to override ARCH_LOW_ADDRESS_LIMIT, but there is /no/ override for ARM,
> so your patch has _zero_ effect there - we still end up with this being
> 0xffffffff, and we still end up with the thing being unable to boot.
>
> Maybe you could describe what this ARCH_LOW_ADDRESS_LIMIT is, and what
> it should be set to - I'm less than clear on how to set this given that
> we have a multitude of different physical memory layouts - where memory
> can start at almost any physical address.

well, I just want to restore the alloc_bootmem_low by following patch.

commit 9233d2be108f573caa21eb450411bf8fa68cadbb
Author: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date:   Tue Jan 21 15:50:47 2014 -0800

    arch/arm/kernel/: use memblock apis for early memory allocations

    Switch to memblock interfaces for early memory allocator instead of
    bootmem allocator.  No functional change in beahvior than what it is in
    current code from bootmem users points of view.

    Archs already converted to NO_BOOTMEM now directly use memblock
    interfaces instead of bootmem wrappers build on top of memblock.  And
    the archs which still uses bootmem, these new apis just fallback to
    exiting bootmem APIs.

    Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
...
@@ -717,7 +717,7 @@ static void __init
request_standard_resources(const struct machine_desc *mdesc)
        kernel_data.end     = virt_to_phys(_end - 1);

        for_each_memblock(memory, region) {
-               res = alloc_bootmem_low(sizeof(*res));
+               res = memblock_virt_alloc(sizeof(*res), 0);
                res->name  = "System RAM";

but looks like memblock_virt_alloc_low is not the same as alloc_bootmem_low yet.

Let's me check further.

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 18:56               ` Konrad Rzeszutek Wilk
@ 2014-01-28 20:16                 ` Strashko, Grygorii
  0 siblings, 0 replies; 25+ messages in thread
From: Strashko, Grygorii @ 2014-01-28 20:16 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Shilimkar, Santosh
  Cc: Russell King - ARM Linux, Yinghai Lu, Kevin Hilman,
	Olof Johansson, Linus Torvalds, Andrew Morton, Ingo Molnar,
	H. Peter Anvin, Dave Hansen, linux-kernel, xen-devel

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

Hi all,

Sorry, for the invalid mail & patch format - have no way to send it properly now.

Suppose there is another way to fix this issue (more generic)
- Correct memblock_virt_allocX() API to limit allocations below memblock.current_limit
(patch attached).

Then the code behavior will become more similar to _alloc_memory_core_early.

Not tested.


Best regards,
- grygorii

________________________________________
From: Konrad Rzeszutek Wilk [konrad.wilk@oracle.com]
Sent: Tuesday, January 28, 2014 8:56 PM
To: Shilimkar, Santosh
Cc: Russell King - ARM Linux; Yinghai Lu; Kevin Hilman; Olof Johansson; Linus Torvalds; Andrew Morton; Ingo Molnar; H. Peter Anvin; Dave Hansen; linux-kernel@vger.kernel.org; Strashko, Grygorii; xen-devel@lists.xensource.com
Subject: Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()

On Tue, Jan 28, 2014 at 01:36:28PM -0500, Santosh Shilimkar wrote:
> + Gryagorii,
> On Tuesday 28 January 2014 01:22 PM, Russell King - ARM Linux wrote:
> > On Tue, Jan 28, 2014 at 12:23:02PM -0500, Santosh Shilimkar wrote:
> >> On Tuesday 28 January 2014 12:12 PM, Yinghai Lu wrote:
> >>> Index: linux-2.6/include/linux/bootmem.h
> >>> ===================================================================
> >>> --- linux-2.6.orig/include/linux/bootmem.h
> >>> +++ linux-2.6/include/linux/bootmem.h
> >>> @@ -179,6 +179,9 @@ static inline void * __init memblock_vir
> >>>                                                     NUMA_NO_NODE);
> >>>  }
> >>>
> >>> +/* Take arch's ARCH_LOW_ADDRESS_LIMIT at first*/
> >>> +#include <asm/processor.h>
> >>> +
> >>>  #ifndef ARCH_LOW_ADDRESS_LIMIT
> >>>  #define ARCH_LOW_ADDRESS_LIMIT  0xffffffffUL
> >>>  #endif
> >>
> >> This won't help mostly since the ARM 32 arch don't set ARCH_LOW_ADDRESS_LIMIT.
> >> Sorry i couldn't respond to the thread earlier because of travel and
> >> don't have access to my board to try out the patches.
> >
> > Let's think about this for a moment, shall we...
> >
> > What does memblock_alloc_virt*() return?  It returns a virtual address.
> >
> > How is that virtual address obtained?  ptr = phys_to_virt(alloc);
> >
> > What is the valid address range for passing into phys_to_virt() ?  Only
> > lowmem addresses.
> >
> > Hence, having ARCH_LOW_ADDRESS_LIMIT set to 4GB-1 by default seems to be
> > completely rediculous - and presumably this also fails on x86_32 if it
> > returns memory up at 4GB.
> >
> > So... yes, I think reverting the arch/arm part of this patch is the right
> > solution, whether the rest of it should be reverted is something I can't
> > comment on.
> >
> Grygorri mentioned an alternate to update the memblock_find_in_range_node() so
> that it takes into account the limit.

This patch breaks also Xen and 32-bit guests (see
http://lists.xen.org/archives/html/xen-devel/2014-01/msg02476.html)

Reverting it fixes it.

>
> Regards,
> Santosh

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-mm-memblock-fix-upper-boundary-of-allocating-region.patch --]
[-- Type: text/x-patch; name="0001-mm-memblock-fix-upper-boundary-of-allocating-region.patch", Size: 901 bytes --]

From ee31afb9c5c0e78819ce624e3a930d31b97527cd Mon Sep 17 00:00:00 2001
From: grygoriis <grygorii.strashko@globallogic.com>
Date: Tue, 28 Jan 2014 21:59:30 +0200
Subject: [PATCH] mm/memblock: fix upper boundary of allocating region

Correct memblock_virt_allocX() API to limit allocations below
memblock.current_limit.

Signed-off-by: grygoriis <grygorii.strashko@globallogic.com>
---
 mm/memblock.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index 87d21a6..e93d669 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -1077,6 +1077,9 @@ static void * __init memblock_virt_alloc_internal(
 	if (!align)
 		align = SMP_CACHE_BYTES;
 
+        if (max_addr > memblock.current_limit)
+                max_addr = memblock.current_limit;
+
 again:
 	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
 					    nid);
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 19:55               ` Yinghai Lu
@ 2014-01-28 20:17                 ` Yinghai Lu
  0 siblings, 0 replies; 25+ messages in thread
From: Yinghai Lu @ 2014-01-28 20:17 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Kevin Hilman, Olof Johansson, Linus Torvalds, Andrew Morton,
	Ingo Molnar, H. Peter Anvin, Dave Hansen, Santosh Shilimkar,
	linux-kernel, Konrad Rzeszutek Wilk

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

On Tue, Jan 28, 2014 at 11:55 AM, Yinghai Lu <yinghai@kernel.org> wrote:

> but looks like memblock_virt_alloc_low is not the same as alloc_bootmem_low yet.
>

The memblock_virt_alloc missed limit checking.

Please check attached patch.

Thanks

Yinghai

[-- Attachment #2: fix_memblock_virt_alloc_low.patch --]
[-- Type: text/x-patch, Size: 762 bytes --]

Subject: [PATCH] memblock: Add limit checking to memblock_virt_alloc

In original bootmem wrapper for memblock, we have limit checking.

Add it to memblock_virt_alloc, to address arm and x86 booting crash.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 mm/memblock.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-2.6/mm/memblock.c
===================================================================
--- linux-2.6.orig/mm/memblock.c
+++ linux-2.6/mm/memblock.c
@@ -1077,6 +1077,9 @@ static void * __init memblock_virt_alloc
 	if (!align)
 		align = SMP_CACHE_BYTES;
 
+	if (max_addr > memblock.current_limit)
+		max_addr = memblock.current_limit;
+
 again:
 	alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
 					    nid);

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 18:22           ` Russell King - ARM Linux
  2014-01-28 18:36             ` Santosh Shilimkar
@ 2014-01-28 20:17             ` Linus Torvalds
  2014-01-28 20:20               ` Yinghai Lu
  1 sibling, 1 reply; 25+ messages in thread
From: Linus Torvalds @ 2014-01-28 20:17 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Santosh Shilimkar, Yinghai Lu, Kevin Hilman, Olof Johansson,
	Andrew Morton, Ingo Molnar, H. Peter Anvin, Dave Hansen,
	linux-kernel, Konrad Rzeszutek Wilk

On Tue, Jan 28, 2014 at 10:22 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
>
> Hence, having ARCH_LOW_ADDRESS_LIMIT set to 4GB-1 by default seems to be
> completely rediculous - and presumably this also fails on x86_32 if it
> returns memory up at 4GB.

Agreed. That looks broken even on x86-32. The low address limit is not
even *close* to 4GB in general on 32-bit, since you not only have the
TASK_SIZE, you have the kmap and the vmalloc area. On x86-32,
ARCH_LOW_ADDRESS_LIMIT should be MAXMEM, which iirc is somewhere
around 890MB or so. Not 4G.

                    Linus

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low()
  2014-01-28 20:17             ` Linus Torvalds
@ 2014-01-28 20:20               ` Yinghai Lu
  0 siblings, 0 replies; 25+ messages in thread
From: Yinghai Lu @ 2014-01-28 20:20 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Russell King - ARM Linux, Santosh Shilimkar, Kevin Hilman,
	Olof Johansson, Andrew Morton, Ingo Molnar, H. Peter Anvin,
	Dave Hansen, linux-kernel, Konrad Rzeszutek Wilk

On Tue, Jan 28, 2014 at 12:17 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Jan 28, 2014 at 10:22 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>>
>> Hence, having ARCH_LOW_ADDRESS_LIMIT set to 4GB-1 by default seems to be
>> completely rediculous - and presumably this also fails on x86_32 if it
>> returns memory up at 4GB.
>
> Agreed. That looks broken even on x86-32. The low address limit is not
> even *close* to 4GB in general on 32-bit, since you not only have the
> TASK_SIZE, you have the kmap and the vmalloc area. On x86-32,
> ARCH_LOW_ADDRESS_LIMIT should be MAXMEM, which iirc is somewhere
> around 890MB or so. Not 4G.
>

yeah, Please check the patch that one minute ago.

Subject: [PATCH] memblock: Add limit checking to memblock_virt_alloc

In original bootmem wrapper for memblock, we have limit checking.

Add it to memblock_virt_alloc, to address arm and x86 booting crash.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 mm/memblock.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux-2.6/mm/memblock.c
===================================================================
--- linux-2.6.orig/mm/memblock.c
+++ linux-2.6/mm/memblock.c
@@ -1077,6 +1077,9 @@ static void * __init memblock_virt_alloc
     if (!align)
         align = SMP_CACHE_BYTES;

+    if (max_addr > memblock.current_limit)
+        max_addr = memblock.current_limit;
+
 again:
     alloc = memblock_find_in_range_node(size, align, min_addr, max_addr,
                         nid);

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2014-01-28 20:20 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-24 19:11 [PATCH 0/3] memblock, x86: fix big numa system booting Yinghai Lu
2014-01-24 19:11 ` [PATCH 2/3] x86: Revert wrong memblock current limit setting Yinghai Lu
2014-01-24 19:11 ` [PATCH 3/3] memblock: Don't align size silent in memblock_virt_alloc() Yinghai Lu
2014-01-24 19:11 ` [PATCH 1/3] memblock, nobootmem: Add memblock_virt_alloc_low() Yinghai Lu
2014-01-24 19:25   ` Konrad Rzeszutek Wilk
2014-01-24 19:30     ` Santosh Shilimkar
2014-01-24 19:34     ` Yinghai Lu
2014-01-28  8:02   ` Olof Johansson
2014-01-28 15:30     ` Kevin Hilman
2014-01-28 17:12       ` Yinghai Lu
2014-01-28 17:18         ` Russell King - ARM Linux
2014-01-28 17:23           ` Santosh Shilimkar
2014-01-28 19:43           ` Yinghai Lu
2014-01-28 19:47             ` Russell King - ARM Linux
2014-01-28 19:55               ` Yinghai Lu
2014-01-28 20:17                 ` Yinghai Lu
2014-01-28 17:23         ` Santosh Shilimkar
2014-01-28 18:22           ` Russell King - ARM Linux
2014-01-28 18:36             ` Santosh Shilimkar
2014-01-28 18:56               ` Konrad Rzeszutek Wilk
2014-01-28 20:16                 ` Strashko, Grygorii
2014-01-28 20:17             ` Linus Torvalds
2014-01-28 20:20               ` Yinghai Lu
2014-01-24 19:33 ` [PATCH 0/3] memblock, x86: fix big numa system booting Santosh Shilimkar
2014-01-24 20:17   ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).