| Message ID | 20190219103233.443069009@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | generic mmu_gather patches | expand |
Hoi Peter, On Tue, Feb 19, 2019 at 11:35 AM Peter Zijlstra <peterz@infradead.org> wrote: > Generic mmu_gather provides everything SH needs (range tracking and > cache coherency). > > Cc: Will Deacon <will.deacon@arm.com> > Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Nick Piggin <npiggin@gmail.com> > Cc: Yoshinori Sato <ysato@users.sourceforge.jp> > Cc: Rich Felker <dalias@libc.org> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> I got remote access to an SH7722-based Migo-R again, which spews a long sequence of BUGs during userspace startup. I've bisected this to commit c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). Do you have a clue? Thanks! mount: mounting none on /dev failed: No such device BUG: Bad page state in process grep pfn:0e8c1 page:8fb50820 count:0 mapcount:2 mapping:8f403480 index:0x0 (null) flags: 0xc000000() raw: 0c000000 00000100 00000200 8f403480 00000000 00000000 00000001 00000000 page dumped because: non-NULL mapping CPU: 0 PID: 765 Comm: grep Tainted: G W 5.1.0-rc3-migor-00024-gc5b27a889da92f4a #31 Stack: (0x8e8a7d4c to 0x8e8a8000) 7d40: 8c072e90 8fb50820 8fbf400c 00000000 8c0734bc 7d60: 00000200 8fbf4010 00000001 00000002 8fbf4000 00000002 8fbf400c 8e8a7d9c 7d80: 0030f231 00000100 8e8a7da0 00000000 8c2a8a38 8c2a8c0c 8c2a8a94 8c339fa0 7da0: 8fb052a4 8fb052a4 9165dc07 8c0744ea 00000010 00000000 8c0027ac 00000000 7dc0: 8c073ad0 8e8a7df8 8fb054a0 8fb50980 8c07ab54 000000aa 00000011 8e8b52b4 7de0: 00010000 000000aa 8c2a8a38 8fb50820 8c0027ac 00000000 8fb50824 8fb054a4 7e00: 9165dc07 8c0928e0 8c210e48 8c28900c 8e8a7ecc 8e8a7ee4 8c07a958 00000000 7e20: 8e8b5000 8c09296e 8e8a7ed4 8c08f414 8c28900c 8e8a7e60 8e8a7ecc 00000000 7e40: 9165dc07 8c099d08 8ef16178 8c339f60 00000001 00000000 8c0950bc 8c0027ac 7e60: 8c08b00e 7ba1bfff 40000000 7ba1c000 00000000 7ba1bfff 00000000 ffffffff 7e80: 8c08b1ec 8ef16140 8c08ae58 00000000 8ef16140 7b9fb000 8c094fe4 9165dc07 7ea0: 8c0929e6 8e8a7ecc 00000001 8e8a7ecc 8c092b3e 00100000 00000001 8e8a7ecc 7ec0: 8c090cfe 00000000 8ef16020 8f4b78e0 ffffffff ffffffff 8fb50501 00000001 7ee0: 8e8b5000 8e8b5000 00000000 00000008 8fb74f80 8fb74fa0 8fb74f00 8fb74f20 7f00: 8fb74f40 8fb74f60 8fb76c00 8fb76c20 9165dc07 8c0128a2 00000000 00000000 7f20: 8f4b7918 8f4b78e0 00000000 8f4b78e0 8c016326 8ea34f00 8ea3503c 8ea34f00 7f40: 8ea351bc 00000000 8ea3503c 8ea3507c 8c297c1c 8f4a717c 8f4f37a0 9165dc07 7f60: 8c016a58 7ba1bbe4 00000000 00000000 00000000 8eedd720 8eedd6e0 00000000 7f80: 8c016ac6 00000000 00000071 00000100 8c016abc 8c006242 00000000 00427b38 7fa0: 004c025c 00000000 00427b38 004c025c 000000fc 00000000 000000fc fffff000 7fc0: 00000000 00098d9c 00000000 004ac6f4 7ba1be94 00000000 00000000 7ba1bbe4 7fe0: 7ba1bbe4 00427b4e 0040a978 00000101 004c5450 00000022 ffffd000 00000044 Call trace: [<(ptrval)>] free_pcppages_bulk+0x114/0x33c [<(ptrval)>] free_unref_page_list+0xae/0x10c [<(ptrval)>] arch_local_irq_restore+0x0/0x24 [<(ptrval)>] free_unref_page_commit.isra.24+0x0/0x74 [<(ptrval)>] release_pages+0x1fc/0x2d0 [<(ptrval)>] arch_local_irq_restore+0x0/0x24 [<(ptrval)>] tlb_flush_mmu_free+0x30/0x4c [<(ptrval)>] down_read+0x0/0xc [<(ptrval)>] release_pages+0x0/0x2d0 [<(ptrval)>] tlb_flush_mmu+0x72/0xc0 [<(ptrval)>] remove_vma+0x0/0x48 [<(ptrval)>] kmem_cache_free+0x34/0x90 [<(ptrval)>] unlink_anon_vmas+0xd8/0x150 [<(ptrval)>] arch_local_irq_restore+0x0/0x24 [<(ptrval)>] free_pgd_range+0x1b6/0x324 [<(ptrval)>] free_pgtables+0x70/0xb4 [<(ptrval)>] free_pgd_range+0x0/0x324 [<(ptrval)>] unlink_anon_vmas+0x0/0x150 [<(ptrval)>] arch_tlb_finish_mmu+0x2a/0x74 [<(ptrval)>] tlb_finish_mmu+0x1a/0x38 [<(ptrval)>] exit_mmap+0xa2/0x16c [<(ptrval)>] mmput+0x2a/0x80 [<(ptrval)>] do_exit+0x186/0x85c [<(ptrval)>] do_group_exit+0x2c/0x90 [<(ptrval)>] sys_exit_group+0xa/0x10 [<(ptrval)>] sys_exit_group+0x0/0x10 [<(ptrval)>] syscall_call+0x18/0x1e Disabling lock debugging due to kernel taint BUG: Bad page state in process rcS pfn:0ee78 page:8fb5bf00 count:0 mapcount:2 mapping:8f403480 index:0x0 (null) flags: 0xc000000() raw: 0c000000 00000100 00000200 8f403480 00000000 00000000 00000001 00000000 page dumped because: non-NULL mapping CPU: 0 PID: 763 Comm: rcS Tainted: G B W 5.1.0-rc3-migor-00024-gc5b27a889da92f4a #31 Stack: (0x8ef55e54 to 0x8ef56000) 5e40: 8c072e90 8fb5bf00 8fbf400c 5e60: 00000000 8c0734bc 00000200 8fbf4010 00000001 00000002 8fbf4000 00000002 5e80: 8fbf400c 00989680 0030f231 00000100 8ef55ea8 00000000 8c2a8a38 00000028 5ea0: 00000000 8c29d340 8fb05344 8fb05344 b362ad67 8c074416 8f4f38e0 8c0027ac 5ec0: 8e9125e0 00000000 00000000 8c0027ac 8fb50760 8c0a59e4 8e9125a0 00000180 5ee0: 00000010 8c0a5a9a 8f4a69e0 8f01ad00 8f4f38e0 8f01b21c 8e9125a0 8c09f526 5f00: 8f414cf0 8f01b120 8f4f38e0 8c0281e0 8c20f7dc 8f4a6cdc 00000000 8f4a6cec 5f20: 8c003c18 00401f80 004c6aa0 00000000 8ef55fa4 8ef55fa4 09000080 8c28900c 5f40: 00000000 b362ad67 8c00bbde 8ef55fe4 8f4b7aa0 00000001 09000000 8f4a6c1c 5f60: 004c6ab4 00000055 8c215f8c ffff8bce 00000004 00000009 00003f10 8f4f38e0 5f80: b362ad67 8c0060f8 00401f80 004c6aa0 00000000 00000000 40000000 00000100 5fa0: 8ef54000 00000000 00000450 00000002 00000006 00000003 00000000 004c6ab4 5fc0: 00000000 7bdf069c 004c61d4 00000003 004c61bc 7bdf0670 004c6aa0 00401f80 5fe0: 7bdf069c 00401f94 0047d1cc 00000001 004c5450 00000043 0000000c 00000044 Call trace: [<(ptrval)>] free_pcppages_bulk+0x114/0x33c [<(ptrval)>] free_unref_page+0x4a/0x70 [<(ptrval)>] arch_local_irq_restore+0x0/0x24 [<(ptrval)>] arch_local_irq_restore+0x0/0x24 [<(ptrval)>] free_pipe_info+0x60/0x84 [<(ptrval)>] pipe_release+0x92/0xb8 [<(ptrval)>] __fput+0x3e/0x120 [<(ptrval)>] task_work_run+0x78/0xa8 [<(ptrval)>] _cond_resched+0x0/0x54 [<(ptrval)>] do_notify_resume+0xd4/0x4c8 [<(ptrval)>] do_page_fault+0xda/0x2a8 [<(ptrval)>] resume_userspace+0x0/0x10 BUG: Bad page state in process rcS pfn:0e8b2 page:8fb50640 count:0 mapcount:1026 mapping:8f403480 index:0x0 (null) flags: 0xc000000() raw: 0c000000 00000100 00000200 8f403480 00000000 00000000 00000401 00000000 page dumped because: non-NULL mapping CPU: 0 PID: 763 Comm: rcS Tainted: G B W 5.1.0-rc3-migor-00024-gc5b27a889da92f4a #31 Stack: (0x8ef55e54 to 0x8ef56000) 5e40: 8c072e90 8fb50640 8fbf400c 5e60: 00000000 8c0734bc 00000200 8fbf4010 00000001 00000001 8fbf4000 00000001 5e80: 8fbf400c 00989680 0030f231 00000100 8ef55ea8 00000000 8c2a8a38 00000028 5ea0: 00000000 8c29d340 8fb05344 8fb05344 b362ad67 8c074416 8f4f38e0 8c0027ac 5ec0: 8e9125e0 00000000 00000000 8c0027ac 8fb50760 8c0a59e4 8e9125a0 00000180 5ee0: 00000010 8c0a5a9a 8f4a69e0 8f01ad00 8f4f38e0 8f01b21c 8e9125a0 8c09f526 5f00: 8f414cf0 8f01b120 8f4f38e0 8c0281e0 8c20f7dc 8f4a6cdc 00000000 8f4a6cec 5f20: 8c003c18 00401f80 004c6aa0 00000000 8ef55fa4 8ef55fa4 09000080 8c28900c 5f40: 00000000 b362ad67 8c00bbde 8ef55fe4 8f4b7aa0 00000001 09000000 8f4a6c1c 5f60: 004c6ab4 00000055 8c215f8c ffff8bce 00000004 00000009 00003f10 8f4f38e0 5f80: b362ad67 8c0060f8 00401f80 004c6aa0 00000000 00000000 40000000 00000100 5fa0: 8ef54000 00000000 00000450 00000002 00000006 00000003 00000000 004c6ab4 5fc0: 00000000 7bdf069c 004c61d4 00000003 004c61bc 7bdf0670 004c6aa0 00401f80 5fe0: 7bdf069c 00401f94 0047d1cc 00000001 004c5450 00000043 0000000c 00000044 Call trace: [<(ptrval)>] free_pcppages_bulk+0x114/0x33c [<(ptrval)>] free_unref_page+0x4a/0x70 [<(ptrval)>] arch_local_irq_restore+0x0/0x24 [<(ptrval)>] arch_local_irq_restore+0x0/0x24 [<(ptrval)>] free_pipe_info+0x60/0x84 [<(ptrval)>] pipe_release+0x92/0xb8 [<(ptrval)>] __fput+0x3e/0x120 [<(ptrval)>] task_work_run+0x78/0xa8 [<(ptrval)>] _cond_resched+0x0/0x54 [<(ptrval)>] do_notify_resume+0xd4/0x4c8 [<(ptrval)>] do_page_fault+0xda/0x2a8 [<(ptrval)>] resume_userspace+0x0/0x10 ... > --- > arch/sh/include/asm/pgalloc.h | 7 ++ > arch/sh/include/asm/tlb.h | 130 ------------------------------------------ > 2 files changed, 8 insertions(+), 129 deletions(-) > > --- a/arch/sh/include/asm/pgalloc.h > +++ b/arch/sh/include/asm/pgalloc.h > @@ -72,6 +72,15 @@ do { \ > tlb_remove_page((tlb), (pte)); \ > } while (0) > > +#if CONFIG_PGTABLE_LEVELS > 2 > +#define __pmd_free_tlb(tlb, pmdp, addr) \ > +do { \ > + struct page *page = virt_to_page(pmdp); \ > + pgtable_pmd_page_dtor(page); \ > + tlb_remove_page((tlb), page); \ > +} while (0); > +#endif > + > static inline void check_pgt_cache(void) > { > quicklist_trim(QUICK_PT, NULL, 25, 16); > --- a/arch/sh/include/asm/tlb.h > +++ b/arch/sh/include/asm/tlb.h > @@ -11,131 +11,8 @@ > > #ifdef CONFIG_MMU > #include <linux/swap.h> > -#include <asm/pgalloc.h> > -#include <asm/tlbflush.h> > -#include <asm/mmu_context.h> > - > -/* > - * TLB handling. This allows us to remove pages from the page > - * tables, and efficiently handle the TLB issues. > - */ > -struct mmu_gather { > - struct mm_struct *mm; > - unsigned int fullmm; > - unsigned long start, end; > -}; > > -static inline void init_tlb_gather(struct mmu_gather *tlb) > -{ > - tlb->start = TASK_SIZE; > - tlb->end = 0; > - > - if (tlb->fullmm) { > - tlb->start = 0; > - tlb->end = TASK_SIZE; > - } > -} > - > -static inline void > -arch_tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, > - unsigned long start, unsigned long end) > -{ > - tlb->mm = mm; > - tlb->start = start; > - tlb->end = end; > - tlb->fullmm = !(start | (end+1)); > - > - init_tlb_gather(tlb); > -} > - > -static inline void > -arch_tlb_finish_mmu(struct mmu_gather *tlb, > - unsigned long start, unsigned long end, bool force) > -{ > - if (tlb->fullmm || force) > - flush_tlb_mm(tlb->mm); > - > - /* keep the page table cache within bounds */ > - check_pgt_cache(); > -} > - > -static inline void > -tlb_remove_tlb_entry(struct mmu_gather *tlb, pte_t *ptep, unsigned long address) > -{ > - if (tlb->start > address) > - tlb->start = address; > - if (tlb->end < address + PAGE_SIZE) > - tlb->end = address + PAGE_SIZE; > -} > - > -#define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \ > - tlb_remove_tlb_entry(tlb, ptep, address) > - > -/* > - * In the case of tlb vma handling, we can optimise these away in the > - * case where we're doing a full MM flush. When we're doing a munmap, > - * the vmas are adjusted to only cover the region to be torn down. > - */ > -static inline void > -tlb_start_vma(struct mmu_gather *tlb, struct vm_area_struct *vma) > -{ > - if (!tlb->fullmm) > - flush_cache_range(vma, vma->vm_start, vma->vm_end); > -} > - > -static inline void > -tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vma) > -{ > - if (!tlb->fullmm && tlb->end) { > - flush_tlb_range(vma, tlb->start, tlb->end); > - init_tlb_gather(tlb); > - } > -} > - > -static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb) > -{ > -} > - > -static inline void tlb_flush_mmu_free(struct mmu_gather *tlb) > -{ > -} > - > -static inline void tlb_flush_mmu(struct mmu_gather *tlb) > -{ > -} > - > -static inline int __tlb_remove_page(struct mmu_gather *tlb, struct page *page) > -{ > - free_page_and_swap_cache(page); > - return false; /* avoid calling tlb_flush_mmu */ > -} > - > -static inline void tlb_remove_page(struct mmu_gather *tlb, struct page *page) > -{ > - __tlb_remove_page(tlb, page); > -} > - > -static inline bool __tlb_remove_page_size(struct mmu_gather *tlb, > - struct page *page, int page_size) > -{ > - return __tlb_remove_page(tlb, page); > -} > - > -static inline void tlb_remove_page_size(struct mmu_gather *tlb, > - struct page *page, int page_size) > -{ > - return tlb_remove_page(tlb, page); > -} > - > -static inline void tlb_change_page_size(struct mmu_gather *tlb, unsigned int page_size) > -{ > -} > - > -#define pte_free_tlb(tlb, ptep, addr) pte_free((tlb)->mm, ptep) > -#define pmd_free_tlb(tlb, pmdp, addr) pmd_free((tlb)->mm, pmdp) > -#define pud_free_tlb(tlb, pudp, addr) pud_free((tlb)->mm, pudp) > - > -#define tlb_migrate_finish(mm) do { } while (0) > +#include <asm-generic/tlb.h> > > #if defined(CONFIG_CPU_SH4) || defined(CONFIG_SUPERH64) > extern void tlb_wire_entry(struct vm_area_struct *, unsigned long, pte_t); > @@ -155,11 +32,6 @@ static inline void tlb_unwire_entry(void > > #else /* CONFIG_MMU */ > > -#define tlb_start_vma(tlb, vma) do { } while (0) > -#define tlb_end_vma(tlb, vma) do { } while (0) > -#define __tlb_remove_tlb_entry(tlb, pte, address) do { } while (0) > -#define tlb_flush(tlb) do { } while (0) > - > #include <asm-generic/tlb.h> > > #endif /* CONFIG_MMU */ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Dec 03, 2019 at 12:19:00PM +0100, Geert Uytterhoeven wrote: > Hoi Peter, > > On Tue, Feb 19, 2019 at 11:35 AM Peter Zijlstra <peterz@infradead.org> wrote: > > Generic mmu_gather provides everything SH needs (range tracking and > > cache coherency). > > > > Cc: Will Deacon <will.deacon@arm.com> > > Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: Nick Piggin <npiggin@gmail.com> > > Cc: Yoshinori Sato <ysato@users.sourceforge.jp> > > Cc: Rich Felker <dalias@libc.org> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > I got remote access to an SH7722-based Migo-R again, which spews a long > sequence of BUGs during userspace startup. I've bisected this to commit > c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). Whoopsy.. also, is this really the first time anybody booted an SH kernel in over a year ?!? > Do you have a clue? Does the below help? diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h index 22d968bfe9bb..73a2c00de6c5 100644 --- a/arch/sh/include/asm/pgalloc.h +++ b/arch/sh/include/asm/pgalloc.h @@ -36,9 +36,8 @@ do { \ #if CONFIG_PGTABLE_LEVELS > 2 #define __pmd_free_tlb(tlb, pmdp, addr) \ do { \ - struct page *page = virt_to_page(pmdp); \ - pgtable_pmd_page_dtor(page); \ - tlb_remove_page((tlb), page); \ + pgtable_pmd_page_dtor(pmdp); \ + tlb_remove_page((tlb), (pmdp)); \ } while (0); #endif
Hoi Peter, On Wed, Dec 4, 2019 at 11:48 AM Peter Zijlstra <peterz@infradead.org> wrote: > On Tue, Dec 03, 2019 at 12:19:00PM +0100, Geert Uytterhoeven wrote: > > On Tue, Feb 19, 2019 at 11:35 AM Peter Zijlstra <peterz@infradead.org> wrote: > > > Generic mmu_gather provides everything SH needs (range tracking and > > > cache coherency). > > > > > > Cc: Will Deacon <will.deacon@arm.com> > > > Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > Cc: Nick Piggin <npiggin@gmail.com> > > > Cc: Yoshinori Sato <ysato@users.sourceforge.jp> > > > Cc: Rich Felker <dalias@libc.org> > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > > I got remote access to an SH7722-based Migo-R again, which spews a long > > sequence of BUGs during userspace startup. I've bisected this to commit > > c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). > > Whoopsy.. also, is this really the first time anybody booted an SH > kernel in over a year ?!? Nah, but the v5.4-rc3 I booted recently on qemu -M r2d had CONFIG_PGTABLE_LEVELS=2, so it didn't show the problem. > > Do you have a clue? > > Does the below help? Unfortunately not. > diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h > index 22d968bfe9bb..73a2c00de6c5 100644 > --- a/arch/sh/include/asm/pgalloc.h > +++ b/arch/sh/include/asm/pgalloc.h > @@ -36,9 +36,8 @@ do { \ > #if CONFIG_PGTABLE_LEVELS > 2 > #define __pmd_free_tlb(tlb, pmdp, addr) \ > do { \ > - struct page *page = virt_to_page(pmdp); \ > - pgtable_pmd_page_dtor(page); \ > - tlb_remove_page((tlb), page); \ > + pgtable_pmd_page_dtor(pmdp); \ expected ‘struct page *’ but argument is of type ‘pmd_t * {aka struct <anonymous> *}’ > + tlb_remove_page((tlb), (pmdp)); \ likewise > } while (0); > #endif Gr{oetje,eeting}s, Geert
On 12/4/19 4:32 AM, Geert Uytterhoeven wrote: > Hoi Peter, > > On Wed, Dec 4, 2019 at 11:48 AM Peter Zijlstra <peterz@infradead.org> wrote: >> On Tue, Dec 03, 2019 at 12:19:00PM +0100, Geert Uytterhoeven wrote: >>> On Tue, Feb 19, 2019 at 11:35 AM Peter Zijlstra <peterz@infradead.org> wrote: >>>> Generic mmu_gather provides everything SH needs (range tracking and >>>> cache coherency). >>>> >>>> Cc: Will Deacon <will.deacon@arm.com> >>>> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> >>>> Cc: Andrew Morton <akpm@linux-foundation.org> >>>> Cc: Nick Piggin <npiggin@gmail.com> >>>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp> >>>> Cc: Rich Felker <dalias@libc.org> >>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> >>> >>> I got remote access to an SH7722-based Migo-R again, which spews a long >>> sequence of BUGs during userspace startup. I've bisected this to commit >>> c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). >> >> Whoopsy.. also, is this really the first time anybody booted an SH >> kernel in over a year ?!? > > Nah, but the v5.4-rc3 I booted recently on qemu -M r2d had > CONFIG_PGTABLE_LEVELS=2, so it didn't show the problem. > Guess that explains why I do not see the problem with my qemu boots. I use rts7751r2dplus_defconfig. Is it possible to reproduce the problem with qemu ? I don't think so, but maybe I am missing something. Guenter >>> Do you have a clue? >> >> Does the below help? > > Unfortunately not. > >> diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h >> index 22d968bfe9bb..73a2c00de6c5 100644 >> --- a/arch/sh/include/asm/pgalloc.h >> +++ b/arch/sh/include/asm/pgalloc.h >> @@ -36,9 +36,8 @@ do { \ >> #if CONFIG_PGTABLE_LEVELS > 2 >> #define __pmd_free_tlb(tlb, pmdp, addr) \ >> do { \ >> - struct page *page = virt_to_page(pmdp); \ >> - pgtable_pmd_page_dtor(page); \ >> - tlb_remove_page((tlb), page); \ >> + pgtable_pmd_page_dtor(pmdp); \ > > expected ‘struct page *’ but argument is of type ‘pmd_t * {aka struct > <anonymous> *}’ > >> + tlb_remove_page((tlb), (pmdp)); \ > > likewise > >> } while (0); >> #endif > > Gr{oetje,eeting}s, > > Geert >
On Wed, Dec 04, 2019 at 01:32:58PM +0100, Geert Uytterhoeven wrote: > > Does the below help? > > Unfortunately not. > > > diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h > > index 22d968bfe9bb..73a2c00de6c5 100644 > > --- a/arch/sh/include/asm/pgalloc.h > > +++ b/arch/sh/include/asm/pgalloc.h > > @@ -36,9 +36,8 @@ do { \ > > #if CONFIG_PGTABLE_LEVELS > 2 > > #define __pmd_free_tlb(tlb, pmdp, addr) \ > > do { \ > > - struct page *page = virt_to_page(pmdp); \ > > - pgtable_pmd_page_dtor(page); \ > > - tlb_remove_page((tlb), page); \ > > + pgtable_pmd_page_dtor(pmdp); \ > > expected ‘struct page *’ but argument is of type ‘pmd_t * {aka struct > <anonymous> *}’ > > > + tlb_remove_page((tlb), (pmdp)); \ > > likewise Duh.. clearly I misplaced my SH cross compiler. Let me go find it. Also, looking at pgtable.c the pmd_t* actually comes from a kmemcach() and should probably use pmd_free() (which is what the old code did too). Also, since SH doesn't have ARCH_ENABLE_SPLIT_PMD_PTLOCK, it will never need pgtable_pmd_page_dtor(). The below seems to build se7722_defconfig using sh4-linux-. That is, the build fails, on 'node_reclaim_distance', not pgtable stuff. Does this fare better? --- arch/sh/include/asm/pgalloc.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h index 22d968bfe9bb..c910e5bcde62 100644 --- a/arch/sh/include/asm/pgalloc.h +++ b/arch/sh/include/asm/pgalloc.h @@ -36,9 +36,7 @@ do { \ #if CONFIG_PGTABLE_LEVELS > 2 #define __pmd_free_tlb(tlb, pmdp, addr) \ do { \ - struct page *page = virt_to_page(pmdp); \ - pgtable_pmd_page_dtor(page); \ - tlb_remove_page((tlb), page); \ + pmd_free((tlb)->mm, (pmdp)); \ } while (0); #endif
Hoi Peter, On Wed, Dec 4, 2019 at 2:35 PM Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, Dec 04, 2019 at 01:32:58PM +0100, Geert Uytterhoeven wrote: > > > Does the below help? > > > > Unfortunately not. > > > > > diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h > > > index 22d968bfe9bb..73a2c00de6c5 100644 > > > --- a/arch/sh/include/asm/pgalloc.h > > > +++ b/arch/sh/include/asm/pgalloc.h > > > @@ -36,9 +36,8 @@ do { \ > > > #if CONFIG_PGTABLE_LEVELS > 2 > > > #define __pmd_free_tlb(tlb, pmdp, addr) \ > > > do { \ > > > - struct page *page = virt_to_page(pmdp); \ > > > - pgtable_pmd_page_dtor(page); \ > > > - tlb_remove_page((tlb), page); \ > > > + pgtable_pmd_page_dtor(pmdp); \ > > > > expected ‘struct page *’ but argument is of type ‘pmd_t * {aka struct > > <anonymous> *}’ > > > > > + tlb_remove_page((tlb), (pmdp)); \ > > > > likewise > > Duh.. clearly I misplaced my SH cross compiler. Let me go find it. > > Also, looking at pgtable.c the pmd_t* actually comes from a kmemcach() > and should probably use pmd_free() (which is what the old code did too). > > Also, since SH doesn't have ARCH_ENABLE_SPLIT_PMD_PTLOCK, it will never > need pgtable_pmd_page_dtor(). > > The below seems to build se7722_defconfig using sh4-linux-. That is, the > build fails, on 'node_reclaim_distance', not pgtable stuff. > > Does this fare better? Yes. Migo-R is happy again. Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/arch/sh/include/asm/pgalloc.h > +++ b/arch/sh/include/asm/pgalloc.h > @@ -36,9 +36,7 @@ do { \ > #if CONFIG_PGTABLE_LEVELS > 2 > #define __pmd_free_tlb(tlb, pmdp, addr) \ > do { \ > - struct page *page = virt_to_page(pmdp); \ > - pgtable_pmd_page_dtor(page); \ > - tlb_remove_page((tlb), page); \ > + pmd_free((tlb)->mm, (pmdp)); \ > } while (0); > #endif Gr{oetje,eeting}s, Geert
Hi Günter, On Wed, Dec 4, 2019 at 2:22 PM Guenter Roeck <linux@roeck-us.net> wrote: > On 12/4/19 4:32 AM, Geert Uytterhoeven wrote: > > On Wed, Dec 4, 2019 at 11:48 AM Peter Zijlstra <peterz@infradead.org> wrote: > >> On Tue, Dec 03, 2019 at 12:19:00PM +0100, Geert Uytterhoeven wrote: > >>> On Tue, Feb 19, 2019 at 11:35 AM Peter Zijlstra <peterz@infradead.org> wrote: > >>>> Generic mmu_gather provides everything SH needs (range tracking and > >>>> cache coherency). > >>> I got remote access to an SH7722-based Migo-R again, which spews a long > >>> sequence of BUGs during userspace startup. I've bisected this to commit > >>> c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). > >> > >> Whoopsy.. also, is this really the first time anybody booted an SH > >> kernel in over a year ?!? > > > > Nah, but the v5.4-rc3 I booted recently on qemu -M r2d had > > CONFIG_PGTABLE_LEVELS=2, so it didn't show the problem. > > > > Guess that explains why I do not see the problem with my qemu boots. > I use rts7751r2dplus_defconfig. Is it possible to reproduce the problem > with qemu ? I don't think so, but maybe I am missing something. Qemu seems to support r2d and shix only. For the latter, the website pointed to by the qemu sources no longer exists. But according to those sources, it's also sh7750-based, so no luck. Gr{oetje,eeting}s, Geert
On Wed, Dec 04, 2019 at 04:07:53PM +0100, Geert Uytterhoeven wrote: > On Wed, Dec 4, 2019 at 2:35 PM Peter Zijlstra <peterz@infradead.org> wrote: > > Does this fare better? > > Yes. Migo-R is happy again. > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/arch/sh/include/asm/pgalloc.h > > +++ b/arch/sh/include/asm/pgalloc.h > > @@ -36,9 +36,7 @@ do { \ > > #if CONFIG_PGTABLE_LEVELS > 2 > > #define __pmd_free_tlb(tlb, pmdp, addr) \ > > do { \ > > - struct page *page = virt_to_page(pmdp); \ > > - pgtable_pmd_page_dtor(page); \ > > - tlb_remove_page((tlb), page); \ > > + pmd_free((tlb)->mm, (pmdp)); \ > > } while (0); > > #endif OK, so I was going to write a Changelog to go with that, but then I realized that while this works and is similar to before the patch, I'm not sure this is in fact correct. With this on (and also before) we're freeing the PMD before we've done the TLB invalidate, that seems wrong! Looking at the size of that pmd_cache, that looks to be 30-(12+12-3)+3 == 12, which is exactly 1 page, for PAGE_SIZE_4K, less for the larger pages. I'm thinking perhaps we should do something like the below instead? --- arch/sh/mm/pgtable.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/arch/sh/mm/pgtable.c b/arch/sh/mm/pgtable.c index 5c8f9247c3c2..fac7e822fd0c 100644 --- a/arch/sh/mm/pgtable.c +++ b/arch/sh/mm/pgtable.c @@ -5,9 +5,6 @@ #define PGALLOC_GFP GFP_KERNEL | __GFP_ZERO static struct kmem_cache *pgd_cachep; -#if PAGETABLE_LEVELS > 2 -static struct kmem_cache *pmd_cachep; -#endif void pgd_ctor(void *x) { @@ -23,11 +20,6 @@ void pgtable_cache_init(void) pgd_cachep = kmem_cache_create("pgd_cache", PTRS_PER_PGD * (1<<PTE_MAGNITUDE), PAGE_SIZE, SLAB_PANIC, pgd_ctor); -#if PAGETABLE_LEVELS > 2 - pmd_cachep = kmem_cache_create("pmd_cache", - PTRS_PER_PMD * (1<<PTE_MAGNITUDE), - PAGE_SIZE, SLAB_PANIC, NULL); -#endif } pgd_t *pgd_alloc(struct mm_struct *mm) @@ -48,11 +40,7 @@ void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd) pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address) { - return kmem_cache_alloc(pmd_cachep, PGALLOC_GFP); -} - -void pmd_free(struct mm_struct *mm, pmd_t *pmd) -{ - kmem_cache_free(pmd_cachep, pmd); + BUILD_BUG_ON(PTRS_PER_PMD * (1<<PTE_MAGNITUDE) <= PAGE_SIZE); + return (pmd_t *)__get_free_page(PGALLOC_GFP); } #endif /* PAGETABLE_LEVELS > 2 */
On Wed, Dec 04, 2019 at 04:17:26PM +0100, Geert Uytterhoeven wrote: > Hi Günter, > > > > Nah, but the v5.4-rc3 I booted recently on qemu -M r2d had > > > CONFIG_PGTABLE_LEVELS=2, so it didn't show the problem. > > > > > > > Guess that explains why I do not see the problem with my qemu boots. > > I use rts7751r2dplus_defconfig. Is it possible to reproduce the problem > > with qemu ? I don't think so, but maybe I am missing something. > > Qemu seems to support r2d and shix only. > For the latter, the website pointed to by the qemu sources no longer exists. > But according to those sources, it's also sh7750-based, so no luck. > Oh, well, worth asking. Thanks for the feedback. Guenter
Hoi Peter, On Wed, Dec 4, 2019 at 5:42 PM Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, Dec 04, 2019 at 04:07:53PM +0100, Geert Uytterhoeven wrote: > > On Wed, Dec 4, 2019 at 2:35 PM Peter Zijlstra <peterz@infradead.org> wrote: > > > Does this fare better? > > > > Yes. Migo-R is happy again. > > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > --- a/arch/sh/include/asm/pgalloc.h > > > +++ b/arch/sh/include/asm/pgalloc.h > > > @@ -36,9 +36,7 @@ do { \ > > > #if CONFIG_PGTABLE_LEVELS > 2 > > > #define __pmd_free_tlb(tlb, pmdp, addr) \ > > > do { \ > > > - struct page *page = virt_to_page(pmdp); \ > > > - pgtable_pmd_page_dtor(page); \ > > > - tlb_remove_page((tlb), page); \ > > > + pmd_free((tlb)->mm, (pmdp)); \ > > > } while (0); > > > #endif > > OK, so I was going to write a Changelog to go with that, but then I > realized that while this works and is similar to before the patch, I'm > not sure this is in fact correct. > > With this on (and also before) we're freeing the PMD before we've done > the TLB invalidate, that seems wrong! > > Looking at the size of that pmd_cache, that looks to be 30-(12+12-3)+3 > == 12, which is exactly 1 page, for PAGE_SIZE_4K, less for the larger > pages. > > I'm thinking perhaps we should do something like the below instead? Your advice is better when in close vicinity of an SH cross compiler, though ;-) > --- a/arch/sh/mm/pgtable.c > +++ b/arch/sh/mm/pgtable.c > @@ -5,9 +5,6 @@ > #define PGALLOC_GFP GFP_KERNEL | __GFP_ZERO > > static struct kmem_cache *pgd_cachep; > -#if PAGETABLE_LEVELS > 2 > -static struct kmem_cache *pmd_cachep; > -#endif > > void pgd_ctor(void *x) > { > @@ -23,11 +20,6 @@ void pgtable_cache_init(void) > pgd_cachep = kmem_cache_create("pgd_cache", > PTRS_PER_PGD * (1<<PTE_MAGNITUDE), > PAGE_SIZE, SLAB_PANIC, pgd_ctor); > -#if PAGETABLE_LEVELS > 2 > - pmd_cachep = kmem_cache_create("pmd_cache", > - PTRS_PER_PMD * (1<<PTE_MAGNITUDE), > - PAGE_SIZE, SLAB_PANIC, NULL); > -#endif > } > > pgd_t *pgd_alloc(struct mm_struct *mm) > @@ -48,11 +40,7 @@ void pud_populate(struct mm_struct *mm, pud_t *pud, pmd_t *pmd) > > pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address) > { > - return kmem_cache_alloc(pmd_cachep, PGALLOC_GFP); > -} > - > -void pmd_free(struct mm_struct *mm, pmd_t *pmd) mm/memory.o: In function `__pmd_alloc': memory.c:(.text+0x1d74): undefined reference to `pmd_free' > -{ > - kmem_cache_free(pmd_cachep, pmd); > + BUILD_BUG_ON(PTRS_PER_PMD * (1<<PTE_MAGNITUDE) <= PAGE_SIZE); ... > PAGE_SIZE ? Else it triggers all the time. > + return (pmd_t *)__get_free_page(PGALLOC_GFP); > } > #endif /* PAGETABLE_LEVELS > 2 */ BTW, I'm still running Willy's fix that never made it upstream to kill an ugly boot warning, which also touches this code: https://patchwork.kernel.org/patch/10549883/#22166333 Gr{oetje,eeting}s, Geert
On Thu, Dec 05, 2019 at 01:24:02PM -0600, Rob Landley wrote: > On 12/4/19 4:47 AM, Peter Zijlstra wrote: > > On Tue, Dec 03, 2019 at 12:19:00PM +0100, Geert Uytterhoeven wrote: > >> Hoi Peter, > >> > >> On Tue, Feb 19, 2019 at 11:35 AM Peter Zijlstra <peterz@infradead.org> wrote: > >>> Generic mmu_gather provides everything SH needs (range tracking and > >>> cache coherency). > >>> > >>> Cc: Will Deacon <will.deacon@arm.com> > >>> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> > >>> Cc: Andrew Morton <akpm@linux-foundation.org> > >>> Cc: Nick Piggin <npiggin@gmail.com> > >>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp> > >>> Cc: Rich Felker <dalias@libc.org> > >>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > >> > >> I got remote access to an SH7722-based Migo-R again, which spews a long > >> sequence of BUGs during userspace startup. I've bisected this to commit > >> c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). > > > > Whoopsy.. also, is this really the first time anybody booted an SH > > kernel in over a year ?!? > > No, but most people running this kind of hardware tend not to upgrade to current > kernels on a regular basis. > > The j-core guys tested the 5.3 release. I can't find an email about 5.4 so I > dunno if that's been tested yet? Being that this code is about mmu, does it affect nommu machines? That's what we've got at present on the J-core side. Rich
On 12/4/19 4:47 AM, Peter Zijlstra wrote: > On Tue, Dec 03, 2019 at 12:19:00PM +0100, Geert Uytterhoeven wrote: >> Hoi Peter, >> >> On Tue, Feb 19, 2019 at 11:35 AM Peter Zijlstra <peterz@infradead.org> wrote: >>> Generic mmu_gather provides everything SH needs (range tracking and >>> cache coherency). >>> >>> Cc: Will Deacon <will.deacon@arm.com> >>> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> >>> Cc: Andrew Morton <akpm@linux-foundation.org> >>> Cc: Nick Piggin <npiggin@gmail.com> >>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp> >>> Cc: Rich Felker <dalias@libc.org> >>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> >> >> I got remote access to an SH7722-based Migo-R again, which spews a long >> sequence of BUGs during userspace startup. I've bisected this to commit >> c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). > > Whoopsy.. also, is this really the first time anybody booted an SH > kernel in over a year ?!? No, but most people running this kind of hardware tend not to upgrade to current kernels on a regular basis. The j-core guys tested the 5.3 release. I can't find an email about 5.4 so I dunno if that's been tested yet? I just tested yesterday's git and it works fine with http://lkml.iu.edu/hypermail/linux/kernel/1912.0/01554.html installed, modulo it _still_ has the suprious stack dump shortly before calling init, which I've complained about on linux-sh and off for a year now? ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at mm/slub.c:2451 kmem_cache_free_bulk+0x2c2/0x37c CPU: 0 PID: 1 Comm: swapper Not tainted 5.4.0 #1 PC is at kmem_cache_free_bulk+0x2c2/0x37c PR is at kmem_cache_alloc_bulk+0x36/0x1a0 PC : 8c0a6fae SP : 8f829e9c SR : 400080f0 TEA : c0001240 R0 : 8c0a6de4 R1 : 00000100 R2 : 00000100 R3 : 00000000 R4 : 8f8020a0 R5 : 00000dc0 R6 : 8c01d66c R7 : 8fff5180 R8 : 8c011a00 R9 : 8fff5180 R10 : 8c01d66c R11 : 80000000 R12 : 00007fff R13 : 00000dc0 R14 : 8f8020a0 MACH: 0000017a MACL: 0ae4849d GBR : 00000000 PR : 8c0a709e Call trace: [<(ptrval)>] copy_process+0x218/0x1094 [<(ptrval)>] copy_process+0x7ba/0x1094 [<(ptrval)>] kmem_cache_alloc_bulk+0x36/0x1a0 [<(ptrval)>] restore_sigcontext+0x94/0x1b0 [<(ptrval)>] restore_sigcontext+0x70/0x1b0 [<(ptrval)>] copy_process+0x218/0x1094 [<(ptrval)>] sysfs_slab_add+0x106/0x354 [<(ptrval)>] restore_sigcontext+0x70/0x1b0 [<(ptrval)>] copy_process+0x218/0x1094 [<(ptrval)>] copy_process+0x218/0x1094 [<(ptrval)>] fprop_fraction_single+0x38/0xa4 [<(ptrval)>] pipe_read+0x7a/0x23c [<(ptrval)>] restore_sigcontext+0x70/0x1b0 [<(ptrval)>] restore_sigcontext+0x94/0x1b0 [<(ptrval)>] alloc_pipe_info+0x162/0x1c8 [<(ptrval)>] restore_sigcontext+0x94/0x1b0 [<(ptrval)>] restore_sigcontext+0x70/0x1b0 [<(ptrval)>] handle_bad_irq+0x154/0x188 [<(ptrval)>] raw6_exit_net+0x0/0x14 [<(ptrval)>] prepare_stack+0xe4/0x2fc [<(ptrval)>] sys_sched_get_priority_min+0x18/0x28 [<(ptrval)>] ndisc_net_exit+0x4/0x24 ---[ end trace 6ce4eefeb577b078 ]--- But it's cosmetic... I haven't got one of the new Turtle boards yet (next time I visit Japan...) and the USB connector broke off my old one, so I haven't got test hardware in my bag to boot it on with me at this coffee shop. So just qemu testing at the moment. The actual j-core deployment environment I'm working on this month is a deeply embedded thing with 128k sram so isn't running Linux. :) Rob
Hi! On 12/4/19 11:47 AM, Peter Zijlstra wrote: >> I got remote access to an SH7722-based Migo-R again, which spews a long >> sequence of BUGs during userspace startup. I've bisected this to commit >> c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). > > Whoopsy.. also, is this really the first time anybody booted an SH > kernel in over a year ?!? I have to admit, I have been very lazy with kernel updates. I have been planning to upgrade to a much more recent release on my boards for a while now, I have just been postponing it since the machines run very stable with the current kernel I am using. Adrian
On Thu, Dec 05, 2019 at 08:30:17PM +0100, John Paul Adrian Glaubitz wrote: > Hi! > > On 12/4/19 11:47 AM, Peter Zijlstra wrote: > >> I got remote access to an SH7722-based Migo-R again, which spews a long > >> sequence of BUGs during userspace startup. I've bisected this to commit > >> c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). > > > > Whoopsy.. also, is this really the first time anybody booted an SH > > kernel in over a year ?!? > > I have to admit, I have been very lazy with kernel updates. I have been > planning to upgrade to a much more recent release on my boards for a while > now, I have just been postponing it since the machines run very stable > with the current kernel I am using. > Hey, if you write a qemu emulation, I'll be happy to run it on a regular basis :-) Problem is really that the architecture doesn't get as much attention as it needs. The backtrace pointed to by Rob has been seen for a long time, but either there is no one with the knowledge to fix it, or they are all busy with other stuff. Guenter > Adrian > > -- > .''`. John Paul Adrian Glaubitz > : :' : Debian Developer - glaubitz@debian.org > `. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de > `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Hi! On 12/5/19 11:56 PM, Guenter Roeck wrote: >> I have to admit, I have been very lazy with kernel updates. I have been >> planning to upgrade to a much more recent release on my boards for a while >> now, I have just been postponing it since the machines run very stable >> with the current kernel I am using. >> > > Hey, if you write a qemu emulation, I'll be happy to run it on a regular > basis :-) There is working SH emulation in QEMU. I have been wanting to prepare a current Debian/sh4 test image for that purpose for a while now. For the time being, you can use the image by Aurelien Jarno: > https://people.debian.org/~aurel32/qemu/sh4/ > Problem is really that the architecture doesn't get as much attention as > it needs. The backtrace pointed to by Rob has been seen for a long time, > but either there is no one with the knowledge to fix it, or they are all > busy with other stuff. I fully agree. I'm mostly busy with the userland stuff, i.e. Debian packages for sh4 and not so much with the kernel. Adrian
On 12/6/19 5:38 AM, John Paul Adrian Glaubitz wrote: > Hi! > > On 12/5/19 11:56 PM, Guenter Roeck wrote: >>> I have to admit, I have been very lazy with kernel updates. I have been >>> planning to upgrade to a much more recent release on my boards for a while >>> now, I have just been postponing it since the machines run very stable >>> with the current kernel I am using. >>> >> >> Hey, if you write a qemu emulation, I'll be happy to run it on a regular >> basis :-) > > There is working SH emulation in QEMU. I have been wanting to prepare a > current Debian/sh4 test image for that purpose for a while now. > I do run this emulation in my tests. It does not support the affected CPU, unfortunately. Guenter > For the time being, you can use the image by Aurelien Jarno: > >> https://people.debian.org/~aurel32/qemu/sh4/ > >> Problem is really that the architecture doesn't get as much attention as >> it needs. The backtrace pointed to by Rob has been seen for a long time, >> but either there is no one with the knowledge to fix it, or they are all >> busy with other stuff. > > I fully agree. I'm mostly busy with the userland stuff, i.e. Debian packages > for sh4 and not so much with the kernel. > > Adrian >
Hi! On 12/4/19 1:32 PM, Geert Uytterhoeven wrote: >>>> Cc: Will Deacon <will.deacon@arm.com> >>>> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> >>>> Cc: Andrew Morton <akpm@linux-foundation.org> >>>> Cc: Nick Piggin <npiggin@gmail.com> >>>> Cc: Yoshinori Sato <ysato@users.sourceforge.jp> >>>> Cc: Rich Felker <dalias@libc.org> >>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> >>> >>> I got remote access to an SH7722-based Migo-R again, which spews a long >>> sequence of BUGs during userspace startup. I've bisected this to commit >>> c5b27a889da92f4a ("sh/tlb: Convert SH to generic mmu_gather"). >> >> Whoopsy.. also, is this really the first time anybody booted an SH >> kernel in over a year ?!? > > Nah, but the v5.4-rc3 I booted recently on qemu -M r2d had > CONFIG_PGTABLE_LEVELS=2, so it didn't show the problem. > >>> Do you have a clue? >> >> Does the below help? > > Unfortunately not. > >> diff --git a/arch/sh/include/asm/pgalloc.h b/arch/sh/include/asm/pgalloc.h >> index 22d968bfe9bb..73a2c00de6c5 100644 >> --- a/arch/sh/include/asm/pgalloc.h >> +++ b/arch/sh/include/asm/pgalloc.h >> @@ -36,9 +36,8 @@ do { \ >> #if CONFIG_PGTABLE_LEVELS > 2 >> #define __pmd_free_tlb(tlb, pmdp, addr) \ >> do { \ >> - struct page *page = virt_to_page(pmdp); \ >> - pgtable_pmd_page_dtor(page); \ >> - tlb_remove_page((tlb), page); \ >> + pgtable_pmd_page_dtor(pmdp); \ > > expected ‘struct page *’ but argument is of type ‘pmd_t * {aka struct > <anonymous> *}’ > >> + tlb_remove_page((tlb), (pmdp)); \ > > likewise > >> } while (0); >> #endif Any chance we can have another go at this? The original change commit c5b27a889da92f4a969d61df77bd4f79ffce57c9 (refs/bisect/bad) Author: Peter Zijlstra <peterz@infradead.org> Date: Tue Sep 4 14:45:04 2018 +0200 sh/tlb: Convert SH to generic mmu_gather Generic mmu_gather provides everything SH needs (range tracking and cache coherency). breaks systemd for me on my SH-7785LCR [1]. Adrian > [1] https://marc.info/?l=linux-kernel&m=159479951822677&w=2 Adrian
Hi Rob! On 12/5/19 8:24 PM, Rob Landley wrote: > No, but most people running this kind of hardware tend not to upgrade to current > kernels on a regular basis. > > The j-core guys tested the 5.3 release. I can't find an email about 5.4 so I > dunno if that's been tested yet? > > I just tested yesterday's git and it works fine with > http://lkml.iu.edu/hypermail/linux/kernel/1912.0/01554.html installed, modulo it > _still_ has the suprious stack dump shortly before calling init, which I've > complained about on linux-sh and off for a year now? > > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 1 at mm/slub.c:2451 kmem_cache_free_bulk+0x2c2/0x37c > > CPU: 0 PID: 1 Comm: swapper Not tainted 5.4.0 #1 > PC is at kmem_cache_free_bulk+0x2c2/0x37c > PR is at kmem_cache_alloc_bulk+0x36/0x1a0 > PC : 8c0a6fae SP : 8f829e9c SR : 400080f0 > TEA : c0001240 > R0 : 8c0a6de4 R1 : 00000100 R2 : 00000100 R3 : 00000000 > R4 : 8f8020a0 R5 : 00000dc0 R6 : 8c01d66c R7 : 8fff5180 > R8 : 8c011a00 R9 : 8fff5180 R10 : 8c01d66c R11 : 80000000 > R12 : 00007fff R13 : 00000dc0 R14 : 8f8020a0 > MACH: 0000017a MACL: 0ae4849d GBR : 00000000 PR : 8c0a709e > > Call trace: > [<(ptrval)>] copy_process+0x218/0x1094 > [<(ptrval)>] copy_process+0x7ba/0x1094 > [<(ptrval)>] kmem_cache_alloc_bulk+0x36/0x1a0 > [<(ptrval)>] restore_sigcontext+0x94/0x1b0 > [<(ptrval)>] restore_sigcontext+0x70/0x1b0 > [<(ptrval)>] copy_process+0x218/0x1094 > [<(ptrval)>] sysfs_slab_add+0x106/0x354 > [<(ptrval)>] restore_sigcontext+0x70/0x1b0 > [<(ptrval)>] copy_process+0x218/0x1094 > [<(ptrval)>] copy_process+0x218/0x1094 > [<(ptrval)>] fprop_fraction_single+0x38/0xa4 > [<(ptrval)>] pipe_read+0x7a/0x23c > [<(ptrval)>] restore_sigcontext+0x70/0x1b0 > [<(ptrval)>] restore_sigcontext+0x94/0x1b0 > [<(ptrval)>] alloc_pipe_info+0x162/0x1c8 > [<(ptrval)>] restore_sigcontext+0x94/0x1b0 > [<(ptrval)>] restore_sigcontext+0x70/0x1b0 > [<(ptrval)>] handle_bad_irq+0x154/0x188 > [<(ptrval)>] raw6_exit_net+0x0/0x14 > [<(ptrval)>] prepare_stack+0xe4/0x2fc > [<(ptrval)>] sys_sched_get_priority_min+0x18/0x28 > [<(ptrval)>] ndisc_net_exit+0x4/0x24 > > ---[ end trace 6ce4eefeb577b078 ]--- > > But it's cosmetic... This is fixed by the following patch [1]: sh: Fix unneeded constructor in page table allocation The pgd kmem_cache allocation both specified __GFP_ZERO and had a constructor which makes no sense. Remove __GFP_ZERO and zero the user parts of the pgd explicitly. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Signed-off-by: Rich Felker <dalias@libc.org> Adrian > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=73c348f31b63d28d176ed290eb1aa2a648f3e51e
--- a/arch/sh/include/asm/pgalloc.h +++ b/arch/sh/include/asm/pgalloc.h @@ -72,6 +72,15 @@ do { \ tlb_remove_page((tlb), (pte)); \ } while (0) +#if CONFIG_PGTABLE_LEVELS > 2 +#define __pmd_free_tlb(tlb, pmdp, addr) \ +do { \ + struct page *page = virt_to_page(pmdp); \ + pgtable_pmd_page_dtor(page); \ + tlb_remove_page((tlb), page); \ +} while (0); +#endif + static inline void check_pgt_cache(void) { quicklist_trim(QUICK_PT, NULL, 25, 16); --- a/arch/sh/include/asm/tlb.h +++ b/arch/sh/include/asm/tlb.h @@ -11,131 +11,8 @@ #ifdef CONFIG_MMU #include <linux/swap.h> -#include <asm/pgalloc.h> -#include <asm/tlbflush.h> -#include <asm/mmu_context.h> - -/* - * TLB handling. This allows us to remove pages from the page - * tables, and efficiently handle the TLB issues. - */ -struct mmu_gather { - struct mm_struct *mm; - unsigned int fullmm; - unsigned long start, end; -}; -static inline void init_tlb_gather(struct mmu_gather *tlb) -{ - tlb->start = TASK_SIZE; - tlb->end = 0; - - if (tlb->fullmm) { - tlb->start = 0; - tlb->end = TASK_SIZE; - } -} - -static inline void -arch_tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, - unsigned long start, unsigned long end) -{ - tlb->mm = mm; - tlb->start = start; - tlb->end = end; - tlb->fullmm = !(start | (end+1)); - - init_tlb_gather(tlb); -} - -static inline void -arch_tlb_finish_mmu(struct mmu_gather *tlb, - unsigned long start, unsigned long end, bool force) -{ - if (tlb->fullmm || force) - flush_tlb_mm(tlb->mm); - - /* keep the page table cache within bounds */ - check_pgt_cache(); -} - -static inline void -tlb_remove_tlb_entry(struct mmu_gather *tlb, pte_t *ptep, unsigned long address) -{ - if (tlb->start > address) - tlb->start = address; - if (tlb->end < address + PAGE_SIZE) - tlb->end = address + PAGE_SIZE; -} - -#define tlb_remove_huge_tlb_entry(h, tlb, ptep, address) \ - tlb_remove_tlb_entry(tlb, ptep, address) - -/* - * In the case of tlb vma handling, we can optimise these away in the - * case where we're doing a full MM flush. When we're doing a munmap, - * the vmas are adjusted to only cover the region to be torn down. - */ -static inline void -tlb_start_vma(struct mmu_gather *tlb, struct vm_area_struct *vma) -{ - if (!tlb->fullmm) - flush_cache_range(vma, vma->vm_start, vma->vm_end); -} - -static inline void -tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vma) -{ - if (!tlb->fullmm && tlb->end) { - flush_tlb_range(vma, tlb->start, tlb->end); - init_tlb_gather(tlb); - } -} - -static inline void tlb_flush_mmu_tlbonly(struct mmu_gather *tlb) -{ -} - -static inline void tlb_flush_mmu_free(struct mmu_gather *tlb) -{ -} - -static inline void tlb_flush_mmu(struct mmu_gather *tlb) -{ -} - -static inline int __tlb_remove_page(struct mmu_gather *tlb, struct page *page) -{ - free_page_and_swap_cache(page); - return false; /* avoid calling tlb_flush_mmu */ -} - -static inline void tlb_remove_page(struct mmu_gather *tlb, struct page *page) -{ - __tlb_remove_page(tlb, page); -} - -static inline bool __tlb_remove_page_size(struct mmu_gather *tlb, - struct page *page, int page_size) -{ - return __tlb_remove_page(tlb, page); -} - -static inline void tlb_remove_page_size(struct mmu_gather *tlb, - struct page *page, int page_size) -{ - return tlb_remove_page(tlb, page); -} - -static inline void tlb_change_page_size(struct mmu_gather *tlb, unsigned int page_size) -{ -} - -#define pte_free_tlb(tlb, ptep, addr) pte_free((tlb)->mm, ptep) -#define pmd_free_tlb(tlb, pmdp, addr) pmd_free((tlb)->mm, pmdp) -#define pud_free_tlb(tlb, pudp, addr) pud_free((tlb)->mm, pudp) - -#define tlb_migrate_finish(mm) do { } while (0) +#include <asm-generic/tlb.h> #if defined(CONFIG_CPU_SH4) || defined(CONFIG_SUPERH64) extern void tlb_wire_entry(struct vm_area_struct *, unsigned long, pte_t); @@ -155,11 +32,6 @@ static inline void tlb_unwire_entry(void #else /* CONFIG_MMU */ -#define tlb_start_vma(tlb, vma) do { } while (0) -#define tlb_end_vma(tlb, vma) do { } while (0) -#define __tlb_remove_tlb_entry(tlb, pte, address) do { } while (0) -#define tlb_flush(tlb) do { } while (0) - #include <asm-generic/tlb.h> #endif /* CONFIG_MMU */
Generic mmu_gather provides everything SH needs (range tracking and cache coherency). Cc: Will Deacon <will.deacon@arm.com> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Nick Piggin <npiggin@gmail.com> Cc: Yoshinori Sato <ysato@users.sourceforge.jp> Cc: Rich Felker <dalias@libc.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- arch/sh/include/asm/pgalloc.h | 7 ++ arch/sh/include/asm/tlb.h | 130 ------------------------------------------ 2 files changed, 8 insertions(+), 129 deletions(-)