Re: [PATCH 04/16] MIPS: use the generic get_user_pages_fast code
Hi, On Tue, Jun 25, 2019 at 04:37:03PM +0200, Christoph Hellwig wrote: > The mips code is mostly equivalent to the generic one, minus various > bugfixes and an arch override for gup_fast_permitted. > > Note that this defines ARCH_HAS_PTE_SPECIAL for mips as mips has > pte_special and pte_mkspecial implemented and used in the existing > gup code. They are no-op stubs, though which makes me a little unsure > if this is really right thing to do. > > Note that this also adds back a missing cpu_has_dc_aliases check for > __get_user_pages_fast, which the old code was only doing for > get_user_pages_fast. This clearly looks like an oversight, as any > condition that makes get_user_pages_fast unsafe also applies to > __get_user_pages_fast. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Jason Gunthorpe This patch causes all mips images (mips, mips64, mipsel, mipsel64) to crash when booting in qemu. Unfortunately the patch can not be reverted easily since there are context changes, causing build failures after the revert, so I can not verify if this is the only problem. Crash log (same for all variants): ... Run /sbin/init as init process BUG: Bad page map in process mount pte:00b70401 pmd:8e5dc000 page:80c24880 refcount:1 mapcount:-1 mapping: index:0x0 flags: 0x1000(reserved) raw: 1000 80c24884 80c24884 fffe 0001 page dumped because: bad pte addr:(ptrval) vm_flags:04044411 anon_vma:(ptrval) mapping:(ptrval) index:0 qemu-system-mips: terminating on signal 15 from pid 13034 (/bin/bash) Guenter --- bisect log: # bad: [48568d8c7f479ec45b9c3d02b4b1895f3ef61a03] Add linux-next specific files for 20190628 # good: [4b972a01a7da614b4796475f933094751a295a2f] Linux 5.2-rc6 git bisect start 'HEAD' 'v5.2-rc6' # good: [89a77c9176fe88f68c3bf7bd255cfea6797258d4] Merge remote-tracking branch 'crypto/master' git bisect good 89a77c9176fe88f68c3bf7bd255cfea6797258d4 # good: [2cedca636ad73ed838bd636685b245404e490c73] Merge remote-tracking branch 'security/next-testing' git bisect good 2cedca636ad73ed838bd636685b245404e490c73 # good: [ea260819fdc2f8a64e6c87f3ad80ecc5e4015921] Merge remote-tracking branch 'char-misc/char-misc-next' git bisect good ea260819fdc2f8a64e6c87f3ad80ecc5e4015921 # good: [aca42ca2a32eacf804ac56a33526f049debc8ec0] Merge remote-tracking branch 'rpmsg/for-next' git bisect good aca42ca2a32eacf804ac56a33526f049debc8ec0 # good: [f4cd0c7f3c07876f7173b5306e974644c6eec141] Merge remote-tracking branch 'pidfd/for-next' git bisect good f4cd0c7f3c07876f7173b5306e974644c6eec141 # bad: [09c57a8ab1fc3474b4a620247a0f9e3ac61c4cfe] mm/sparsemem: support sub-section hotplug git bisect bad 09c57a8ab1fc3474b4a620247a0f9e3ac61c4cfe # good: [aaffcf10880c363870413c5cdee5dfb6a923e9ae] mm: memcontrol: dump memory.stat during cgroup OOM git bisect good aaffcf10880c363870413c5cdee5dfb6a923e9ae # bad: [81d90bb2d2784258ed7c0762ecf34d4665198bad] um: switch to generic version of pte allocation git bisect bad 81d90bb2d2784258ed7c0762ecf34d4665198bad # bad: [dadae650472841f004882a2409aa844e37809c60] sparc64-add-the-missing-pgd_page-definition-fix git bisect bad dadae650472841f004882a2409aa844e37809c60 # good: [d1edd06c6ac8c8c49345ff34de1c72ee571f3f7b] mm: memcg/slab: stop setting page->mem_cgroup pointer for slab pages git bisect good d1edd06c6ac8c8c49345ff34de1c72ee571f3f7b # good: [b1ceaacca9e63794bd3f574c928e7e6aca01bce7] mm: simplify gup_fast_permitted git bisect good b1ceaacca9e63794bd3f574c928e7e6aca01bce7 # bad: [59f238b3353caf43b118e1bb44010aa1abd56d7f] sh: add the missing pud_page definition git bisect bad 59f238b3353caf43b118e1bb44010aa1abd56d7f # bad: [93a184240a74cb0242b9b970f0bc018c4fdf24fd] MIPS: use the generic get_user_pages_fast code git bisect bad 93a184240a74cb0242b9b970f0bc018c4fdf24fd # good: [7c6a77cff73127e9495e345a0903d55b1b0fb323] mm: lift the x86_32 PAE version of gup_get_pte to common code git bisect good 7c6a77cff73127e9495e345a0903d55b1b0fb323 # first bad commit: [93a184240a74cb0242b9b970f0bc018c4fdf24fd] MIPS: use the generic get_user_pages_fast code
Re: [PATCH 04/16] MIPS: use the generic get_user_pages_fast code
On Fri, Jun 21, 2019 at 11:05:42AM -0300, Jason Gunthorpe wrote:
> Today this check is only being done on the get_user_pages_fast() -
> after this patch it is also done for __get_user_pages_fast().
>
> Which means __get_user_pages_fast is now non-functional on a range of
> MIPS CPUs, but that seems OK as far as I can tell, so:
> However, looks to me like this patch is also a bug fix for this:
Yes.
> > - pgdp = pgd_offset(mm, addr);
> > - do {
> > - pgd_t pgd = *pgdp;
> > -
> > - next = pgd_addr_end(addr, end);
> > - if (pgd_none(pgd))
> > - goto slow;
> > - if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE,
> > - pages, ))
>
> This is different too, the core code has a p4d layer, but I see that
> whole thing gets NOP'd by the compiler as mips uses pgtable-nop4d.h -
> right?
Exactly.
Re: [PATCH 04/16] MIPS: use the generic get_user_pages_fast code
On Tue, Jun 11, 2019 at 04:40:50PM +0200, Christoph Hellwig wrote:
> diff --git a/arch/mips/include/asm/pgtable.h b/arch/mips/include/asm/pgtable.h
> index 4ccb465ef3f2..7d27194e3b45 100644
> +++ b/arch/mips/include/asm/pgtable.h
> @@ -20,6 +20,7 @@
> #include
> #include
> #include
> +#include
>
> struct mm_struct;
> struct vm_area_struct;
> @@ -626,6 +627,8 @@ static inline pmd_t pmdp_huge_get_and_clear(struct
> mm_struct *mm,
>
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> +#define gup_fast_permitted(start, end) (!cpu_has_dc_aliases)
> +
Today this check is only being done on the get_user_pages_fast() -
after this patch it is also done for __get_user_pages_fast().
Which means __get_user_pages_fast is now non-functional on a range of
MIPS CPUs, but that seems OK as far as I can tell, so:
Reviewed-by: Jason Gunthorpe
However, looks to me like this patch is also a bug fix for this:
commit 5b167c123b3c3582f62cf1896465019bc40fe526
Author: Kamal Dasu
Date: Fri Jun 14 17:10:03 2013 +
MIPS: Fix get_user_page_fast() for mips with cache alias
get_user_pages_fast() is missing cache flushes for MIPS platforms with
cache aliases. Filesystem failures observed with DirectIO operations due
to missing flush_anon_page() that use page coloring logic to work with
cache aliases. This fix falls through to take slow_irqon path that calls
get_user_pages() that has required logic for platforms where
cpu_has_dc_aliases is true.
> - pgdp = pgd_offset(mm, addr);
> - do {
> - pgd_t pgd = *pgdp;
> -
> - next = pgd_addr_end(addr, end);
> - if (pgd_none(pgd))
> - goto slow;
> - if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE,
> -pages, ))
This is different too, the core code has a p4d layer, but I see that
whole thing gets NOP'd by the compiler as mips uses pgtable-nop4d.h -
right?
Jason
