From: Linus Torvalds <torvalds@linux-foundation.org>
To: Song Liu <songliubraving@fb.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@kernel.org>,
"Peter Zijlstra (Intel)" <peterz@infradead.org>,
Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
"the arch/x86 maintainers" <x86@kernel.org>
Subject: Re: [GIT pull] x86/pti for 5.4-rc1
Date: Tue, 17 Sep 2019 12:01:53 -0700 [thread overview]
Message-ID: <CAHk-=whbOhAmc3FEhnEkdM9AXFuKM+964r+uzzm_Q9qFaxTC7g@mail.gmail.com> (raw)
In-Reply-To: <CDB07BE3-6491-4159-89E7-FBA1631A01C3@fb.com>
On Tue, Sep 17, 2019 at 11:49 AM Song Liu <songliubraving@fb.com> wrote:
>
> I guess we need something like the following?
>
> diff --git i/arch/x86/mm/pti.c w/arch/x86/mm/pti.c
> index b196524759ec..7846916c3bcd 100644
> --- i/arch/x86/mm/pti.c
> +++ w/arch/x86/mm/pti.c
> @@ -306,6 +306,8 @@ pti_clone_pgtable(unsigned long start, unsigned long end,
> {
> unsigned long addr;
>
> + if (WARN_ON_ONCE(start & ~PAGE_MASK))
> + return;
I don't think we ever care about the low bits of the address below the
page mask, so this one probably wouldn't make any difference.
To match the other cases, I'd make it just a plain
WARN_ON_ONCE(start & ~PAGE_MASK));
and leave it at that. The existing commit added the warning, but then
just made the code work despite it all. I'd continue that same logic.
> if (pmd_large(*pmd) || level == PTI_CLONE_PMD) {
> + if (WARN_ON_ONCE(addr & ~PMD_MASK))
> + return;
> target_pmd = pti_user_pagetable_walk_pmd(addr);
> if (WARN_ON(!target_pmd))
> return;
Again, I think to match the other cases, I'd just do
- addr += PMD_SIZE;
+ WARN_ON_ONCE(addr & ~PMD_MASK);
+ addr = round_up(addr + 1, PMD_SIZE);
which now admittedly clones too much, but _does_ clone the requested range.
But maybe it really doesn't matter, since this condition just
shouldn't happen in the first place.
And arguably, the "clone more than requested" issue is true, and maybe
your "warn and refuse to clone by returning" is the right thing to do.
So I have very few strong opinions in this area, I just reacted to
looking at the patch that it didn't seem to cover all the cases.
> > Also, it would have been lovely to have some background on how this
> > was even noticed. The link in the commit message goes to the
> > development thread, but that one doesn't have the original report from
> > Song either.
>
> I didn't really notice any issue. I was debugging an increase in iTLB
> miss rate, which was caused by splitting of kernel text page table,
> and was fixed by Thomas in:
>
> https://lore.kernel.org/lkml/alpine.DEB.2.21.1908282355340.1938@nanos.tec.linutronix.de/
>
> I mistakenly suspected the issue was caused by the pti code, and
> mistakenly proposed the first patch here. It turned out to be useful,
> but it is not related to the original issue.
Ok, thanks for the explanation.
Linus
next prev parent reply other threads:[~2019-09-17 19:02 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-16 13:30 [GIT pull] irq/core for 5.4-rc1 Thomas Gleixner
2019-09-16 13:30 ` [GIT pull] timers/core " Thomas Gleixner
2019-09-17 20:15 ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] x86/pti " Thomas Gleixner
2019-09-17 18:13 ` Linus Torvalds
2019-09-17 18:48 ` Song Liu
2019-09-17 19:01 ` Linus Torvalds [this message]
2019-09-17 23:28 ` Song Liu
2019-09-17 23:35 ` Linus Torvalds
2019-09-18 10:40 ` Song Liu
2019-09-25 6:23 ` Ingo Molnar
2019-09-17 20:15 ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] timers/urgent " Thomas Gleixner
2019-09-17 20:15 ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] x86/apic " Thomas Gleixner
2019-09-17 20:15 ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] x86/irq " Thomas Gleixner
2019-09-17 20:15 ` pr-tracker-bot
2019-09-16 13:30 ` [GIT pull] smp/hotplug " Thomas Gleixner
2019-09-17 20:15 ` pr-tracker-bot
2019-09-17 20:15 ` [GIT pull] irq/core " pr-tracker-bot
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='CAHk-=whbOhAmc3FEhnEkdM9AXFuKM+964r+uzzm_Q9qFaxTC7g@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=songliubraving@fb.com \
--cc=tglx@linutronix.de \
--cc=x86@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.