linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Hugh Dickins <hughd@google.com>
Cc: linux-mm@kvack.org, Bibo Mao <maobibo@loongson.cn>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Huang Pei <huangpei@loongson.cn>,
	linux-arch@vger.kernel.org, linux-mips@vger.kernel.org
Subject: Re: [PATCH v3 3/3] mm: optimise pte dirty/accessed bit setting by demand based pte insertion
Date: Tue, 22 Dec 2020 13:24:53 +1000	[thread overview]
Message-ID: <1608606460.clzumasfvm.astroid@bobo.none> (raw)
In-Reply-To: <alpine.LSU.2.11.2012211000260.1880@eggly.anvils>

Excerpts from Hugh Dickins's message of December 22, 2020 4:21 am:
> Hi Nick,
> 
> On Sun, 20 Dec 2020, Nicholas Piggin wrote:
> 
>> Similarly to the previous patch, this tries to optimise dirty/accessed
>> bits in ptes to avoid access costs of hardware setting them.
>> 
>> This tidies up a few last cases where dirty/accessed faults can be seen,
>> and subsumes the pte_sw_mkyoung helper -- it's not just architectures
>> with explicit software dirty/accessed bits that take expensive faults to
>> modify ptes.
>> 
>> The vast majority of the remaining dirty/accessed faults on kbuild
>> workloads after this patch are from NUMA migration, due to
>> remove_migration_pte inserting old/clean ptes.
> 
> Are you sure about this patch? It looks wrong to me: because isn't
> _PAGE_ACCESSED (young) already included in the vm_page_prot __S001 etc?
> 
> I haven't checked all instances below, but in general, that's why the
> existing code tends not to bother to mkyoung, but does sometimes mkold
> (admittedly confusing).

There might have been one or two cases where it didn't come directly
from vm_page_prot, but it was a few rebases and updates ago. I did see
one or two places where powerpc was taking faults. Good point though I 
can test again and see, and I might split the patch.

> 
> A quick check on x86 and powerpc looks like they do have _PAGE_ACCESSED
> in there; and IIRC that's true, or ought to be true, of all architectures.
> 
> Maybe not mips, which I see you have singled out: I remember going round
> this loop a few months ago with mips, where strange changes were being
> proposed to compensate for not having that bit in their vm_page_prot.
> I didn't follow through to see how that ended up, but I did suggest
> mips needed to follow the same convention as the other architectures.

Yeah the main thing is to try get all architectures doing the same thing
and get rid of that sw_mkyoung too. Given the (Intel) x86 result of the
heavy micro-fault, I don't think anybody is special and we should 
require them to follow the same behaviour unless it's proven that one
needs something different.

If we can get all arch to set accessed in vm_page_prot and get rid of 
most of these mkyoung()s then all the better. And it actually looks like
MIPS may be changing direction:

https://lore.kernel.org/linux-arch/20200919074731.22372-1-huangpei@loongson.cn/

What's the chances of that going upstream for the next merge window? It
seems like the right thing to do.

Thanks,
Nick


  reply	other threads:[~2020-12-22  3:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-20  4:55 [PATCH v3 0/3] mm: improve pte updates and dirty/accessed Nicholas Piggin
2020-12-20  4:55 ` [PATCH v3 1/3] mm/cow: don't bother write protecting already write-protected huge pages Nicholas Piggin
2020-12-20  4:55 ` [PATCH v3 2/3] mm/cow: optimise pte accessed bit handling in fork Nicholas Piggin
2020-12-20  4:55 ` [PATCH v3 3/3] mm: optimise pte dirty/accessed bit setting by demand based pte insertion Nicholas Piggin
2020-12-21 18:21   ` Hugh Dickins
2020-12-22  3:24     ` Nicholas Piggin [this message]
2020-12-23  0:56       ` Huang Pei
2020-12-20 18:00 ` [PATCH v3 0/3] mm: improve pte updates and dirty/accessed Linus Torvalds

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=1608606460.clzumasfvm.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=huangpei@loongson.cn \
    --cc=hughd@google.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=maobibo@loongson.cn \
    --cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox