From: Ingo Molnar <mingo@kernel.org>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>, X86 ML <x86@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Borislav Petkov <bp@alien8.de>,
Linus Torvalds <torvalds@linux-foundation.org>,
Thomas Garnier <thgarnie@google.com>
Subject: Re: [PATCH tip:x86/mm] x86/tls: Forcibly set the accessed bit in TLS segments
Date: Tue, 21 Mar 2017 08:24:50 +0100 [thread overview]
Message-ID: <20170321072450.GA18180@gmail.com> (raw)
In-Reply-To: <CALCETrVHm1ovumDyK6oGs=hQ74+GtkZ4z7ndsSENjEAtrUFGUg@mail.gmail.com>
* Andy Lutomirski <luto@amacapital.net> wrote:
> On Sat, Mar 18, 2017 at 10:17 PM, Andy Lutomirski <luto@kernel.org> wrote:
> > For mysterious historical reasons, struct user_desc doesn't indicate
> > whether segments are accessed. set_thread_area() has always
> > programmed segments as non-accessed, so the first write will set the
> > accessed bit. This will fault if the GDT is read-only.
> >
> > Fix it by making TLS segments start out accessed.
> >
> > If this ends up breaking something, we could, in principle, leave
> > TLS segments non-accessed and fix them up when we get the page
> > fault. I'd be surprised, though -- AFAIK all the nasty legacy
> > segmented programs (DOSEMU, Wine, things that run on DOSEMU and
> > Wine, etc.) do their nasty segmented things using the LDT and not
> > the GDT. I assume this is mainly because old OSes (Linux and
> > otherwise) didn't historically provide APIs to do nasty things in
> > the GDT.
> >
> > Fixes: 45fc8757d1d2 ("x86: Make the GDT remapping read-only on 64-bit")
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>
> FWIW, I'm now extra convinced that this won't break anything: the
> accessed bit didn't work properly before this patch. When we
> scheduled a task in, we'd copy the TLS segment descriptors to the GDT,
> but we never copied them back out when we scheduled out, so the
> accessed bit would randomly clear itself. Whoops :)
>
> So arguably this patch would be a bugfix even without Thomas' changes.
It's probably even a small speedup per scheduling atom, as we'd avoid dirtying the
GDT again and again, right?
On very high context switching rates it might even be measurable in principle, as
this ought to be the only thing that dirtied the (per CPU) GDT cacheline, so if
the workload is write bandwidth or store queue depth bound this change will
slightly improve things.
Thanks,
Ingo
prev parent reply other threads:[~2017-03-21 7:26 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-19 5:17 [PATCH tip:x86/mm] x86/tls: Forcibly set the accessed bit in TLS segments Andy Lutomirski
2017-03-19 11:24 ` [tip:x86/mm] " tip-bot for Andy Lutomirski
2017-03-21 5:18 ` [PATCH tip:x86/mm] " Andy Lutomirski
2017-03-21 7:24 ` Ingo Molnar [this message]
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=20170321072450.GA18180@gmail.com \
--to=mingo@kernel.org \
--cc=bp@alien8.de \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=luto@kernel.org \
--cc=thgarnie@google.com \
--cc=torvalds@linux-foundation.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox