From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752234AbdKTSvm (ORCPT ); Mon, 20 Nov 2017 13:51:42 -0500 Received: from mail-pg0-f68.google.com ([74.125.83.68]:33597 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751637AbdKTSvl (ORCPT ); Mon, 20 Nov 2017 13:51:41 -0500 X-Google-Smtp-Source: AGs4zMbzqyZA3niz8nN3HbcgwawTR5wIJMhlajo81cJ4QXvO7dNFezxmmsjLwJUJ8mp7w8w11EM6dg== Date: Mon, 20 Nov 2017 10:51:38 -0800 From: Guenter Roeck To: Nicolas Pitre Cc: Tejun Heo , Christoph Lameter , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Mikael Starvik , Jesper Nilsson , linux-cris-kernel@axis.com Subject: Re: mm/percpu.c: use smarter memory allocation for struct pcpu_alloc_info (crisv32 hang) Message-ID: <20171120185138.GB23789@roeck-us.net> References: <20171118182542.GA23928@roeck-us.net> <62a3b680-6dde-d308-3da8-9c9a2789b114@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 20, 2017 at 01:18:38PM -0500, Nicolas Pitre wrote: > On Sun, 19 Nov 2017, Guenter Roeck wrote: > > > On 11/19/2017 08:08 PM, Nicolas Pitre wrote: > > > On Sun, 19 Nov 2017, Guenter Roeck wrote: > > > > On 11/19/2017 12:36 PM, Nicolas Pitre wrote: > > > > > On Sat, 18 Nov 2017, Guenter Roeck wrote: > > > > > > On Tue, Oct 03, 2017 at 06:29:49PM -0400, Nicolas Pitre wrote: > > > > > > > @@ -2295,6 +2295,7 @@ void __init setup_per_cpu_areas(void) > > > > > > > if (pcpu_setup_first_chunk(ai, fc) < 0) > > > > > > > panic("Failed to initialize percpu areas."); > > > > > > > + pcpu_free_alloc_info(ai); > > > > > > > > > > > > This is the culprit. Everything works fine if I remove this line. > > > > > > > > > > Without this line, the memory at the ai pointer is leaked. Maybe this is > > > > > modifying the memory allocation pattern and that triggers a bug later on > > > > > in your case. > > > > > > > > > > At that point the console driver is not yet initialized and any error > > > > > message won't be printed. You should enable the early console mechanism > > > > > in your kernel (see arch/cris/arch-v32/kernel/debugport.c) and see what > > > > > that might tell you. > > > > > > > > > > > > > The problem is that BUG() on crisv32 does not yield useful output. > > > > Anyway, here is the culprit. > > > > > > > > diff --git a/mm/bootmem.c b/mm/bootmem.c > > > > index 6aef64254203..2bcc8901450c 100644 > > > > --- a/mm/bootmem.c > > > > +++ b/mm/bootmem.c > > > > @@ -382,7 +382,8 @@ static int __init mark_bootmem(unsigned long start, > > > > unsigned long end, > > > > return 0; > > > > pos = bdata->node_low_pfn; > > > > } > > > > - BUG(); > > > > + WARN(1, "mark_bootmem(): memory range 0x%lx-0x%lx not found\n", > > > > start, > > > > end); > > > > + return -ENOMEM; > > > > } > > > > > > > > /** > > > > diff --git a/mm/percpu.c b/mm/percpu.c > > > > index 79e3549cab0f..c75622d844f1 100644 > > > > --- a/mm/percpu.c > > > > +++ b/mm/percpu.c > > > > @@ -1881,6 +1881,7 @@ struct pcpu_alloc_info * __init > > > > pcpu_alloc_alloc_info(int nr_groups, > > > > */ > > > > void __init pcpu_free_alloc_info(struct pcpu_alloc_info *ai) > > > > { > > > > + printk("pcpu_free_alloc_info(%p (0x%lx))\n", ai, __pa(ai)); > > > > memblock_free_early(__pa(ai), ai->__ai_size); > > > > > > The problem here is that there is two possibilities for > > > memblock_free_early(). From include/linux/bootmem.h: > > > > > > #if defined(CONFIG_HAVE_MEMBLOCK) && defined(CONFIG_NO_BOOTMEM) > > > > > > static inline void __init memblock_free_early( > > > phys_addr_t base, phys_addr_t size) > > > { > > > __memblock_free_early(base, size); > > > } > > > > > > #else > > > > > > static inline void __init memblock_free_early( > > > phys_addr_t base, phys_addr_t size) > > > { > > > free_bootmem(base, size); > > > } > > > > > > #endif > > > > > > It looks like most architectures use the memblock variant, including all > > > the ones I have access to. > > > > > > > results in: > > > > > > > > pcpu_free_alloc_info(c0534000 (0x40534000)) > > > > ------------[ cut here ]------------ > > > > WARNING: CPU: 0 PID: 0 at mm/bootmem.c:385 mark_bootmem+0x9a/0xaa > > > > mark_bootmem(): memory range 0x2029a-0x2029b not found > > > > > > Well... PFN_UP(0x40534000) should give 0x40534. How you might end up > > > with 0x2029a in mark_bootmem(), let alone not exit on the first "if (max > > > == end) return 0;" within the loop is rather weird. > > > > > pcpu_free_alloc_info: ai=c0536000, __pa(ai)=0x40536000, > > PFN_UP(__pa(ai))=0x2029b, PFN_UP(ai)=0x6029b > > > > bootmem range is 0x60000..0x61000. It doesn't get to "if (max == end)" > > because "pos (=0x2029b) < bdata->node_min_pfn (=0x60000)". > > OK. the 0x2029b is the result of PAGE_SIZE being 8192 in your case. > However the bootmem allocator deals with physical addresses not virtual > ones. So it shouldn't give you a 0x60000..0x61000 range. > > Would be interesting to see what result you get on line 860 of > mm/bootmem.c. > Nothing; __alloc_bootmem_low_node() is not called. Call chain is: pcpu_alloc_alloc_info memblock_virt_alloc_nopanic __alloc_bootmem_nopanic ___alloc_bootmem_nopanic and returns 0xc0536000. Guenter From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f70.google.com (mail-pg0-f70.google.com [74.125.83.70]) by kanga.kvack.org (Postfix) with ESMTP id 42FA46B0261 for ; Mon, 20 Nov 2017 13:51:42 -0500 (EST) Received: by mail-pg0-f70.google.com with SMTP id u3so10276899pgn.11 for ; Mon, 20 Nov 2017 10:51:42 -0800 (PST) Received: from mail-sor-f65.google.com (mail-sor-f65.google.com. [209.85.220.65]) by mx.google.com with SMTPS id 84sor2991569pgc.32.2017.11.20.10.51.40 for (Google Transport Security); Mon, 20 Nov 2017 10:51:41 -0800 (PST) Date: Mon, 20 Nov 2017 10:51:38 -0800 From: Guenter Roeck Subject: Re: mm/percpu.c: use smarter memory allocation for struct pcpu_alloc_info (crisv32 hang) Message-ID: <20171120185138.GB23789@roeck-us.net> References: <20171118182542.GA23928@roeck-us.net> <62a3b680-6dde-d308-3da8-9c9a2789b114@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: owner-linux-mm@kvack.org List-ID: To: Nicolas Pitre Cc: Tejun Heo , Christoph Lameter , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Mikael Starvik , Jesper Nilsson , linux-cris-kernel@axis.com On Mon, Nov 20, 2017 at 01:18:38PM -0500, Nicolas Pitre wrote: > On Sun, 19 Nov 2017, Guenter Roeck wrote: > > > On 11/19/2017 08:08 PM, Nicolas Pitre wrote: > > > On Sun, 19 Nov 2017, Guenter Roeck wrote: > > > > On 11/19/2017 12:36 PM, Nicolas Pitre wrote: > > > > > On Sat, 18 Nov 2017, Guenter Roeck wrote: > > > > > > On Tue, Oct 03, 2017 at 06:29:49PM -0400, Nicolas Pitre wrote: > > > > > > > @@ -2295,6 +2295,7 @@ void __init setup_per_cpu_areas(void) > > > > > > > if (pcpu_setup_first_chunk(ai, fc) < 0) > > > > > > > panic("Failed to initialize percpu areas."); > > > > > > > + pcpu_free_alloc_info(ai); > > > > > > > > > > > > This is the culprit. Everything works fine if I remove this line. > > > > > > > > > > Without this line, the memory at the ai pointer is leaked. Maybe this is > > > > > modifying the memory allocation pattern and that triggers a bug later on > > > > > in your case. > > > > > > > > > > At that point the console driver is not yet initialized and any error > > > > > message won't be printed. You should enable the early console mechanism > > > > > in your kernel (see arch/cris/arch-v32/kernel/debugport.c) and see what > > > > > that might tell you. > > > > > > > > > > > > > The problem is that BUG() on crisv32 does not yield useful output. > > > > Anyway, here is the culprit. > > > > > > > > diff --git a/mm/bootmem.c b/mm/bootmem.c > > > > index 6aef64254203..2bcc8901450c 100644 > > > > --- a/mm/bootmem.c > > > > +++ b/mm/bootmem.c > > > > @@ -382,7 +382,8 @@ static int __init mark_bootmem(unsigned long start, > > > > unsigned long end, > > > > return 0; > > > > pos = bdata->node_low_pfn; > > > > } > > > > - BUG(); > > > > + WARN(1, "mark_bootmem(): memory range 0x%lx-0x%lx not found\n", > > > > start, > > > > end); > > > > + return -ENOMEM; > > > > } > > > > > > > > /** > > > > diff --git a/mm/percpu.c b/mm/percpu.c > > > > index 79e3549cab0f..c75622d844f1 100644 > > > > --- a/mm/percpu.c > > > > +++ b/mm/percpu.c > > > > @@ -1881,6 +1881,7 @@ struct pcpu_alloc_info * __init > > > > pcpu_alloc_alloc_info(int nr_groups, > > > > */ > > > > void __init pcpu_free_alloc_info(struct pcpu_alloc_info *ai) > > > > { > > > > + printk("pcpu_free_alloc_info(%p (0x%lx))\n", ai, __pa(ai)); > > > > memblock_free_early(__pa(ai), ai->__ai_size); > > > > > > The problem here is that there is two possibilities for > > > memblock_free_early(). From include/linux/bootmem.h: > > > > > > #if defined(CONFIG_HAVE_MEMBLOCK) && defined(CONFIG_NO_BOOTMEM) > > > > > > static inline void __init memblock_free_early( > > > phys_addr_t base, phys_addr_t size) > > > { > > > __memblock_free_early(base, size); > > > } > > > > > > #else > > > > > > static inline void __init memblock_free_early( > > > phys_addr_t base, phys_addr_t size) > > > { > > > free_bootmem(base, size); > > > } > > > > > > #endif > > > > > > It looks like most architectures use the memblock variant, including all > > > the ones I have access to. > > > > > > > results in: > > > > > > > > pcpu_free_alloc_info(c0534000 (0x40534000)) > > > > ------------[ cut here ]------------ > > > > WARNING: CPU: 0 PID: 0 at mm/bootmem.c:385 mark_bootmem+0x9a/0xaa > > > > mark_bootmem(): memory range 0x2029a-0x2029b not found > > > > > > Well... PFN_UP(0x40534000) should give 0x40534. How you might end up > > > with 0x2029a in mark_bootmem(), let alone not exit on the first "if (max > > > == end) return 0;" within the loop is rather weird. > > > > > pcpu_free_alloc_info: ai=c0536000, __pa(ai)=0x40536000, > > PFN_UP(__pa(ai))=0x2029b, PFN_UP(ai)=0x6029b > > > > bootmem range is 0x60000..0x61000. It doesn't get to "if (max == end)" > > because "pos (=0x2029b) < bdata->node_min_pfn (=0x60000)". > > OK. the 0x2029b is the result of PAGE_SIZE being 8192 in your case. > However the bootmem allocator deals with physical addresses not virtual > ones. So it shouldn't give you a 0x60000..0x61000 range. > > Would be interesting to see what result you get on line 860 of > mm/bootmem.c. > Nothing; __alloc_bootmem_low_node() is not called. Call chain is: pcpu_alloc_alloc_info memblock_virt_alloc_nopanic __alloc_bootmem_nopanic ___alloc_bootmem_nopanic and returns 0xc0536000. Guenter -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org