[Top] [All Lists]

Re: [PATCH 2/2] mm: numa: Do not clear PTEs or PMDs for NUMA hinting fau

To: Mel Gorman <mgorman@xxxxxxx>
Subject: Re: [PATCH 2/2] mm: numa: Do not clear PTEs or PMDs for NUMA hinting faults
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Thu, 5 Mar 2015 16:53:09 -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=RMrpitC8vHEXl5aPWSLijoPP5WzTUao7zHKZ2+Kc1B8=; b=gqX4ZnJql9rGiZQvhjoXkn8b8OV895F1KNR7tObSl6gPskmXKAhfTEd+LGLiBpOaHO xsjKTEbSr3vlrL7JS9JWjifxaOG4l9vYZXukE/QmnQSyhhu02JFCoucagvtJCuV+v5LX TfyGelJLeoiW6hM8FdFlvOl0uRrDTHkaPRqzcXpewlubDnUry3sJcOMLoosU6+FEujgY 6zmU2u4CdbBNvfscsUbTqr1tXNpmb1ugVclaEDfjZJ4hnE9GrepKbN/HplQBSet42y25 Ti1eoi0/lS+LZ7N9X7v0Vj4XmkUkujvHVuz314wPFRjly1G6fUDadsrA1VtwFpoympxt 36Xg==
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=RMrpitC8vHEXl5aPWSLijoPP5WzTUao7zHKZ2+Kc1B8=; b=YWNMp0ymCa9fqSRMz0aM6PNVIcxP3DIZP/s8u8jIakSnCNoGWYehtHRx6lR8K5pcFY NG8WIbfbAJKpBD2icLQzI/mY5U9eZtkzOfn9X+ioxpKhanUG9n5SRDYuD32jqb616UtI hvZoZsnSQ1ZRBG64AMX7eowajDXO9E97mEaLE=
In-reply-to: <1425599692-32445-3-git-send-email-mgorman@xxxxxxx>
References: <1425599692-32445-1-git-send-email-mgorman@xxxxxxx> <1425599692-32445-3-git-send-email-mgorman@xxxxxxx>
Sender: linus971@xxxxxxxxx
On Thu, Mar 5, 2015 at 3:54 PM, Mel Gorman <mgorman@xxxxxxx> wrote:
>                 if (!prot_numa || !pmd_protnone(*pmd)) {
> -                       entry = pmdp_get_and_clear_notify(mm, addr, pmd);
> -                       entry = pmd_modify(entry, newprot);
> +                       /*
> +                        * NUMA hinting update can avoid a clear and defer the
> +                        * flush as it is not a functional correctness issue 
> if
> +                        * access occurs after the update and this avoids
> +                        * spurious faults.
> +                        */
> +                       if (prot_numa) {
> +                               entry = *pmd;
> +                               entry = pmd_mkprotnone(entry);
> +                       } else {
> +                               entry = pmdp_get_and_clear_notify(mm, addr,
> +                                                                 pmd);
> +                               entry = pmd_modify(entry, newprot);
> +                               BUG_ON(pmd_write(entry));
> +                       }
> +
>                         ret = HPAGE_PMD_NR;
>                         set_pmd_at(mm, addr, pmd, entry);
> -                       BUG_ON(pmd_write(entry));

So I don't think this is right, nor is the new pte code.

You cannot just read the old pte entry, change it, and write it back.
That's fundamentally racy, and can drop any concurrent dirty or
accessed bit setting. And there are no locks you can use to protect
against that, since the accessed and dirty bit are set by hardware.

Now, losing the accessed bit doesn't matter - it's a small race, and
not a correctness issue. But potentially losing dirty bits is a data
loss problem.

Did the old prot_numa code do this too? Because if it did, it sounds
like it was just buggy.


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