Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1629072 > unrolled thread

Re: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t

Started byDan Williams <dan.j.williams@intel.com>
First post2017-04-23 22:00 +0200
Last post2017-04-25 11:10 +0200
Articles 6 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t Dan Williams <dan.j.williams@intel.com> - 2017-04-23 22:00 +0200
    Re: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-04-24 19:00 +0200
      Re: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t Dan Williams <dan.j.williams@intel.com> - 2017-04-24 20:50 +0200
        Re: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2017-04-25 03:10 +0200
          Re: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t Ingo Molnar <mingo@kernel.org> - 2017-04-25 08:40 +0200
            Re: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-04-25 11:10 +0200

#1629072 — Re: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t

FromDan Williams <dan.j.williams@intel.com>
Date2017-04-23 22:00 +0200
SubjectRe: [tip:x86/mm] x86: Convert the rest of the code to support p4d_t
Message-ID<tzvPQ-4qm-5@gated-at.bofh.it>
On Mon, Mar 27, 2017 at 3:25 AM, tip-bot for Kirill A. Shutemov
<tipbot@zytor.com> wrote:
> Commit-ID:  f2a6a7050109e0a5c7a84c70aa6010f682b2f1ee
> Gitweb:     http://git.kernel.org/tip/f2a6a7050109e0a5c7a84c70aa6010f682b2f1ee
> Author:     Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> AuthorDate: Fri, 17 Mar 2017 21:55:15 +0300
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Mon, 27 Mar 2017 08:56:58 +0200
>
> x86: Convert the rest of the code to support p4d_t
>
> This patch converts x86 to use proper folding of a new (fifth) page table level
> with <asm-generic/pgtable-nop4d.h>.
>
> That's a bit of a kitchen sink patch, but I don't see how to split it further
> without hurting bisectability.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: Dave Hansen <dave.hansen@intel.com>
> Cc: Denys Vlasenko <dvlasenk@redhat.com>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: linux-arch@vger.kernel.org
> Cc: linux-mm@kvack.org
> Link: http://lkml.kernel.org/r/20170317185515.8636-7-kirill.shutemov@linux.intel.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>

Following Ingo's revert of commit 2947ba054a4d "x86/mm/gup: Switch GUP
to the generic get_user_page_fast() implementation", I gave
tip.git/master a try to see if it resolved the original failing
signature I was seeing with the "device-dax" regression test [1] on
next/master. It turns out that there are two failing signatures and I
need to use the revert during the bisect to find this other
regression. This new bisect points to commit f2a6a7050109 "x86:
Convert the rest of the code to support p4d_t" as the culprit.

[   43.194330] BUG: unable to handle kernel paging request at ffff880300000000
[   43.196312] IP: memchr_inv+0x60/0x110
[   43.197352] PGD 317d067
[   43.197354] P4D 317d067
[   43.198110] PUD 3180067
[   43.198872] PMD 33f102067
[   43.199639] PTE 8000000300000060
[   43.200448]
[   43.201878] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[   43.203122] Dumping ftrace buffer:
[   43.204090]    (ftrace buffer empty)
[   43.205067] Modules linked in: ip6t_rpfilter ip6t_REJECT
nf_reject_ipv6 xt_conntrack ebtable_nat ebtable_broute bridge stp llc
ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip
6table_mangle ip6table_raw ip6table_security iptable_nat
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_raw iptable_security ebtable_filter ebtables
ip6table_filter ip6_tables crct10dif_pclmul crc32_pclmul crc32c_intel
ghash_clmulni_intel nd_pmem(O) dax_pmem(O) nd_btt(O) dax(O) tpm_tis
serio_raw nd_e820(O) tpm_tis_core tpm nfit(O) libnvd
imm(O) nfit_test_iomap(O) nfsd nfs_acl
[   43.214795] CPU: 28 PID: 1894 Comm: lt-device-dax Tainted: G
   O    4.11.0-rc2+ #73
[   43.217240] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.9.3-1.fc25 04/01/2014
[   43.219727] task: ffff8803250e9880 task.stack: ffffc90003160000
[   43.221273] RIP: 0010:memchr_inv+0x60/0x110
[   43.222517] RSP: 0018:ffffc90003163aa8 EFLAGS: 00010a83
[   43.223902] RAX: fdfdfdfdfdfdfdfd RBX: 00003fffc0000000 RCX: 0000000008000000
[   43.225598] RDX: 0000000040000000 RSI: 00000000000000fd RDI: ffff880300000000
[   43.227271] RBP: ffffc90003163aa8 R08: 00000000000000fd R09: 0101010101010101
[   43.228952] R10: 0000000000000000 R11: 0000000000000000 R12: 00003ffffffff000
[   43.230640] R13: ffffea0008000000 R14: ffff88033e7e7000 R15: ffffea0008000000
[   43.232342] FS:  00007f3f72935380(0000) GS:ffff880336300000(0000)
knlGS:0000000000000000
[   43.234601] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   43.235964] CR2: ffff880300000000 CR3: 0000000323c7c000 CR4: 00000000000406e0
[   43.237536] Call Trace:
[   43.238409]  remove_pagetable+0x214/0x783
[   43.239519]  ? sparse_remove_one_section+0xeb/0x180
[   43.240749]  vmemmap_free+0x10/0x20
[   43.241776]  sparse_remove_one_section+0xff/0x180
[   43.242979]  __remove_pages+0x33f/0x590
[   43.244054]  ? arch_remove_memory+0x80/0xe0
[   43.245182]  arch_remove_memory+0x80/0xe0
[   43.246296]  devm_memremap_pages_release+0x1af/0x3a0
[   43.247539]  ? devm_memremap_pages_release+0x5/0x3a0
[   43.248784]  release_nodes+0x16d/0x2b0
[   43.249849]  devres_release_all+0x3c/0x60
[   43.250953]  device_release_driver_internal+0x16d/0x210
[   43.252243]  device_release_driver+0x12/0x20
[   43.253386]  unbind_store+0x10f/0x160
[   43.254435]  drv_attr_store+0x25/0x30

[1]: https://github.com/pmem/ndctl/blob/pending/test/device-dax.c

[toc] | [next] | [standalone]

#1629788

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2017-04-24 19:00 +0200
Message-ID<tzPvc-118-19@gated-at.bofh.it>
In reply to#1629072
On Sun, Apr 23, 2017 at 12:56:14PM -0700, Dan Williams wrote:
> On Mon, Mar 27, 2017 at 3:25 AM, tip-bot for Kirill A. Shutemov
> <tipbot@zytor.com> wrote:
> > Commit-ID:  f2a6a7050109e0a5c7a84c70aa6010f682b2f1ee
> > Gitweb:     http://git.kernel.org/tip/f2a6a7050109e0a5c7a84c70aa6010f682b2f1ee
> > Author:     Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > AuthorDate: Fri, 17 Mar 2017 21:55:15 +0300
> > Committer:  Ingo Molnar <mingo@kernel.org>
> > CommitDate: Mon, 27 Mar 2017 08:56:58 +0200
> >
> > x86: Convert the rest of the code to support p4d_t
> >
> > This patch converts x86 to use proper folding of a new (fifth) page table level
> > with <asm-generic/pgtable-nop4d.h>.
> >
> > That's a bit of a kitchen sink patch, but I don't see how to split it further
> > without hurting bisectability.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Acked-by: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Andy Lutomirski <luto@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Brian Gerst <brgerst@gmail.com>
> > Cc: Dave Hansen <dave.hansen@intel.com>
> > Cc: Denys Vlasenko <dvlasenk@redhat.com>
> > Cc: H. Peter Anvin <hpa@zytor.com>
> > Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > Cc: Michal Hocko <mhocko@suse.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: linux-arch@vger.kernel.org
> > Cc: linux-mm@kvack.org
> > Link: http://lkml.kernel.org/r/20170317185515.8636-7-kirill.shutemov@linux.intel.com
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> 
> Following Ingo's revert of commit 2947ba054a4d "x86/mm/gup: Switch GUP
> to the generic get_user_page_fast() implementation", I gave
> tip.git/master a try to see if it resolved the original failing
> signature I was seeing with the "device-dax" regression test [1] on
> next/master. It turns out that there are two failing signatures and I
> need to use the revert during the bisect to find this other
> regression. This new bisect points to commit f2a6a7050109 "x86:
> Convert the rest of the code to support p4d_t" as the culprit.
> 
> [   43.194330] BUG: unable to handle kernel paging request at ffff880300000000

I cannot reproduce the issue.

Could you share your kernel kernel config?

-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1629888

FromDan Williams <dan.j.williams@intel.com>
Date2017-04-24 20:50 +0200
Message-ID<tzRdD-29v-13@gated-at.bofh.it>
In reply to#1629788
On Mon, Apr 24, 2017 at 9:57 AM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> On Sun, Apr 23, 2017 at 12:56:14PM -0700, Dan Williams wrote:
>> On Mon, Mar 27, 2017 at 3:25 AM, tip-bot for Kirill A. Shutemov
>> <tipbot@zytor.com> wrote:
>> > Commit-ID:  f2a6a7050109e0a5c7a84c70aa6010f682b2f1ee
>> > Gitweb:     http://git.kernel.org/tip/f2a6a7050109e0a5c7a84c70aa6010f682b2f1ee
>> > Author:     Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> > AuthorDate: Fri, 17 Mar 2017 21:55:15 +0300
>> > Committer:  Ingo Molnar <mingo@kernel.org>
>> > CommitDate: Mon, 27 Mar 2017 08:56:58 +0200
>> >
>> > x86: Convert the rest of the code to support p4d_t
>> >
>> > This patch converts x86 to use proper folding of a new (fifth) page table level
>> > with <asm-generic/pgtable-nop4d.h>.
>> >
>> > That's a bit of a kitchen sink patch, but I don't see how to split it further
>> > without hurting bisectability.
>> >
>> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> > Acked-by: Thomas Gleixner <tglx@linutronix.de>
>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>> > Cc: Andy Lutomirski <luto@amacapital.net>
>> > Cc: Andy Lutomirski <luto@kernel.org>
>> > Cc: Arnd Bergmann <arnd@arndb.de>
>> > Cc: Borislav Petkov <bp@alien8.de>
>> > Cc: Brian Gerst <brgerst@gmail.com>
>> > Cc: Dave Hansen <dave.hansen@intel.com>
>> > Cc: Denys Vlasenko <dvlasenk@redhat.com>
>> > Cc: H. Peter Anvin <hpa@zytor.com>
>> > Cc: Josh Poimboeuf <jpoimboe@redhat.com>
>> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> > Cc: Michal Hocko <mhocko@suse.com>
>> > Cc: Peter Zijlstra <peterz@infradead.org>
>> > Cc: linux-arch@vger.kernel.org
>> > Cc: linux-mm@kvack.org
>> > Link: http://lkml.kernel.org/r/20170317185515.8636-7-kirill.shutemov@linux.intel.com
>> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
>>
>> Following Ingo's revert of commit 2947ba054a4d "x86/mm/gup: Switch GUP
>> to the generic get_user_page_fast() implementation", I gave
>> tip.git/master a try to see if it resolved the original failing
>> signature I was seeing with the "device-dax" regression test [1] on
>> next/master. It turns out that there are two failing signatures and I
>> need to use the revert during the bisect to find this other
>> regression. This new bisect points to commit f2a6a7050109 "x86:
>> Convert the rest of the code to support p4d_t" as the culprit.
>>
>> [   43.194330] BUG: unable to handle kernel paging request at ffff880300000000
>
> I cannot reproduce the issue.
>
> Could you share your kernel kernel config?

Kernel config available here:
https://gist.github.com/djbw/2b99a014396cca9f4406586573be98c0

[toc] | [prev] | [next] | [standalone]

#1630137

From"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date2017-04-25 03:10 +0200
Message-ID<tzX9n-6aR-11@gated-at.bofh.it>
In reply to#1629888
On Mon, Apr 24, 2017 at 11:42:34AM -0700, Dan Williams wrote:
> >
> > I cannot reproduce the issue.
> >
> > Could you share your kernel kernel config?
> 
> Kernel config available here:
> https://gist.github.com/djbw/2b99a014396cca9f4406586573be98c0

Patch below should help. Please check.

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index a242139df8fe..745e5e183169 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -962,7 +962,7 @@ remove_pud_table(pud_t *pud_start, unsigned long addr, unsigned long end,
 			continue;
 		}
 
-		pmd_base = (pmd_t *)pud_page_vaddr(*pud);
+		pmd_base = pmd_offset(pud, 0);
 		remove_pmd_table(pmd_base, addr, next, direct);
 		free_pmd_table(pmd_base, pud);
 	}
@@ -988,7 +988,7 @@ remove_p4d_table(p4d_t *p4d_start, unsigned long addr, unsigned long end,
 
 		BUILD_BUG_ON(p4d_large(*p4d));
 
-		pud_base = (pud_t *)p4d_page_vaddr(*p4d);
+		pud_base = pud_offset(p4d, 0);
 		remove_pud_table(pud_base, addr, next, direct);
 		free_pud_table(pud_base, p4d);
 	}
@@ -1013,7 +1013,7 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct)
 		if (!pgd_present(*pgd))
 			continue;
 
-		p4d = (p4d_t *)pgd_page_vaddr(*pgd);
+		p4d = p4d_offset(pgd, 0);
 		remove_p4d_table(p4d, addr, next, direct);
 	}
 
-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1630211

FromIngo Molnar <mingo@kernel.org>
Date2017-04-25 08:40 +0200
Message-ID<tA2iK-148-25@gated-at.bofh.it>
In reply to#1630137
* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote:

> On Mon, Apr 24, 2017 at 11:42:34AM -0700, Dan Williams wrote:
> > >
> > > I cannot reproduce the issue.
> > >
> > > Could you share your kernel kernel config?
> > 
> > Kernel config available here:
> > https://gist.github.com/djbw/2b99a014396cca9f4406586573be98c0
> 
> Patch below should help. Please check.
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index a242139df8fe..745e5e183169 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -962,7 +962,7 @@ remove_pud_table(pud_t *pud_start, unsigned long addr, unsigned long end,
>  			continue;
>  		}
>  
> -		pmd_base = (pmd_t *)pud_page_vaddr(*pud);
> +		pmd_base = pmd_offset(pud, 0);
>  		remove_pmd_table(pmd_base, addr, next, direct);
>  		free_pmd_table(pmd_base, pud);
>  	}
> @@ -988,7 +988,7 @@ remove_p4d_table(p4d_t *p4d_start, unsigned long addr, unsigned long end,
>  
>  		BUILD_BUG_ON(p4d_large(*p4d));
>  
> -		pud_base = (pud_t *)p4d_page_vaddr(*p4d);
> +		pud_base = pud_offset(p4d, 0);
>  		remove_pud_table(pud_base, addr, next, direct);
>  		free_pud_table(pud_base, p4d);
>  	}
> @@ -1013,7 +1013,7 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct)
>  		if (!pgd_present(*pgd))
>  			continue;
>  
> -		p4d = (p4d_t *)pgd_page_vaddr(*pgd);
> +		p4d = p4d_offset(pgd, 0);
>  		remove_p4d_table(p4d, addr, next, direct);
>  	}

Type casts are dangerous, chapter 378. I missed them when applying the patches.

Could you please double-check all other type casts introduced by the 5-level 
paging patches, to make sure it's all using natural type conversions?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1630318

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2017-04-25 11:10 +0200
Message-ID<tA4DT-2H8-3@gated-at.bofh.it>
In reply to#1630211
On Tue, Apr 25, 2017 at 08:38:42AM +0200, Ingo Molnar wrote:
> 
> * Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote:
> 
> > On Mon, Apr 24, 2017 at 11:42:34AM -0700, Dan Williams wrote:
> > > >
> > > > I cannot reproduce the issue.
> > > >
> > > > Could you share your kernel kernel config?
> > > 
> > > Kernel config available here:
> > > https://gist.github.com/djbw/2b99a014396cca9f4406586573be98c0
> > 
> > Patch below should help. Please check.
> > 
> > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> > index a242139df8fe..745e5e183169 100644
> > --- a/arch/x86/mm/init_64.c
> > +++ b/arch/x86/mm/init_64.c
> > @@ -962,7 +962,7 @@ remove_pud_table(pud_t *pud_start, unsigned long addr, unsigned long end,
> >  			continue;
> >  		}
> >  
> > -		pmd_base = (pmd_t *)pud_page_vaddr(*pud);
> > +		pmd_base = pmd_offset(pud, 0);
> >  		remove_pmd_table(pmd_base, addr, next, direct);
> >  		free_pmd_table(pmd_base, pud);
> >  	}
> > @@ -988,7 +988,7 @@ remove_p4d_table(p4d_t *p4d_start, unsigned long addr, unsigned long end,
> >  
> >  		BUILD_BUG_ON(p4d_large(*p4d));
> >  
> > -		pud_base = (pud_t *)p4d_page_vaddr(*p4d);
> > +		pud_base = pud_offset(p4d, 0);
> >  		remove_pud_table(pud_base, addr, next, direct);
> >  		free_pud_table(pud_base, p4d);
> >  	}
> > @@ -1013,7 +1013,7 @@ remove_pagetable(unsigned long start, unsigned long end, bool direct)
> >  		if (!pgd_present(*pgd))
> >  			continue;
> >  
> > -		p4d = (p4d_t *)pgd_page_vaddr(*pgd);
> > +		p4d = p4d_offset(pgd, 0);
> >  		remove_p4d_table(p4d, addr, next, direct);
> >  	}
> 
> Type casts are dangerous, chapter 378. I missed them when applying the patches.
> 
> Could you please double-check all other type casts introduced by the 5-level 
> paging patches, to make sure it's all using natural type conversions?

I looked though at the rest of (p4d_t) and (p4d_t *) casts and they look
okay: under pgd_none()/!pgd_preset() or within #if PTRS_PER_P4d > 1.

I'll prepare proper patch.

-- 
 Kirill A. Shutemov

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web