xfs
[Top] [All Lists]

Re: [PATCH 3/4] mm: numa: Mark huge PTEs young when clearing NUMA hintin

To: Mel Gorman <mgorman@xxxxxxx>
Subject: Re: [PATCH 3/4] mm: numa: Mark huge PTEs young when clearing NUMA hinting faults
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Sat, 7 Mar 2015 10:33:43 -0800
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, Ingo Molnar <mingo@xxxxxxxxxx>, Aneesh Kumar <aneesh.kumar@xxxxxxxxxxxxxxxxxx>, Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>, Linux-MM <linux-mm@xxxxxxxxx>, xfs@xxxxxxxxxxx, ppc-dev <linuxppc-dev@xxxxxxxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=4SXK9VmbQ6Y4KIMJC4k8DuUhXCuw5+5A8o+Fc2dmay0=; b=zQCHeNyxxs+okLvMSz7B/+i8YcFyRM4uRfUuEnhMBFLgyUtf8J8UwNWRpA+k9XzrdK CsX0kvxE0tsZ1MFwnZ/dEl01NCYSiQfbCDA1u6JEfDFA5A+qSZEeQ4EHf7Bzup/3mpnV t/yh09q2LX6MMOzMbPbe+hx+HG0fIf+viCLgC8zcoYJAnF996DNxF9bh1lvBiEOM1Fgx Kbtzdl3osWPnrl4i7cXiGRJKXq7pbFaWHC/YNsImbZ0YfTTiM2S68+zy5fjzogRh8ZyT kF6KPD5q2q1eLfXPYnpygbs3sWrhT1EUvGWPrxi5+mdNh0lkTtxwPOEKaUeZQAnC21b9 IWqg==
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=4SXK9VmbQ6Y4KIMJC4k8DuUhXCuw5+5A8o+Fc2dmay0=; b=G5NlYuY6RbrnaSyA5D6R9W/6UOWDfVVTx58FInsRmMz6M0XfaHkk1PHQhWGJaTbQkD OVArBNkdGBvmtd/juBwZ/mrbm6DWeLxv1CzYQU6zV1WA6rkr5I5mjLKjUlN/bLOfRV5s 0263utRQEiHlqHdmhSj7qoF+ob5SwBtK/HKTQ=
In-reply-to: <1425741651-29152-4-git-send-email-mgorman@xxxxxxx>
References: <1425741651-29152-1-git-send-email-mgorman@xxxxxxx> <1425741651-29152-4-git-send-email-mgorman@xxxxxxx>
Sender: linus971@xxxxxxxxx
On Sat, Mar 7, 2015 at 7:20 AM, Mel Gorman <mgorman@xxxxxxx> wrote:
>         pmd = pmd_modify(pmd, vma->vm_page_prot);
> +       pmd = pmd_mkyoung(pmd);

Hmm. I *thought* this should be unnecessary. vm_page_prot alreadty has
the accessed bit set, and we kind of depend on the initial page table
setup and mk_pte() and friends (ie all new pages are installed
"young").

But it looks like I am wrong - the way we use _[H]PAGE_CHG_MASK means
that we always take the accessed and dirty bits from the old entry,
ignoring the bit in vm_page_prot.

I wonder if we should just make pte/pmd_modify() work the way I
*thought* they worked (remove the masking of vm_page_prot bits).

So the patch isn't wrong. It's just that we *migth* instead just do
something like this:

    arch/x86/include/asm/pgtable.h | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

   diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
   index a0c35bf6cb92..79b898bb9e18 100644
   --- a/arch/x86/include/asm/pgtable.h
   +++ b/arch/x86/include/asm/pgtable.h
   @@ -355,7 +355,7 @@ static inline pte_t pte_modify(pte_t pte,
pgprot_t newprot)
            * the newprot (if present):
            */
           val &= _PAGE_CHG_MASK;
   -       val |= massage_pgprot(newprot) & ~_PAGE_CHG_MASK;
   +       val |= massage_pgprot(newprot);

           return __pte(val);
    }
   @@ -365,7 +365,7 @@ static inline pmd_t pmd_modify(pmd_t pmd,
pgprot_t newprot)
           pmdval_t val = pmd_val(pmd);

           val &= _HPAGE_CHG_MASK;
   -       val |= massage_pgprot(newprot) & ~_HPAGE_CHG_MASK;
   +       val |= massage_pgprot(newprot);

           return __pmd(val);
    }

instead, and remove the mkyoung. Completely untested, but that "just
or in the new protection bits" is what pnf_pte() does just a few lines
above this.

                        Linus

<Prev in Thread] Current Thread [Next in Thread>