Re: PageHead macro broken?

[Andrea Arcangeli <aarcange@redhat.com>]

Hi everyone,

On Mon, Dec 24, 2012 at 11:21:02AM -0800, Linus Torvalds wrote:
> On Mon, Dec 24, 2012 at 10:53 AM, Christoffer Dall
> <cdall@cs.columbia.edu> wrote:
> >
> > I think I may have found an issue with the PageHead macro, which
> > returns true for tail compound pages when CONFIG_PAGEFLAGS_EXTENDED is
> > not defined.
> 
> Hmm. Your patch *looks* obviously correct, in that it actually makes
> the code match the comment just above it. And making PageHead() test
> just the "compound" flag (and thus a tail-page would trigger it too)
> sounds wrong. But I join you in the "let's check the expected
> semantics with the people who use it" chorus.

Yes, it's wrong if PageHead returns true on a tail page. PageHead and
PageTail are mutually exclusive flags. Only PageCompound returns true
for both PageHead and PageTail.

> The fact that it fixes a problem on KVM/ARM is obviously another good sign.
> 
> At the same time, I wonder why it hasn't shown up as a problem on
> x86-32. On x86-64 PAGEFLAGS_EXTENDED is always true, but afaik, it
> should be possible to trigger this on 32-bit architectures if you just
> have SPARSEMEM && !SPARSEMEM_VMEMMAP.

Most of the PageHead checks are consistently run on real head pages,
so they're unlikely to run on tail pages. When !PageHead is used in
the bugchecks, the bug would lead to a false negative in the worst
case. This may be why this didn't show up on x86 32bit?

But AFIK no binary x86 kernel was shipped with THP compiled in, so
it's also hard to quantify the different configs for the x86 32bit
self-built kernel images out there.

> And SPARSEMEM on x86-32 is enabled with NUMA or EXPERIMENTAL set. And
> afaik, x86-32 never has SPARSEMEM_VMEMMAP. So this should not be a
> very uncommon setup.
> 
> Added Andrea and Kirill to the Cc, since most of the *uses* of
> PageHead() in the generic VM code are attributed to either of them
> according to "git blame". Left the rest of the email quoted for the
> new participants.. Also, you seem to have used Christoph's old SGI
> email address that I don't think is in use any more.
> 
> Andrea? Kirill? Christoph?

The fix looks good to me, thanks!
Andrea

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>