LinuxLists.cc - [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

Hoi Peter,

On Tue, Feb 19, 2019 at 11:35 AM Peter Zijlstra <[email protected]> wrote:
> Generic mmu_gather provides everything SH needs (range tracking and
> cache coherency).
>
> Cc: Will Deacon <[email protected]>
> Cc: "Aneesh Kumar K.V" <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Nick Piggin <[email protected]>
> Cc: Yoshinori Sato <[email protected]>
> Cc: Rich Felker <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>

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 -- [email protected]

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

2019-12-04 10:50:00

by Peter Zijlstra

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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 <[email protected]> wrote:
> > Generic mmu_gather provides everything SH needs (range tracking and
> > cache coherency).
> >
> > Cc: Will Deacon <[email protected]>
> > Cc: "Aneesh Kumar K.V" <[email protected]>
> > Cc: Andrew Morton <[email protected]>
> > Cc: Nick Piggin <[email protected]>
> > Cc: Yoshinori Sato <[email protected]>
> > Cc: Rich Felker <[email protected]>
> > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
>
> 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

2019-12-04 12:34:22

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

Hoi Peter,

On Wed, Dec 4, 2019 at 11:48 AM Peter Zijlstra <[email protected]> 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 <[email protected]> wrote:
> > > Generic mmu_gather provides everything SH needs (range tracking and
> > > cache coherency).
> > >
> > > Cc: Will Deacon <[email protected]>
> > > Cc: "Aneesh Kumar K.V" <[email protected]>
> > > Cc: Andrew Morton <[email protected]>
> > > Cc: Nick Piggin <[email protected]>
> > > Cc: Yoshinori Sato <[email protected]>
> > > Cc: Rich Felker <[email protected]>
> > > Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> >
> > 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

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2019-12-04 13:24:34

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

On 12/4/19 4:32 AM, Geert Uytterhoeven wrote:
> Hoi Peter,
>
> On Wed, Dec 4, 2019 at 11:48 AM Peter Zijlstra <[email protected]> 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 <[email protected]> wrote:
>>>> Generic mmu_gather provides everything SH needs (range tracking and
>>>> cache coherency).
>>>>
>>>> Cc: Will Deacon <[email protected]>
>>>> Cc: "Aneesh Kumar K.V" <[email protected]>
>>>> Cc: Andrew Morton <[email protected]>
>>>> Cc: Nick Piggin <[email protected]>
>>>> Cc: Yoshinori Sato <[email protected]>
>>>> Cc: Rich Felker <[email protected]>
>>>> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
>>>
>>> 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
>

2019-12-04 13:37:05

by Peter Zijlstra

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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

2019-12-04 15:09:45

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

Hoi Peter,

On Wed, Dec 4, 2019 at 2:35 PM Peter Zijlstra <[email protected]> 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 <[email protected]>

> --- 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

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2019-12-04 15:18:47

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

Hi Günter,

On Wed, Dec 4, 2019 at 2:22 PM Guenter Roeck <[email protected]> wrote:
> On 12/4/19 4:32 AM, Geert Uytterhoeven wrote:
> > On Wed, Dec 4, 2019 at 11:48 AM Peter Zijlstra <[email protected]> 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 <[email protected]> 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

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2019-12-04 17:31:17

by Peter Zijlstra

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

On Wed, Dec 04, 2019 at 04:07:53PM +0100, Geert Uytterhoeven wrote:
> On Wed, Dec 4, 2019 at 2:35 PM Peter Zijlstra <[email protected]> wrote:

> > Does this fare better?
>
> Yes. Migo-R is happy again.
> Tested-by: Geert Uytterhoeven <[email protected]>
>
> > --- 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 */

2019-12-04 19:50:23

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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

2019-12-05 16:15:09

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

Hoi Peter,

On Wed, Dec 4, 2019 at 5:42 PM Peter Zijlstra <[email protected]> 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 <[email protected]> wrote:
> > > Does this fare better?
> >
> > Yes. Migo-R is happy again.
> > Tested-by: Geert Uytterhoeven <[email protected]>
> >
> > > --- 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

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2019-12-05 19:23:10

by Rob Landley

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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 <[email protected]> wrote:
>>> Generic mmu_gather provides everything SH needs (range tracking and
>>> cache coherency).
>>>
>>> Cc: Will Deacon <[email protected]>
>>> Cc: "Aneesh Kumar K.V" <[email protected]>
>>> Cc: Andrew Morton <[email protected]>
>>> Cc: Nick Piggin <[email protected]>
>>> Cc: Yoshinori Sato <[email protected]>
>>> Cc: Rich Felker <[email protected]>
>>> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
>>
>> 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

2019-12-05 19:26:23

by Rich Felker

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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 <[email protected]> wrote:
> >>> Generic mmu_gather provides everything SH needs (range tracking and
> >>> cache coherency).
> >>>
> >>> Cc: Will Deacon <[email protected]>
> >>> Cc: "Aneesh Kumar K.V" <[email protected]>
> >>> Cc: Andrew Morton <[email protected]>
> >>> Cc: Nick Piggin <[email protected]>
> >>> Cc: Yoshinori Sato <[email protected]>
> >>> Cc: Rich Felker <[email protected]>
> >>> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> >>
> >> 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

2019-12-05 19:31:05

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2019-12-05 22:56:55

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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 - [email protected]
> `. `' Freie Universitaet Berlin - [email protected]
> `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2019-12-06 13:39:38

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2019-12-06 14:04:57

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

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
>

2020-07-15 19:46:54

[permalink] [raw]

Subject: Re: [PATCH v6 10/18] sh/tlb: Convert SH to generic mmu_gather

Hi!

On 12/4/19 1:32 PM, Geert Uytterhoeven wrote:
>>>> Cc: Will Deacon <[email protected]>
>>>> Cc: "Aneesh Kumar K.V" <[email protected]>
>>>> Cc: Andrew Morton <[email protected]>
>>>> Cc: Nick Piggin <[email protected]>
>>>> Cc: Yoshinori Sato <[email protected]>
>>>> Cc: Rich Felker <[email protected]>
>>>> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
>>>
>>> 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 <[email protected]>
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

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2020-07-15 19:51:12