All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Bader <stefan.bader@canonical.com>
To: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>
Cc: Andy Whitcroft <apw@canonical.com>, Mel Gorman <mgorman@suse.de>
Subject: x86/mm/pageattr: Code without effect?
Date: Fri, 05 Apr 2013 11:01:02 +0200	[thread overview]
Message-ID: <515E92CE.2000509@canonical.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1598 bytes --]

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

Thanks,
Stefan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]

             reply	other threads:[~2013-04-05  9:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-05  9:01 Stefan Bader [this message]
2013-04-05 14:21 ` x86/mm/pageattr: Code without effect? Borislav Petkov
2013-04-06 14:58   ` Andrea Arcangeli
2013-04-06 15:47     ` Borislav Petkov
2013-04-08 11:53       ` Ingo Molnar
2013-04-08 11:59         ` Borislav Petkov
2013-04-08 12:28           ` Stefan Bader
2013-04-08 12:51             ` Borislav Petkov
2013-04-08 13:10               ` Stefan Bader
2013-04-08 14:15                 ` Borislav Petkov
2013-04-08 14:51                   ` Stefan Bader
2013-04-08 14:53     ` Andy Whitcroft
2013-04-08 15:32       ` Andrea Arcangeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=515E92CE.2000509@canonical.com \
    --to=stefan.bader@canonical.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@canonical.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.