On Tue, Jan 17, 2023 at 03:39:15PM +0800, Vlastimil Babka wrote:
> On 1/17/23 08:10, kernel test robot wrote:
> >
> > +Vlastimil Babka, Hyeonggon Yoo, Feng Tang and Fengwei Yin
> >
> > Hi, Mike Kravetz,
> >
> > we reported
> > "[linus:master] [mm, slub] 0af8489b02: kernel_BUG_at_include/linux/mm.h" [1]
> >
> > Vlastimil, Hyeonggon, Feng and Fengwei gave us a lot of great guidances based on
> > it, and, perticularly, after enabling below config per Vlastimil's suggestion
> > CONFIG_DEBUG_PAGEALLOC
> > CONFIG_DEBUG_PAGEALLOC_ENABLE_DEFAULT
> > CONFIG_SLUB_DEBUG
> > CONFIG_SLUB_DEBUG_ON
> > by more tests, we realized the "0af8489b02" is not the real culprit.
> >
> > the new bisection was triggered and finally it pointed to this "7118fc2906".
> >
> > though reporting for different issues
> > ("kernel_BUG_at_include/linux/mm.h" for 0af8489b02 vs.
> > "kernel_BUG_at_lib/list_debug.c" for this commit),
> > Feng and Fengwei helped further to confirm they are similar.
> > They will supply more technical wise analysis later.
> >
> > please be noted the issues are not always happening
> > (~10% on this commit or 0af8489b02)
>
> Great find! Looking at the commit, I'd bet the only part relevant to our bug
> is the "by the way we remove setting refcount to zero on tail pages which
> should already be zero":
>
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index db00ee8d79d2..eeff64843718 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -754,7 +754,6 @@ void prep_compound_page(struct page *page, unsigned int order)
> > __SetPageHead(page);
> > for (i = 1; i < nr_pages; i++) {
> > struct page *p = page + i;
> > - set_page_count(p, 0);
> > p->mapping = TAIL_MAPPING;
> > set_compound_head(p, page);
> > }
>
> So either the assumption of refcount being already 0 is wrong (shouldn't be,
> AFAIK?), or this atomic operation effectively prevents some very subtle race
> (although IIRC atomic_set() has no barrier semantics defined, it could still
> affect a specific CPU?
Yes, "set_page_count(p, 0);" seems to be what matters here. Restoring
it make the list corruption issue not reproducable for 300+ runs.
And back in debugging 0af8489b02, the thing was similar that if we
added some code inside prep_compound_page(), the issue also can't
be reproduced.
So this 7118fc2906 seems just 'expose' the problem on i386, and is
not the root cause.
I suspect it is related with i386 compiling, based on the debug and
memory dump. I'm doing some compiler option and adding memory
barrier in prep_compound_page(), and will update when the test run
is done.
Thanks,
Feng
> I guess we could
> - try to restore that set_page_count(p, 0); on current kernel to see if it
> kills the bug
> - instead of restoring it, add (only locally for purposes of the test) a
> BUG_ON() if refcount is not zero already, and find out why if it triggers
> (unfortunately might also appear to fix the bug even if it doesn't trigger).
