Re: [linus:master] [hugetlb] 7118fc2906: kernel_BUG_at_lib/list_debug.c

[Feng Tang <feng.tang@intel.com>]

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