Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 11:30:34PM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> > On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> > ...
> > > >
> > > > My concern is that there are a few E820 memory types rather than
> > > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > > all into memblock.reserved is really acceptable.
> > >
> > > Hi Naoya,
> > >
> > > Maybe you could just add to memblock.reserved, all unavailable ranges
> > > within
> > > E820_TYPE_RAM.
> > > Actually, in your original patch, you are walking memblock.memory, which
> > > should
> > > only contain E820_TYPE_RAM ranges (talking about x86).
> > >
> > > So I think the below would to the trick as well?
> > >
> > > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > > {
> > > int i;
> > > u64 end;
> > > + u64 next = 0;
> > >
> > > /*
> > > * The bootstrap memblock region count maximum is 128 entries
> > >
> > > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> > >
> > > if (entry->type != E820_TYPE_RAM && entry->type !=
> > > E820_TYPE_RESERVED_KERN)
> > > continue;
> > >
> > > +
> > > + if (entry->type == E820_TYPE_RAM)
> > > + if (next < entry->addr) {
> > > + memblock_reserve (next, next + (entry->addr -
> > > next));
> > > + next = end;
> > > + }
> > >
> > > With the above patch, I can no longer see the issues either.
> >
> > I double-checked and this change looks good to me.
> >
> > >
> > > Although, there is a difference between this and your original patch.
> > > In your original patch, you are just zeroing the pages, while with this
> > > one (or with your second patch),
> > > we will zero the page in reserve_bootmem_region(), but that function also
> > > init
> > > some other fields of the struct page:
> > >
> > > mm_zero_struct_page(page);
> > > set_page_links(page, zone, nid, pfn);
> > > init_page_count(page);
> > > page_mapcount_reset(page);
> > > page_cpupid_reset_last(page);
> > >
> > > So I am not sure we want to bother doing that for pages that are really
> > > unreachable.
> >
> > I think that considering that /proc/kpageflags can check them, some data
> > (even if it's trivial) might be better than just zeros.
> >
> > Here's the updated patch.
> > Thanks for the suggestion and testing!
> >
> > ---
> > From: Naoya Horiguchi
> > Date: Thu, 14 Jun 2018 14:44:36 +0900
> > Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
> >
> > There is a kernel panic that is triggered when reading /proc/kpageflags
> > on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
> >
> > BUG: unable to handle kernel paging request at fffe
> > PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> > Oops: [#1] SMP PTI
> > CPU: 2 PID: 1728 Comm: page-types Not tainted
> > 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> > 04/01/2014
> > RIP: 0010:stable_page_flags+0x27/0x3c0
> > Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89
> > fc 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6
> > c4 01 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> > RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> > RAX: fffe RBX: 7fffeff9 RCX:
> > RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> > RBP: R08: 0001 R09: 0001
> > R10: bbd44111fed8 R11: R12: ed1182fff5c0
> > R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> > FS: 7efc4335a500() GS:93a5bfc0()
> > knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: fffe CR3: b2a58000 CR4: 001406e0
> > Call Trace:
> >kpageflags_read+0xc7/0x120
> >proc_reg_read+0x3c/0x60
> >__vfs_read+0x36/0x170
> >vfs_read+0x89/0x130
> >ksys_pread64+0x71/0x90
> >do_syscall_64+0x5b/0x160
> >entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > RIP: 0033:0x7efc42e75e23
> > Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00
> > 90 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0
> > ff ff 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
> >
> > According to kernel bisection, this problem became visible due to commit
> > f7f99100d8d9 which changes how struct pages are initialized.
> >
> > Memblock layout affects the pfn ranges covered by node/zone. Consider
> > that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> > the
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 11:30:34PM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> > On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> > ...
> > > >
> > > > My concern is that there are a few E820 memory types rather than
> > > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > > all into memblock.reserved is really acceptable.
> > >
> > > Hi Naoya,
> > >
> > > Maybe you could just add to memblock.reserved, all unavailable ranges
> > > within
> > > E820_TYPE_RAM.
> > > Actually, in your original patch, you are walking memblock.memory, which
> > > should
> > > only contain E820_TYPE_RAM ranges (talking about x86).
> > >
> > > So I think the below would to the trick as well?
> > >
> > > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > > {
> > > int i;
> > > u64 end;
> > > + u64 next = 0;
> > >
> > > /*
> > > * The bootstrap memblock region count maximum is 128 entries
> > >
> > > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> > >
> > > if (entry->type != E820_TYPE_RAM && entry->type !=
> > > E820_TYPE_RESERVED_KERN)
> > > continue;
> > >
> > > +
> > > + if (entry->type == E820_TYPE_RAM)
> > > + if (next < entry->addr) {
> > > + memblock_reserve (next, next + (entry->addr -
> > > next));
> > > + next = end;
> > > + }
> > >
> > > With the above patch, I can no longer see the issues either.
> >
> > I double-checked and this change looks good to me.
> >
> > >
> > > Although, there is a difference between this and your original patch.
> > > In your original patch, you are just zeroing the pages, while with this
> > > one (or with your second patch),
> > > we will zero the page in reserve_bootmem_region(), but that function also
> > > init
> > > some other fields of the struct page:
> > >
> > > mm_zero_struct_page(page);
> > > set_page_links(page, zone, nid, pfn);
> > > init_page_count(page);
> > > page_mapcount_reset(page);
> > > page_cpupid_reset_last(page);
> > >
> > > So I am not sure we want to bother doing that for pages that are really
> > > unreachable.
> >
> > I think that considering that /proc/kpageflags can check them, some data
> > (even if it's trivial) might be better than just zeros.
> >
> > Here's the updated patch.
> > Thanks for the suggestion and testing!
> >
> > ---
> > From: Naoya Horiguchi
> > Date: Thu, 14 Jun 2018 14:44:36 +0900
> > Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
> >
> > There is a kernel panic that is triggered when reading /proc/kpageflags
> > on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
> >
> > BUG: unable to handle kernel paging request at fffe
> > PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> > Oops: [#1] SMP PTI
> > CPU: 2 PID: 1728 Comm: page-types Not tainted
> > 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> > 04/01/2014
> > RIP: 0010:stable_page_flags+0x27/0x3c0
> > Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89
> > fc 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6
> > c4 01 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> > RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> > RAX: fffe RBX: 7fffeff9 RCX:
> > RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> > RBP: R08: 0001 R09: 0001
> > R10: bbd44111fed8 R11: R12: ed1182fff5c0
> > R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> > FS: 7efc4335a500() GS:93a5bfc0()
> > knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: fffe CR3: b2a58000 CR4: 001406e0
> > Call Trace:
> >kpageflags_read+0xc7/0x120
> >proc_reg_read+0x3c/0x60
> >__vfs_read+0x36/0x170
> >vfs_read+0x89/0x130
> >ksys_pread64+0x71/0x90
> >do_syscall_64+0x5b/0x160
> >entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > RIP: 0033:0x7efc42e75e23
> > Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00
> > 90 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0
> > ff ff 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
> >
> > According to kernel bisection, this problem became visible due to commit
> > f7f99100d8d9 which changes how struct pages are initialized.
> >
> > Memblock layout affects the pfn ranges covered by node/zone. Consider
> > that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> > the
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 01:24:37PM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 09:21:03AM +0200, Oscar Salvador wrote:
> > On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> > > On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > > > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> > > ...
> > > > >
> > > > > My concern is that there are a few E820 memory types rather than
> > > > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting
> > > > > them
> > > > > all into memblock.reserved is really acceptable.
> > > >
> > > > Hi Naoya,
> > > >
> > > > Maybe you could just add to memblock.reserved, all unavailable ranges
> > > > within
> > > > E820_TYPE_RAM.
> > > > Actually, in your original patch, you are walking memblock.memory,
> > > > which should
> > > > only contain E820_TYPE_RAM ranges (talking about x86).
> > > >
> > > > So I think the below would to the trick as well?
> > > >
> > > > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > > > {
> > > > int i;
> > > > u64 end;
> > > > + u64 next = 0;
> > > >
> > > > /*
> > > > * The bootstrap memblock region count maximum is 128 entries
> > > >
> > > > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> > > >
> > > > if (entry->type != E820_TYPE_RAM && entry->type !=
> > > > E820_TYPE_RESERVED_KERN)
> > > > continue;
> > > >
> > > > +
> > > > + if (entry->type == E820_TYPE_RAM)
> > > > + if (next < entry->addr) {
> > > > + memblock_reserve (next, next +
> > > > (entry->addr - next));
> > > > + next = end;
> > > > + }
> > > >
> > > > With the above patch, I can no longer see the issues either.
> > >
> > > I double-checked and this change looks good to me.
> > >
> > > >
> > > > Although, there is a difference between this and your original patch.
> > > > In your original patch, you are just zeroing the pages, while with this
> > > > one (or with your second patch),
> > > > we will zero the page in reserve_bootmem_region(), but that function
> > > > also init
> > > > some other fields of the struct page:
> > > >
> > > > mm_zero_struct_page(page);
> > > > set_page_links(page, zone, nid, pfn);
> > > > init_page_count(page);
> > > > page_mapcount_reset(page);
> > > > page_cpupid_reset_last(page);
> > > >
> > > > So I am not sure we want to bother doing that for pages that are really
> > > > unreachable.
> > >
> > > I think that considering that /proc/kpageflags can check them, some data
> > > (even if it's trivial) might be better than just zeros.
> > >
> > > Here's the updated patch.
> > > Thanks for the suggestion and testing!
> > >
> > > ---
> > > From: Naoya Horiguchi
> > > Date: Thu, 14 Jun 2018 14:44:36 +0900
> > > Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into
> > > memblock.reserved
> > >
> > > There is a kernel panic that is triggered when reading /proc/kpageflags
> > > on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
> > >
> > > BUG: unable to handle kernel paging request at fffe
> > > PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> > > Oops: [#1] SMP PTI
> > > CPU: 2 PID: 1728 Comm: page-types Not tainted
> > > 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > > 1.11.0-2.fc28 04/01/2014
> > > RIP: 0010:stable_page_flags+0x27/0x3c0
> > > Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89
> > > fc 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6
> > > c4 01 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> > > RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> > > RAX: fffe RBX: 7fffeff9 RCX:
> > > RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> > > RBP: R08: 0001 R09: 0001
> > > R10: bbd44111fed8 R11: R12: ed1182fff5c0
> > > R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> > > FS: 7efc4335a500() GS:93a5bfc0()
> > > knlGS:
> > > CS: 0010 DS: ES: CR0: 80050033
> > > CR2: fffe CR3: b2a58000 CR4: 001406e0
> > > Call Trace:
> > >kpageflags_read+0xc7/0x120
> > >proc_reg_read+0x3c/0x60
> > >__vfs_read+0x36/0x170
> > >vfs_read+0x89/0x130
> > >ksys_pread64+0x71/0x90
> > >do_syscall_64+0x5b/0x160
> > >entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > > RIP: 0033:0x7efc42e75e23
> > > Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00
> > > 90 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0
> > > ff ff 73 34 c3 48 83 ec 08 e8 db d3
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 01:24:37PM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 09:21:03AM +0200, Oscar Salvador wrote:
> > On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> > > On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > > > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> > > ...
> > > > >
> > > > > My concern is that there are a few E820 memory types rather than
> > > > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting
> > > > > them
> > > > > all into memblock.reserved is really acceptable.
> > > >
> > > > Hi Naoya,
> > > >
> > > > Maybe you could just add to memblock.reserved, all unavailable ranges
> > > > within
> > > > E820_TYPE_RAM.
> > > > Actually, in your original patch, you are walking memblock.memory,
> > > > which should
> > > > only contain E820_TYPE_RAM ranges (talking about x86).
> > > >
> > > > So I think the below would to the trick as well?
> > > >
> > > > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > > > {
> > > > int i;
> > > > u64 end;
> > > > + u64 next = 0;
> > > >
> > > > /*
> > > > * The bootstrap memblock region count maximum is 128 entries
> > > >
> > > > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> > > >
> > > > if (entry->type != E820_TYPE_RAM && entry->type !=
> > > > E820_TYPE_RESERVED_KERN)
> > > > continue;
> > > >
> > > > +
> > > > + if (entry->type == E820_TYPE_RAM)
> > > > + if (next < entry->addr) {
> > > > + memblock_reserve (next, next +
> > > > (entry->addr - next));
> > > > + next = end;
> > > > + }
> > > >
> > > > With the above patch, I can no longer see the issues either.
> > >
> > > I double-checked and this change looks good to me.
> > >
> > > >
> > > > Although, there is a difference between this and your original patch.
> > > > In your original patch, you are just zeroing the pages, while with this
> > > > one (or with your second patch),
> > > > we will zero the page in reserve_bootmem_region(), but that function
> > > > also init
> > > > some other fields of the struct page:
> > > >
> > > > mm_zero_struct_page(page);
> > > > set_page_links(page, zone, nid, pfn);
> > > > init_page_count(page);
> > > > page_mapcount_reset(page);
> > > > page_cpupid_reset_last(page);
> > > >
> > > > So I am not sure we want to bother doing that for pages that are really
> > > > unreachable.
> > >
> > > I think that considering that /proc/kpageflags can check them, some data
> > > (even if it's trivial) might be better than just zeros.
> > >
> > > Here's the updated patch.
> > > Thanks for the suggestion and testing!
> > >
> > > ---
> > > From: Naoya Horiguchi
> > > Date: Thu, 14 Jun 2018 14:44:36 +0900
> > > Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into
> > > memblock.reserved
> > >
> > > There is a kernel panic that is triggered when reading /proc/kpageflags
> > > on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
> > >
> > > BUG: unable to handle kernel paging request at fffe
> > > PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> > > Oops: [#1] SMP PTI
> > > CPU: 2 PID: 1728 Comm: page-types Not tainted
> > > 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > > 1.11.0-2.fc28 04/01/2014
> > > RIP: 0010:stable_page_flags+0x27/0x3c0
> > > Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89
> > > fc 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6
> > > c4 01 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> > > RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> > > RAX: fffe RBX: 7fffeff9 RCX:
> > > RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> > > RBP: R08: 0001 R09: 0001
> > > R10: bbd44111fed8 R11: R12: ed1182fff5c0
> > > R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> > > FS: 7efc4335a500() GS:93a5bfc0()
> > > knlGS:
> > > CS: 0010 DS: ES: CR0: 80050033
> > > CR2: fffe CR3: b2a58000 CR4: 001406e0
> > > Call Trace:
> > >kpageflags_read+0xc7/0x120
> > >proc_reg_read+0x3c/0x60
> > >__vfs_read+0x36/0x170
> > >vfs_read+0x89/0x130
> > >ksys_pread64+0x71/0x90
> > >do_syscall_64+0x5b/0x160
> > >entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > > RIP: 0033:0x7efc42e75e23
> > > Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00
> > > 90 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0
> > > ff ff 73 34 c3 48 83 ec 08 e8 db d3
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> ...
> > >
> > > My concern is that there are a few E820 memory types rather than
> > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > all into memblock.reserved is really acceptable.
> >
> > Hi Naoya,
> >
> > Maybe you could just add to memblock.reserved, all unavailable ranges within
> > E820_TYPE_RAM.
> > Actually, in your original patch, you are walking memblock.memory, which
> > should
> > only contain E820_TYPE_RAM ranges (talking about x86).
> >
> > So I think the below would to the trick as well?
> >
> > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > {
> > int i;
> > u64 end;
> > + u64 next = 0;
> >
> > /*
> > * The bootstrap memblock region count maximum is 128 entries
> >
> > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> >
> > if (entry->type != E820_TYPE_RAM && entry->type !=
> > E820_TYPE_RESERVED_KERN)
> > continue;
> >
> > +
> > + if (entry->type == E820_TYPE_RAM)
> > + if (next < entry->addr) {
> > + memblock_reserve (next, next + (entry->addr -
> > next));
> > + next = end;
> > + }
> >
> > With the above patch, I can no longer see the issues either.
>
> I double-checked and this change looks good to me.
>
> >
> > Although, there is a difference between this and your original patch.
> > In your original patch, you are just zeroing the pages, while with this one
> > (or with your second patch),
> > we will zero the page in reserve_bootmem_region(), but that function also
> > init
> > some other fields of the struct page:
> >
> > mm_zero_struct_page(page);
> > set_page_links(page, zone, nid, pfn);
> > init_page_count(page);
> > page_mapcount_reset(page);
> > page_cpupid_reset_last(page);
> >
> > So I am not sure we want to bother doing that for pages that are really
> > unreachable.
>
> I think that considering that /proc/kpageflags can check them, some data
> (even if it's trivial) might be better than just zeros.
>
> Here's the updated patch.
> Thanks for the suggestion and testing!
>
> ---
> From: Naoya Horiguchi
> Date: Thu, 14 Jun 2018 14:44:36 +0900
> Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
>
> There is a kernel panic that is triggered when reading /proc/kpageflags
> on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
>
> BUG: unable to handle kernel paging request at fffe
> PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> Oops: [#1] SMP PTI
> CPU: 2 PID: 1728 Comm: page-types Not tainted
> 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> 04/01/2014
> RIP: 0010:stable_page_flags+0x27/0x3c0
> Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89 fc
> 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6 c4 01
> 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> RAX: fffe RBX: 7fffeff9 RCX:
> RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> RBP: R08: 0001 R09: 0001
> R10: bbd44111fed8 R11: R12: ed1182fff5c0
> R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> FS: 7efc4335a500() GS:93a5bfc0() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: fffe CR3: b2a58000 CR4: 001406e0
> Call Trace:
>kpageflags_read+0xc7/0x120
>proc_reg_read+0x3c/0x60
>__vfs_read+0x36/0x170
>vfs_read+0x89/0x130
>ksys_pread64+0x71/0x90
>do_syscall_64+0x5b/0x160
>entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7efc42e75e23
> Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00 90
> 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0 ff ff
> 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
>
> According to kernel bisection, this problem became visible due to commit
> f7f99100d8d9 which changes how struct pages are initialized.
>
> Memblock layout affects the pfn ranges covered by node/zone. Consider
> that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> the default (no memmap= given) memblock layout is like below:
>
> MEMBLOCK configuration:
>memory size = 0x0001fff75c00 reserved size = 0x0300c000
>memory.cnt = 0x4
>memory[0x0] [0x1000-0x0009efff],
> 0x0009e000 bytes on node 0 flags: 0x0
>
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> ...
> > >
> > > My concern is that there are a few E820 memory types rather than
> > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > all into memblock.reserved is really acceptable.
> >
> > Hi Naoya,
> >
> > Maybe you could just add to memblock.reserved, all unavailable ranges within
> > E820_TYPE_RAM.
> > Actually, in your original patch, you are walking memblock.memory, which
> > should
> > only contain E820_TYPE_RAM ranges (talking about x86).
> >
> > So I think the below would to the trick as well?
> >
> > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > {
> > int i;
> > u64 end;
> > + u64 next = 0;
> >
> > /*
> > * The bootstrap memblock region count maximum is 128 entries
> >
> > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> >
> > if (entry->type != E820_TYPE_RAM && entry->type !=
> > E820_TYPE_RESERVED_KERN)
> > continue;
> >
> > +
> > + if (entry->type == E820_TYPE_RAM)
> > + if (next < entry->addr) {
> > + memblock_reserve (next, next + (entry->addr -
> > next));
> > + next = end;
> > + }
> >
> > With the above patch, I can no longer see the issues either.
>
> I double-checked and this change looks good to me.
>
> >
> > Although, there is a difference between this and your original patch.
> > In your original patch, you are just zeroing the pages, while with this one
> > (or with your second patch),
> > we will zero the page in reserve_bootmem_region(), but that function also
> > init
> > some other fields of the struct page:
> >
> > mm_zero_struct_page(page);
> > set_page_links(page, zone, nid, pfn);
> > init_page_count(page);
> > page_mapcount_reset(page);
> > page_cpupid_reset_last(page);
> >
> > So I am not sure we want to bother doing that for pages that are really
> > unreachable.
>
> I think that considering that /proc/kpageflags can check them, some data
> (even if it's trivial) might be better than just zeros.
>
> Here's the updated patch.
> Thanks for the suggestion and testing!
>
> ---
> From: Naoya Horiguchi
> Date: Thu, 14 Jun 2018 14:44:36 +0900
> Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
>
> There is a kernel panic that is triggered when reading /proc/kpageflags
> on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
>
> BUG: unable to handle kernel paging request at fffe
> PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> Oops: [#1] SMP PTI
> CPU: 2 PID: 1728 Comm: page-types Not tainted
> 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> 04/01/2014
> RIP: 0010:stable_page_flags+0x27/0x3c0
> Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89 fc
> 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6 c4 01
> 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> RAX: fffe RBX: 7fffeff9 RCX:
> RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> RBP: R08: 0001 R09: 0001
> R10: bbd44111fed8 R11: R12: ed1182fff5c0
> R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> FS: 7efc4335a500() GS:93a5bfc0() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: fffe CR3: b2a58000 CR4: 001406e0
> Call Trace:
>kpageflags_read+0xc7/0x120
>proc_reg_read+0x3c/0x60
>__vfs_read+0x36/0x170
>vfs_read+0x89/0x130
>ksys_pread64+0x71/0x90
>do_syscall_64+0x5b/0x160
>entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7efc42e75e23
> Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00 90
> 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0 ff ff
> 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
>
> According to kernel bisection, this problem became visible due to commit
> f7f99100d8d9 which changes how struct pages are initialized.
>
> Memblock layout affects the pfn ranges covered by node/zone. Consider
> that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> the default (no memmap= given) memblock layout is like below:
>
> MEMBLOCK configuration:
>memory size = 0x0001fff75c00 reserved size = 0x0300c000
>memory.cnt = 0x4
>memory[0x0] [0x1000-0x0009efff],
> 0x0009e000 bytes on node 0 flags: 0x0
>
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 09:21:03AM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> > On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> > ...
> > > >
> > > > My concern is that there are a few E820 memory types rather than
> > > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > > all into memblock.reserved is really acceptable.
> > >
> > > Hi Naoya,
> > >
> > > Maybe you could just add to memblock.reserved, all unavailable ranges
> > > within
> > > E820_TYPE_RAM.
> > > Actually, in your original patch, you are walking memblock.memory, which
> > > should
> > > only contain E820_TYPE_RAM ranges (talking about x86).
> > >
> > > So I think the below would to the trick as well?
> > >
> > > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > > {
> > > int i;
> > > u64 end;
> > > + u64 next = 0;
> > >
> > > /*
> > > * The bootstrap memblock region count maximum is 128 entries
> > >
> > > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> > >
> > > if (entry->type != E820_TYPE_RAM && entry->type !=
> > > E820_TYPE_RESERVED_KERN)
> > > continue;
> > >
> > > +
> > > + if (entry->type == E820_TYPE_RAM)
> > > + if (next < entry->addr) {
> > > + memblock_reserve (next, next + (entry->addr -
> > > next));
> > > + next = end;
> > > + }
> > >
> > > With the above patch, I can no longer see the issues either.
> >
> > I double-checked and this change looks good to me.
> >
> > >
> > > Although, there is a difference between this and your original patch.
> > > In your original patch, you are just zeroing the pages, while with this
> > > one (or with your second patch),
> > > we will zero the page in reserve_bootmem_region(), but that function also
> > > init
> > > some other fields of the struct page:
> > >
> > > mm_zero_struct_page(page);
> > > set_page_links(page, zone, nid, pfn);
> > > init_page_count(page);
> > > page_mapcount_reset(page);
> > > page_cpupid_reset_last(page);
> > >
> > > So I am not sure we want to bother doing that for pages that are really
> > > unreachable.
> >
> > I think that considering that /proc/kpageflags can check them, some data
> > (even if it's trivial) might be better than just zeros.
> >
> > Here's the updated patch.
> > Thanks for the suggestion and testing!
> >
> > ---
> > From: Naoya Horiguchi
> > Date: Thu, 14 Jun 2018 14:44:36 +0900
> > Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
> >
> > There is a kernel panic that is triggered when reading /proc/kpageflags
> > on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
> >
> > BUG: unable to handle kernel paging request at fffe
> > PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> > Oops: [#1] SMP PTI
> > CPU: 2 PID: 1728 Comm: page-types Not tainted
> > 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> > 04/01/2014
> > RIP: 0010:stable_page_flags+0x27/0x3c0
> > Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89
> > fc 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6
> > c4 01 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> > RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> > RAX: fffe RBX: 7fffeff9 RCX:
> > RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> > RBP: R08: 0001 R09: 0001
> > R10: bbd44111fed8 R11: R12: ed1182fff5c0
> > R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> > FS: 7efc4335a500() GS:93a5bfc0()
> > knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: fffe CR3: b2a58000 CR4: 001406e0
> > Call Trace:
> >kpageflags_read+0xc7/0x120
> >proc_reg_read+0x3c/0x60
> >__vfs_read+0x36/0x170
> >vfs_read+0x89/0x130
> >ksys_pread64+0x71/0x90
> >do_syscall_64+0x5b/0x160
> >entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > RIP: 0033:0x7efc42e75e23
> > Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00
> > 90 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0
> > ff ff 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
> >
> > According to kernel bisection, this problem became visible due to commit
> > f7f99100d8d9 which changes how struct pages are initialized.
> >
> > Memblock layout affects the pfn ranges covered by node/zone. Consider
> > that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> > the
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 09:21:03AM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> > On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> > ...
> > > >
> > > > My concern is that there are a few E820 memory types rather than
> > > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > > all into memblock.reserved is really acceptable.
> > >
> > > Hi Naoya,
> > >
> > > Maybe you could just add to memblock.reserved, all unavailable ranges
> > > within
> > > E820_TYPE_RAM.
> > > Actually, in your original patch, you are walking memblock.memory, which
> > > should
> > > only contain E820_TYPE_RAM ranges (talking about x86).
> > >
> > > So I think the below would to the trick as well?
> > >
> > > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > > {
> > > int i;
> > > u64 end;
> > > + u64 next = 0;
> > >
> > > /*
> > > * The bootstrap memblock region count maximum is 128 entries
> > >
> > > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> > >
> > > if (entry->type != E820_TYPE_RAM && entry->type !=
> > > E820_TYPE_RESERVED_KERN)
> > > continue;
> > >
> > > +
> > > + if (entry->type == E820_TYPE_RAM)
> > > + if (next < entry->addr) {
> > > + memblock_reserve (next, next + (entry->addr -
> > > next));
> > > + next = end;
> > > + }
> > >
> > > With the above patch, I can no longer see the issues either.
> >
> > I double-checked and this change looks good to me.
> >
> > >
> > > Although, there is a difference between this and your original patch.
> > > In your original patch, you are just zeroing the pages, while with this
> > > one (or with your second patch),
> > > we will zero the page in reserve_bootmem_region(), but that function also
> > > init
> > > some other fields of the struct page:
> > >
> > > mm_zero_struct_page(page);
> > > set_page_links(page, zone, nid, pfn);
> > > init_page_count(page);
> > > page_mapcount_reset(page);
> > > page_cpupid_reset_last(page);
> > >
> > > So I am not sure we want to bother doing that for pages that are really
> > > unreachable.
> >
> > I think that considering that /proc/kpageflags can check them, some data
> > (even if it's trivial) might be better than just zeros.
> >
> > Here's the updated patch.
> > Thanks for the suggestion and testing!
> >
> > ---
> > From: Naoya Horiguchi
> > Date: Thu, 14 Jun 2018 14:44:36 +0900
> > Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
> >
> > There is a kernel panic that is triggered when reading /proc/kpageflags
> > on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
> >
> > BUG: unable to handle kernel paging request at fffe
> > PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> > Oops: [#1] SMP PTI
> > CPU: 2 PID: 1728 Comm: page-types Not tainted
> > 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> > 04/01/2014
> > RIP: 0010:stable_page_flags+0x27/0x3c0
> > Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89
> > fc 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6
> > c4 01 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> > RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> > RAX: fffe RBX: 7fffeff9 RCX:
> > RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> > RBP: R08: 0001 R09: 0001
> > R10: bbd44111fed8 R11: R12: ed1182fff5c0
> > R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> > FS: 7efc4335a500() GS:93a5bfc0()
> > knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: fffe CR3: b2a58000 CR4: 001406e0
> > Call Trace:
> >kpageflags_read+0xc7/0x120
> >proc_reg_read+0x3c/0x60
> >__vfs_read+0x36/0x170
> >vfs_read+0x89/0x130
> >ksys_pread64+0x71/0x90
> >do_syscall_64+0x5b/0x160
> >entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > RIP: 0033:0x7efc42e75e23
> > Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00
> > 90 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0
> > ff ff 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
> >
> > According to kernel bisection, this problem became visible due to commit
> > f7f99100d8d9 which changes how struct pages are initialized.
> >
> > Memblock layout affects the pfn ranges covered by node/zone. Consider
> > that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> > the
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> ...
> > >
> > > My concern is that there are a few E820 memory types rather than
> > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > all into memblock.reserved is really acceptable.
> >
> > Hi Naoya,
> >
> > Maybe you could just add to memblock.reserved, all unavailable ranges within
> > E820_TYPE_RAM.
> > Actually, in your original patch, you are walking memblock.memory, which
> > should
> > only contain E820_TYPE_RAM ranges (talking about x86).
> >
> > So I think the below would to the trick as well?
> >
> > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > {
> > int i;
> > u64 end;
> > + u64 next = 0;
> >
> > /*
> > * The bootstrap memblock region count maximum is 128 entries
> >
> > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> >
> > if (entry->type != E820_TYPE_RAM && entry->type !=
> > E820_TYPE_RESERVED_KERN)
> > continue;
> >
> > +
> > + if (entry->type == E820_TYPE_RAM)
> > + if (next < entry->addr) {
> > + memblock_reserve (next, next + (entry->addr -
> > next));
> > + next = end;
> > + }
> >
> > With the above patch, I can no longer see the issues either.
>
> I double-checked and this change looks good to me.
>
> >
> > Although, there is a difference between this and your original patch.
> > In your original patch, you are just zeroing the pages, while with this one
> > (or with your second patch),
> > we will zero the page in reserve_bootmem_region(), but that function also
> > init
> > some other fields of the struct page:
> >
> > mm_zero_struct_page(page);
> > set_page_links(page, zone, nid, pfn);
> > init_page_count(page);
> > page_mapcount_reset(page);
> > page_cpupid_reset_last(page);
> >
> > So I am not sure we want to bother doing that for pages that are really
> > unreachable.
>
> I think that considering that /proc/kpageflags can check them, some data
> (even if it's trivial) might be better than just zeros.
>
> Here's the updated patch.
> Thanks for the suggestion and testing!
>
> ---
> From: Naoya Horiguchi
> Date: Thu, 14 Jun 2018 14:44:36 +0900
> Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
>
> There is a kernel panic that is triggered when reading /proc/kpageflags
> on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
>
> BUG: unable to handle kernel paging request at fffe
> PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> Oops: [#1] SMP PTI
> CPU: 2 PID: 1728 Comm: page-types Not tainted
> 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> 04/01/2014
> RIP: 0010:stable_page_flags+0x27/0x3c0
> Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89 fc
> 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6 c4 01
> 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> RAX: fffe RBX: 7fffeff9 RCX:
> RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> RBP: R08: 0001 R09: 0001
> R10: bbd44111fed8 R11: R12: ed1182fff5c0
> R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> FS: 7efc4335a500() GS:93a5bfc0() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: fffe CR3: b2a58000 CR4: 001406e0
> Call Trace:
>kpageflags_read+0xc7/0x120
>proc_reg_read+0x3c/0x60
>__vfs_read+0x36/0x170
>vfs_read+0x89/0x130
>ksys_pread64+0x71/0x90
>do_syscall_64+0x5b/0x160
>entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7efc42e75e23
> Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00 90
> 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0 ff ff
> 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
>
> According to kernel bisection, this problem became visible due to commit
> f7f99100d8d9 which changes how struct pages are initialized.
>
> Memblock layout affects the pfn ranges covered by node/zone. Consider
> that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> the default (no memmap= given) memblock layout is like below:
>
> MEMBLOCK configuration:
>memory size = 0x0001fff75c00 reserved size = 0x0300c000
>memory.cnt = 0x4
>memory[0x0] [0x1000-0x0009efff],
> 0x0009e000 bytes on node 0 flags: 0x0
>
Re: [PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 06:34:55AM +, Naoya Horiguchi wrote:
> On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> > On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
> ...
> > >
> > > My concern is that there are a few E820 memory types rather than
> > > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > > all into memblock.reserved is really acceptable.
> >
> > Hi Naoya,
> >
> > Maybe you could just add to memblock.reserved, all unavailable ranges within
> > E820_TYPE_RAM.
> > Actually, in your original patch, you are walking memblock.memory, which
> > should
> > only contain E820_TYPE_RAM ranges (talking about x86).
> >
> > So I think the below would to the trick as well?
> >
> > @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> > {
> > int i;
> > u64 end;
> > + u64 next = 0;
> >
> > /*
> > * The bootstrap memblock region count maximum is 128 entries
> >
> > @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
> >
> > if (entry->type != E820_TYPE_RAM && entry->type !=
> > E820_TYPE_RESERVED_KERN)
> > continue;
> >
> > +
> > + if (entry->type == E820_TYPE_RAM)
> > + if (next < entry->addr) {
> > + memblock_reserve (next, next + (entry->addr -
> > next));
> > + next = end;
> > + }
> >
> > With the above patch, I can no longer see the issues either.
>
> I double-checked and this change looks good to me.
>
> >
> > Although, there is a difference between this and your original patch.
> > In your original patch, you are just zeroing the pages, while with this one
> > (or with your second patch),
> > we will zero the page in reserve_bootmem_region(), but that function also
> > init
> > some other fields of the struct page:
> >
> > mm_zero_struct_page(page);
> > set_page_links(page, zone, nid, pfn);
> > init_page_count(page);
> > page_mapcount_reset(page);
> > page_cpupid_reset_last(page);
> >
> > So I am not sure we want to bother doing that for pages that are really
> > unreachable.
>
> I think that considering that /proc/kpageflags can check them, some data
> (even if it's trivial) might be better than just zeros.
>
> Here's the updated patch.
> Thanks for the suggestion and testing!
>
> ---
> From: Naoya Horiguchi
> Date: Thu, 14 Jun 2018 14:44:36 +0900
> Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
>
> There is a kernel panic that is triggered when reading /proc/kpageflags
> on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
>
> BUG: unable to handle kernel paging request at fffe
> PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
> Oops: [#1] SMP PTI
> CPU: 2 PID: 1728 Comm: page-types Not tainted
> 4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
> 04/01/2014
> RIP: 0010:stable_page_flags+0x27/0x3c0
> Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89 fc
> 53 48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6 c4 01
> 0f 84 10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
> RSP: 0018:bbd44111fde0 EFLAGS: 00010202
> RAX: fffe RBX: 7fffeff9 RCX:
> RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
> RBP: R08: 0001 R09: 0001
> R10: bbd44111fed8 R11: R12: ed1182fff5c0
> R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
> FS: 7efc4335a500() GS:93a5bfc0() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: fffe CR3: b2a58000 CR4: 001406e0
> Call Trace:
>kpageflags_read+0xc7/0x120
>proc_reg_read+0x3c/0x60
>__vfs_read+0x36/0x170
>vfs_read+0x89/0x130
>ksys_pread64+0x71/0x90
>do_syscall_64+0x5b/0x160
>entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7efc42e75e23
> Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00 90
> 83 3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0 ff ff
> 73 34 c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
>
> According to kernel bisection, this problem became visible due to commit
> f7f99100d8d9 which changes how struct pages are initialized.
>
> Memblock layout affects the pfn ranges covered by node/zone. Consider
> that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
> the default (no memmap= given) memblock layout is like below:
>
> MEMBLOCK configuration:
>memory size = 0x0001fff75c00 reserved size = 0x0300c000
>memory.cnt = 0x4
>memory[0x0] [0x1000-0x0009efff],
> 0x0009e000 bytes on node 0 flags: 0x0
>
[PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
...
> >
> > My concern is that there are a few E820 memory types rather than
> > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > all into memblock.reserved is really acceptable.
>
> Hi Naoya,
>
> Maybe you could just add to memblock.reserved, all unavailable ranges within
> E820_TYPE_RAM.
> Actually, in your original patch, you are walking memblock.memory, which
> should
> only contain E820_TYPE_RAM ranges (talking about x86).
>
> So I think the below would to the trick as well?
>
> @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> {
> int i;
> u64 end;
> + u64 next = 0;
>
> /*
> * The bootstrap memblock region count maximum is 128 entries
>
> @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
>
> if (entry->type != E820_TYPE_RAM && entry->type !=
> E820_TYPE_RESERVED_KERN)
> continue;
>
> +
> + if (entry->type == E820_TYPE_RAM)
> + if (next < entry->addr) {
> + memblock_reserve (next, next + (entry->addr -
> next));
> + next = end;
> + }
>
> With the above patch, I can no longer see the issues either.
I double-checked and this change looks good to me.
>
> Although, there is a difference between this and your original patch.
> In your original patch, you are just zeroing the pages, while with this one
> (or with your second patch),
> we will zero the page in reserve_bootmem_region(), but that function also init
> some other fields of the struct page:
>
> mm_zero_struct_page(page);
> set_page_links(page, zone, nid, pfn);
> init_page_count(page);
> page_mapcount_reset(page);
> page_cpupid_reset_last(page);
>
> So I am not sure we want to bother doing that for pages that are really
> unreachable.
I think that considering that /proc/kpageflags can check them, some data
(even if it's trivial) might be better than just zeros.
Here's the updated patch.
Thanks for the suggestion and testing!
---
From: Naoya Horiguchi
Date: Thu, 14 Jun 2018 14:44:36 +0900
Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
There is a kernel panic that is triggered when reading /proc/kpageflags
on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
BUG: unable to handle kernel paging request at fffe
PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
Oops: [#1] SMP PTI
CPU: 2 PID: 1728 Comm: page-types Not tainted
4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
04/01/2014
RIP: 0010:stable_page_flags+0x27/0x3c0
Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89 fc 53
48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6 c4 01 0f 84
10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
RSP: 0018:bbd44111fde0 EFLAGS: 00010202
RAX: fffe RBX: 7fffeff9 RCX:
RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
RBP: R08: 0001 R09: 0001
R10: bbd44111fed8 R11: R12: ed1182fff5c0
R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
FS: 7efc4335a500() GS:93a5bfc0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: fffe CR3: b2a58000 CR4: 001406e0
Call Trace:
kpageflags_read+0xc7/0x120
proc_reg_read+0x3c/0x60
__vfs_read+0x36/0x170
vfs_read+0x89/0x130
ksys_pread64+0x71/0x90
do_syscall_64+0x5b/0x160
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7efc42e75e23
Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00 90 83
3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 34
c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
According to kernel bisection, this problem became visible due to commit
f7f99100d8d9 which changes how struct pages are initialized.
Memblock layout affects the pfn ranges covered by node/zone. Consider
that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
the default (no memmap= given) memblock layout is like below:
MEMBLOCK configuration:
memory size = 0x0001fff75c00 reserved size = 0x0300c000
memory.cnt = 0x4
memory[0x0] [0x1000-0x0009efff], 0x0009e000
bytes on node 0 flags: 0x0
memory[0x1] [0x0010-0xbffd6fff], 0xbfed7000
bytes on node 0 flags: 0x0
memory[0x2] [0x0001-0x00013fff], 0x4000
bytes on node 0 flags: 0x0
memory[0x3] [0x00014000-0x00023fff], 0x0001
bytes on node 1
[PATCH v2] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
On Thu, Jun 14, 2018 at 07:38:59AM +0200, Oscar Salvador wrote:
> On Thu, Jun 14, 2018 at 05:16:18AM +, Naoya Horiguchi wrote:
...
> >
> > My concern is that there are a few E820 memory types rather than
> > E820_TYPE_RAM and E820_TYPE_RESERVED, and I'm not sure that putting them
> > all into memblock.reserved is really acceptable.
>
> Hi Naoya,
>
> Maybe you could just add to memblock.reserved, all unavailable ranges within
> E820_TYPE_RAM.
> Actually, in your original patch, you are walking memblock.memory, which
> should
> only contain E820_TYPE_RAM ranges (talking about x86).
>
> So I think the below would to the trick as well?
>
> @@ -1248,6 +1276,7 @@ void __init e820__memblock_setup(void)
> {
> int i;
> u64 end;
> + u64 next = 0;
>
> /*
> * The bootstrap memblock region count maximum is 128 entries
>
> @@ -1269,6 +1299,14 @@ void __init e820__memblock_setup(void)
>
> if (entry->type != E820_TYPE_RAM && entry->type !=
> E820_TYPE_RESERVED_KERN)
> continue;
>
> +
> + if (entry->type == E820_TYPE_RAM)
> + if (next < entry->addr) {
> + memblock_reserve (next, next + (entry->addr -
> next));
> + next = end;
> + }
>
> With the above patch, I can no longer see the issues either.
I double-checked and this change looks good to me.
>
> Although, there is a difference between this and your original patch.
> In your original patch, you are just zeroing the pages, while with this one
> (or with your second patch),
> we will zero the page in reserve_bootmem_region(), but that function also init
> some other fields of the struct page:
>
> mm_zero_struct_page(page);
> set_page_links(page, zone, nid, pfn);
> init_page_count(page);
> page_mapcount_reset(page);
> page_cpupid_reset_last(page);
>
> So I am not sure we want to bother doing that for pages that are really
> unreachable.
I think that considering that /proc/kpageflags can check them, some data
(even if it's trivial) might be better than just zeros.
Here's the updated patch.
Thanks for the suggestion and testing!
---
From: Naoya Horiguchi
Date: Thu, 14 Jun 2018 14:44:36 +0900
Subject: [PATCH] x86/e820: put !E820_TYPE_RAM regions into memblock.reserved
There is a kernel panic that is triggered when reading /proc/kpageflags
on the kernel booted with kernel parameter 'memmap=nn[KMG]!ss[KMG]':
BUG: unable to handle kernel paging request at fffe
PGD 9b20e067 P4D 9b20e067 PUD 9b210067 PMD 0
Oops: [#1] SMP PTI
CPU: 2 PID: 1728 Comm: page-types Not tainted
4.17.0-rc6-mm1-v4.17-rc6-180605-0816-00236-g2dfb086ef02c+ #160
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.fc28
04/01/2014
RIP: 0010:stable_page_flags+0x27/0x3c0
Code: 00 00 00 0f 1f 44 00 00 48 85 ff 0f 84 a0 03 00 00 41 54 55 49 89 fc 53
48 8b 57 08 48 8b 2f 48 8d 42 ff 83 e2 01 48 0f 44 c7 <48> 8b 00 f6 c4 01 0f 84
10 03 00 00 31 db 49 8b 54 24 08 4c 89 e7
RSP: 0018:bbd44111fde0 EFLAGS: 00010202
RAX: fffe RBX: 7fffeff9 RCX:
RDX: 0001 RSI: 0202 RDI: ed1182fff5c0
RBP: R08: 0001 R09: 0001
R10: bbd44111fed8 R11: R12: ed1182fff5c0
R13: 000bffd7 R14: 02fff5c0 R15: bbd44111ff10
FS: 7efc4335a500() GS:93a5bfc0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: fffe CR3: b2a58000 CR4: 001406e0
Call Trace:
kpageflags_read+0xc7/0x120
proc_reg_read+0x3c/0x60
__vfs_read+0x36/0x170
vfs_read+0x89/0x130
ksys_pread64+0x71/0x90
do_syscall_64+0x5b/0x160
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7efc42e75e23
Code: 09 00 ba 9f 01 00 00 e8 ab 81 f4 ff 66 2e 0f 1f 84 00 00 00 00 00 90 83
3d 29 0a 2d 00 00 75 13 49 89 ca b8 11 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 34
c3 48 83 ec 08 e8 db d3 01 00 48 89 04 24
According to kernel bisection, this problem became visible due to commit
f7f99100d8d9 which changes how struct pages are initialized.
Memblock layout affects the pfn ranges covered by node/zone. Consider
that we have a VM with 2 NUMA nodes and each node has 4GB memory, and
the default (no memmap= given) memblock layout is like below:
MEMBLOCK configuration:
memory size = 0x0001fff75c00 reserved size = 0x0300c000
memory.cnt = 0x4
memory[0x0] [0x1000-0x0009efff], 0x0009e000
bytes on node 0 flags: 0x0
memory[0x1] [0x0010-0xbffd6fff], 0xbfed7000
bytes on node 0 flags: 0x0
memory[0x2] [0x0001-0x00013fff], 0x4000
bytes on node 0 flags: 0x0
memory[0x3] [0x00014000-0x00023fff], 0x0001
bytes on node 1
