From: Peter Xu <peterx@redhat.com>
To: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Cc: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-alpha@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-hexagon@vger.kernel.org, linux-ia64@vger.kernel.org,
linux-m68k@lists.linux-m68k.org, Michal Simek <monstr@monstr.eu>,
linux-mips@vger.kernel.org, Nick Hu <nickhu@andestech.com>,
Ley Foon Tan <ley.foon.tan@intel.com>,
openrisc@lists.librecores.org, linux-parisc@vger.kernel.org,
linux-riscv@lists.infradead.org, linux-sh@vger.kernel.org,
sparclinux@vger.kernel.org, linux-um@lists.infradead.org,
Guan Xuetao <gxt@pku.edu.cn>,
linux-xtensa@linux-xtensa.org,
Heiko Carstens <heiko.carstens@de.ibm.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Andrea Arcangeli <aarcange@redhat.com>
Subject: Re: Possible duplicate page fault accounting on some archs after commit 4064b9827063
Date: Mon, 15 Jun 2020 17:34:31 -0400 [thread overview]
Message-ID: <20200615213431.GC111927@xz-x1> (raw)
In-Reply-To: <20200610165023.GA67179@xz-x1>
On Wed, Jun 10, 2020 at 12:50:23PM -0400, Peter Xu wrote:
> On Wed, Jun 10, 2020 at 05:48:11PM +0200, Gerald Schaefer wrote:
> > Hi,
>
> Hi, Gerald,
>
> >
> > Some architectures have their page fault accounting code inside the fault
> > retry loop, and rely on only going through that code once. Before commit
> > 4064b9827063 ("mm: allow VM_FAULT_RETRY for multiple times"), that was
> > ensured by testing for and clearing FAULT_FLAG_ALLOW_RETRY.
> >
> > That commit had to remove the clearing of FAULT_FLAG_ALLOW_RETRY for all
> > architectures, and introduced a subtle change to page fault accounting
> > logic in the affected archs. It is now possible to go through the retry
> > loop multiple times, and the affected archs would then account multiple
> > page faults instead of just one.
> >
> > This was found by coincidence in s390 code, and a quick check showed that
> > there are quite a lot of other architectures that seem to be affected in a
> > similar way. I'm preparing a fix for s390, by moving the accounting behind
> > the retry loop, similar to x86. It is not completely straight-forward, so
> > I leave the fix for other archs to the respective maintainers.
>
> Sorry for not noticing this before. The accounting part should definitely be
> put at least into a check against fault_flag_allow_retry_first() to mimic what
> was done before. And I agree it would be even better to put it after the retry
> logic, so if any of the page faults gets a major fault, it'll be accounted as a
> major fault which makes more sense to me, just like what x86 is doing now with:
>
> major |= fault & VM_FAULT_MAJOR;
>
> I'm not sure what's the preference of the arch maintainers, just let me know if
> it's preferred to use a single series to address this issue for all affected
> archs (or the archs besides s390), then I'll do.
To make sure this won't fall through the cracks... I'll give it a shot with a
single series to address this issue for all archs. Although it might not be
easy to do accounting directly in handle_mm_fault(), it might be still a chance
to introduce a helper so the accounting can be done in general code.
Thanks,
--
Peter Xu
next prev parent reply other threads:[~2020-06-15 21:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-10 15:48 Possible duplicate page fault accounting on some archs after commit 4064b9827063 Gerald Schaefer
2020-06-10 16:50 ` Peter Xu
2020-06-15 21:34 ` Peter Xu [this message]
2020-06-10 16:53 ` 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=20200615213431.GC111927@xz-x1 \
--to=peterx@redhat.com \
--cc=aarcange@redhat.com \
--cc=gerald.schaefer@de.ibm.com \
--cc=gxt@pku.edu.cn \
--cc=heiko.carstens@de.ibm.com \
--cc=ley.foon.tan@intel.com \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hexagon@vger.kernel.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux-mips@vger.kernel.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=linux-sh@vger.kernel.org \
--cc=linux-um@lists.infradead.org \
--cc=linux-xtensa@linux-xtensa.org \
--cc=monstr@monstr.eu \
--cc=nickhu@andestech.com \
--cc=openrisc@lists.librecores.org \
--cc=sparclinux@vger.kernel.org \
--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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).