Re: Alpha: rare random memory corruption/segfault in user space bisected

[Yu Zhao <yuzhao@google.com>]

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

On Fri, May 6, 2022 at 6:57 PM Hillf Danton <hdanton@sina.com> wrote:
>
> On Sat, 7 May 2022 09:21:25 +1200 Michael Cree wrote:
> > Alpha kernel has been exhibiting rare and random memory
> > corruptions/segaults in user space since the 5.9.y kernel.  First seen
> > on the Debian Ports build daemon when running 5.10.y kernel resulting
> > in the occasional (one or two a day) build failures with gcc ICEs either
> > due to self detected corrupt memory structures or segfaults.  Have been
> > running 5.8.y kernel without such problems for over six months.
> >
> > Tried bisecting last year but went off track with incorrect good/bad
> > determinations due to rare nature of bug.  After trying a 5.16.y kernel
> > early this year and seen the bug is still present retried the bisection
> > and have got to:
> >
> > aae466b0052e1888edd1d7f473d4310d64936196 is the first bad commit
> > commit aae466b0052e1888edd1d7f473d4310d64936196
> > Author: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> > Date:   Tue Aug 11 18:30:50 2020 -0700
> >
> >     mm/swap: implement workingset detection for anonymous LRU

This commit seems innocent to me. While not ruling out anything, i.e.,
this commit, compiler, qemu, userspace itself, etc., my wild guess is
the problem is memory barrier related. Two lock/unlock pairs, which
imply two full barriers, were removed. This is not a small deal on
Alpha, since it imposes no constraints on cache coherency, AFAIK.

Can you please try the attached patch on top of this commit? Thanks!

> > Pretty confident this is the bad commit as the kernel built to the parent
> > commit (3852f6768ede54...) has not failed in four days running. Always have
> > seen the failure within one day of running in past.
>
> See if the fix to the syzbot bisection [1] is not a cure to your issue.
>
> [1] https://lore.kernel.org/lkml/000000000000625fa705dd1802e3@google.com/

[-- Attachment #2: test.diff --]
[-- Type: application/octet-stream, Size: 653 bytes --]

diff --git a/mm/memory.c b/mm/memory.c
index de311fc7639e..f1cf07416cf4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3150,6 +3150,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 					goto out_page;
 				}
 
+				smp_mb();
+
 				shadow = get_shadow_from_swap_cache(entry);
 				if (shadow)
 					workingset_refault(page, shadow);
diff --git a/mm/swap_state.c b/mm/swap_state.c
index b73aabdfd35a..310d4049cdf3 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -499,6 +499,8 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
 		goto fail_unlock;
 	}
 
+	smp_mb();
+
 	if (shadow)
 		workingset_refault(page, shadow);