From: Nicolas Pitre <nicolas.pitre@linaro.org> To: Guenter Roeck <linux@roeck-us.net> Cc: Tejun Heo <tj@kernel.org>, Christoph Lameter <cl@linux.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Mikael Starvik <starvik@axis.com>, Jesper Nilsson <jesper.nilsson@axis.com>, linux-cris-kernel@axis.com Subject: Re: mm/percpu.c: use smarter memory allocation for struct pcpu_alloc_info (crisv32 hang) Date: Mon, 20 Nov 2017 22:50:46 -0500 (EST) [thread overview] Message-ID: <nycvar.YSQ.7.76.1711202224490.16045@knanqh.ubzr> (raw) In-Reply-To: <20171121014818.GA360@roeck-us.net> On Mon, 20 Nov 2017, Guenter Roeck wrote: > On Mon, Nov 20, 2017 at 07:28:21PM -0500, Nicolas Pitre wrote: > > On Mon, 20 Nov 2017, Guenter Roeck wrote: > > > > > bdata->node_min_pfn=60000 PFN_PHYS(bdata->node_min_pfn)=c0000000 start_off=536000 region=c0536000 > > > > If PFN_PHYS(bdata->node_min_pfn)=c0000000 and > > region=c0536000 that means phys_to_virt() is a no-op. > > > No, it is |= 0x80000000 Then the bootmem registration looks very fishy. If you have: > I think the problem is the 0x60000 in bdata->node_min_pfn. It is shifted > left by PFN_PHYS, making it 0xc0000000, which in my understanding is > a virtual address. Exact. #define __pa(x) ((unsigned long)(x) & 0x7fffffff) #define __va(x) ((void *)((unsigned long)(x) | 0x80000000)) With that, the only possible physical address range you may have is 0x40000000 - 0x7fffffff, and it better start at 0x40000000. If that's not where your RAM is then something is wrong. This is in fact a very bad idea to define __va() and __pa() using bitwise operations as this hides mistakes like defining physical RAM address at 0xc0000000. Instead, it should look like: #define __pa(x) ((unsigned long)(x) - 0x80000000) #define __va(x) ((void *)((unsigned long)(x) + 0x80000000)) This way, bad physical RAM address definitions will be caught immediately. > That doesn't seem to be easy to fix. It seems there is a mixup of physical > and virtual addresses in the architecture. Well... I don't think there is much else to say other than this needs fixing. Nicolas
WARNING: multiple messages have this Message-ID (diff)
From: Nicolas Pitre <nicolas.pitre@linaro.org> To: Guenter Roeck <linux@roeck-us.net> Cc: Tejun Heo <tj@kernel.org>, Christoph Lameter <cl@linux.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Mikael Starvik <starvik@axis.com>, Jesper Nilsson <jesper.nilsson@axis.com>, linux-cris-kernel@axis.com Subject: Re: mm/percpu.c: use smarter memory allocation for struct pcpu_alloc_info (crisv32 hang) Date: Mon, 20 Nov 2017 22:50:46 -0500 (EST) [thread overview] Message-ID: <nycvar.YSQ.7.76.1711202224490.16045@knanqh.ubzr> (raw) In-Reply-To: <20171121014818.GA360@roeck-us.net> On Mon, 20 Nov 2017, Guenter Roeck wrote: > On Mon, Nov 20, 2017 at 07:28:21PM -0500, Nicolas Pitre wrote: > > On Mon, 20 Nov 2017, Guenter Roeck wrote: > > > > > bdata->node_min_pfn=60000 PFN_PHYS(bdata->node_min_pfn)=c0000000 start_off=536000 region=c0536000 > > > > If PFN_PHYS(bdata->node_min_pfn)=c0000000 and > > region=c0536000 that means phys_to_virt() is a no-op. > > > No, it is |= 0x80000000 Then the bootmem registration looks very fishy. If you have: > I think the problem is the 0x60000 in bdata->node_min_pfn. It is shifted > left by PFN_PHYS, making it 0xc0000000, which in my understanding is > a virtual address. Exact. #define __pa(x) ((unsigned long)(x) & 0x7fffffff) #define __va(x) ((void *)((unsigned long)(x) | 0x80000000)) With that, the only possible physical address range you may have is 0x40000000 - 0x7fffffff, and it better start at 0x40000000. If that's not where your RAM is then something is wrong. This is in fact a very bad idea to define __va() and __pa() using bitwise operations as this hides mistakes like defining physical RAM address at 0xc0000000. Instead, it should look like: #define __pa(x) ((unsigned long)(x) - 0x80000000) #define __va(x) ((void *)((unsigned long)(x) + 0x80000000)) This way, bad physical RAM address definitions will be caught immediately. > That doesn't seem to be easy to fix. It seems there is a mixup of physical > and virtual addresses in the architecture. Well... I don't think there is much else to say other than this needs fixing. Nicolas -- 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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2017-11-21 3:50 UTC|newest] Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-03 20:57 [PATCH] mm/percpu.c: use smarter memory allocation for struct pcpu_alloc_info Nicolas Pitre 2017-10-03 20:57 ` Nicolas Pitre 2017-10-03 21:05 ` Tejun Heo 2017-10-03 21:05 ` Tejun Heo 2017-10-03 22:29 ` Nicolas Pitre 2017-10-03 22:29 ` Nicolas Pitre 2017-10-03 22:36 ` Tejun Heo 2017-10-03 22:36 ` Tejun Heo 2017-10-03 23:48 ` Dennis Zhou 2017-10-03 23:48 ` Dennis Zhou 2017-10-04 0:13 ` Nicolas Pitre 2017-10-04 0:13 ` Nicolas Pitre 2017-10-04 14:15 ` Tejun Heo 2017-10-04 14:15 ` Tejun Heo 2017-11-18 18:25 ` mm/percpu.c: use smarter memory allocation for struct pcpu_alloc_info (crisv32 hang) Guenter Roeck 2017-11-18 18:25 ` Guenter Roeck 2017-11-19 20:36 ` Nicolas Pitre 2017-11-19 20:36 ` Nicolas Pitre 2017-11-20 2:03 ` Guenter Roeck 2017-11-20 2:03 ` Guenter Roeck 2017-11-20 4:08 ` Nicolas Pitre 2017-11-20 4:08 ` Nicolas Pitre 2017-11-20 5:05 ` Guenter Roeck 2017-11-20 5:05 ` Guenter Roeck 2017-11-20 18:18 ` Nicolas Pitre 2017-11-20 18:18 ` Nicolas Pitre 2017-11-20 18:51 ` Guenter Roeck 2017-11-20 18:51 ` Guenter Roeck 2017-11-20 20:21 ` Nicolas Pitre 2017-11-20 20:21 ` Nicolas Pitre 2017-11-20 21:11 ` Guenter Roeck 2017-11-20 21:11 ` Guenter Roeck 2017-11-21 0:28 ` Nicolas Pitre 2017-11-21 0:28 ` Nicolas Pitre 2017-11-21 1:48 ` Guenter Roeck 2017-11-21 1:48 ` Guenter Roeck 2017-11-21 3:50 ` Nicolas Pitre [this message] 2017-11-21 3:50 ` Nicolas Pitre 2017-11-22 15:34 ` Jesper Nilsson 2017-11-22 15:34 ` Jesper Nilsson 2017-11-22 20:17 ` Nicolas Pitre 2017-11-22 20:17 ` Nicolas Pitre 2017-11-23 7:56 ` Jesper Nilsson 2017-11-23 7:56 ` Jesper Nilsson 2017-11-27 19:41 ` Tejun Heo 2017-11-27 19:41 ` Tejun Heo 2017-11-27 20:31 ` Nicolas Pitre 2017-11-27 20:31 ` Nicolas Pitre 2017-11-27 20:33 ` Tejun Heo 2017-11-27 20:33 ` Tejun Heo 2017-11-27 20:51 ` Nicolas Pitre 2017-11-27 20:51 ` Nicolas Pitre 2017-11-27 20:54 ` Tejun Heo 2017-11-27 20:54 ` Tejun Heo 2017-11-27 21:11 ` Guenter Roeck 2017-11-27 21:11 ` Guenter Roeck 2017-11-28 8:19 ` Jesper Nilsson 2017-11-28 8:19 ` Jesper Nilsson
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=nycvar.YSQ.7.76.1711202224490.16045@knanqh.ubzr \ --to=nicolas.pitre@linaro.org \ --cc=cl@linux.com \ --cc=jesper.nilsson@axis.com \ --cc=linux-cris-kernel@axis.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux@roeck-us.net \ --cc=starvik@axis.com \ --cc=tj@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.