x86/mm/pageattr: Code without effect?

[Stefan Bader]

When looking through some mm code I stumbled over one part in
arch/x86/mm/pageattr.c that looks somewhat bogus to me. Cannot
say what exactly the effects are, but maybe you do (or you could
explain to me why I am wrong :)).

commit a8aed3e0752b4beb2e37cbed6df69faae88268da
Author: Andrea Arcangeli <aarcange@xxxxxxxxxx>
Date:   Fri Feb 22 15:11:51 2013 -0800

    x86/mm/pageattr: Prevent PSE and GLOABL leftovers to confuse
    pmd/pte_present and pmd_huge

added the following to try_preserve_large_page:

 	/*
+	 * Set the PSE and GLOBAL flags only if the PRESENT flag is
+	 * set otherwise pmd_present/pmd_huge will return true even on
+	 * a non present pmd. The canon_pgprot will clear _PAGE_GLOBAL
+	 * for the ancient hardware that doesn't support it.
+	 */
+	if (pgprot_val(new_prot) & _PAGE_PRESENT)
+		pgprot_val(new_prot) |= _PAGE_PSE | _PAGE_GLOBAL;
+	else
+		pgprot_val(new_prot) &= ~(_PAGE_PSE | _PAGE_GLOBAL);
+
+	new_prot = canon_pgprot(new_prot);
+
+	/*

but (extending what follows after the changes)

	 * old_pte points to the large page base address. So we need
	 * to add the offset of the virtual address:
	 */
	pfn = pte_pfn(old_pte) + ((address & (psize - 1)) >> PAGE_SHIFT);
	cpa->pfn = pfn;

	new_prot = static_protections(req_prot, address, pfn);

So new_prot gets completely replaced by req_prot and all changes done to
new_prot before look to be lost (the PSE and GLOBAL bit settings as well
as the canon_pgprot call.

Maybe the hunk is useless anyway, or the breakage is subtle, or I miss something...

Thanks,
Stefan

Attachment: signature.asc
Description: OpenPGP digital signature