lkml.org 
[lkml]   [2013]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: x86/mm/pageattr: Code without effect?
On Fri, Apr 05, 2013 at 11:01:02AM +0200, Stefan Bader wrote:
> 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@redhat.com>
> 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...

Yeah, I had to unwillingly stare at this crazy code recently too and
I can share your confusion. And from trying to grok what's going
on, I *think* what we actually meant to do is sanitize our required
protections first, i.e.

new_prot = static_protections(req_prot, address, pfn);

and *then* do the _PAGE_PRESENT massaging. It does at least make sense
that way. And this is what we already do in __change_page_attr() for a
4K pte.

Andrea?

Thanks.

--
Regards/Gruss,
Boris.

Sent from a fat crate under my desk. Formatting is fine.
--


\
 
 \ /
  Last update: 2013-04-05 17:01    [W:0.063 / U:0.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site