Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1391317 > unrolled thread

Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Dave Hansen <dave.hansen@linux.intel.com> - 2016-04-29 20:10 +0200
    Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Ingo Molnar <mingo@kernel.org> - 2016-04-29 21:50 +0200
    Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Dave Hansen <dave.hansen@linux.intel.com> - 2016-04-29 22:10 +0200
      Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Yu-cheng Yu <yu-cheng.yu@intel.com> - 2016-04-29 22:20 +0200
        Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Andy Lutomirski <luto@amacapital.net> - 2016-04-29 22:30 +0200
          Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Dave Hansen <dave.hansen@linux.intel.com> - 2016-04-29 22:50 +0200
            Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Andy Lutomirski <luto@amacapital.net> - 2016-04-29 22:50 +0200
              Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Dave Hansen <dave.hansen@linux.intel.com> - 2016-04-30 00:50 +0200
            Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Ingo Molnar <mingo@kernel.org> - 2016-04-30 10:00 +0200
              Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Dave Hansen <dave.hansen@linux.intel.com> - 2016-05-02 18:30 +0200
                Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Ingo Molnar <mingo@kernel.org> - 2016-05-02 20:40 +0200
        Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Dave Hansen <dave.hansen@linux.intel.com> - 2016-04-29 22:40 +0200
    Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues Yu-cheng Yu <yu-cheng.yu@intel.com> - 2016-04-29 22:10 +0200

#1391317 — Re: [PATCH v4 0/10] x86/xsaves: Fix XSAVES known issues

Hi Folks,

I've heard through the grapevine that there's some concern that we
should not be bothering to enable XSAVES because there's not a
sufficient use case for it.  Maybe it's meager today, but I still think
we should do it.

I'll try to lay out why.

Today, on every Skylake system, this patch saves 128 bytes in each
task_struct.  If there were an Atom system with XSAVES it would save 384
bytes since there is no AVX support on Atom.  If there were a future
processor which has an xstate _past_ AVX-512, but that does not have
AVX-512 itself, that savings goes up to 2048+384 bytes.  I believe it is
*inevitable* that the savings will become substantial.

Plus, if the processors ever start supporting a supervisor state that we
_need_ in Linux, we have to XSAVES support anyway.

It's inevitable that we _will_ need it.

Why do it today?

Now that Skylake is out, we _can_ get reasonable testing of this feature
from early adopters in the wild.  If we turn this on today, and it
breaks, we break a relatively modest number of Skylake systems (1%? 2%?
0.1%?).  Let's say we wait $X years when the benefits are greater.  We
turn it on, and something breaks.  We'll break 50% (or 40% or whatever)
of the systems in production.

Once we *HAVE* XSAVES support, it also opens up the possibilities for
doing things like dynamic XSAVE buffer allocation.  For instance, let
threads that are not _using_ AVX-512 not waste the 2k of space for it.

So why wait?

[toc] | [next] | [standalone]

#1391366

* Dave Hansen <dave.hansen@linux.intel.com> wrote:

> Hi Folks,
> 
> I've heard through the grapevine that there's some concern that we
> should not be bothering to enable XSAVES because there's not a
> sufficient use case for it. [...]

So I have no fundamental objections against this series - I didn't apply it back 
in March because not all patches had your Reviewed-by tag. Basically after you 
sorted out all the XSAVE dynamic feature detection/sizing issues I was a happy 
camper and have no objection against XSAVES.

Could you please send a refreshed version against the latestest tip:master?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1391372

On 04/29/2016 12:57 PM, Yu-cheng Yu wrote:
> On Fri, Apr 29, 2016 at 11:09:23AM -0700, Dave Hansen wrote:
>> Once we *HAVE* XSAVES support, it also opens up the possibilities for
>> doing things like dynamic XSAVE buffer allocation.  For instance, let
>> threads that are not _using_ AVX-512 not waste the 2k of space for
>> it.
> 
> If we can somehow modify exec* system call to scan the executable
> binary (in user space) and pass along a bitmask containing xfeatures
> used in the binary, and XSAVES is enabled in the kernel, we can
> easily save a lot of memory.  The kernel only needs to allocate space
> for tasks that actually use xstates; most of them do not.

That's not feasible.  Think of dynamic libraries or just-in-time
compilers.  What instruction set does /usr/bin/java use, for instance? :)

[toc] | [prev] | [next] | [standalone]

#1391384

On Fri, Apr 29, 2016 at 01:03:43PM -0700, Dave Hansen wrote:
> That's not feasible.  Think of dynamic libraries or just-in-time
> compilers.  What instruction set does /usr/bin/java use, for instance? :)

The java argument is true. In that case or when the bitmask is missing, we can allocate for all supported features. 

[toc] | [prev] | [next] | [standalone]

#1391390

On Fri, Apr 29, 2016 at 1:07 PM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> On Fri, Apr 29, 2016 at 01:03:43PM -0700, Dave Hansen wrote:
>> That's not feasible.  Think of dynamic libraries or just-in-time
>> compilers.  What instruction set does /usr/bin/java use, for instance? :)
>
> The java argument is true. In that case or when the bitmask is missing, we can allocate for all supported features.
>

I actually want to see us moving in the direction of unconditionally
allocating everything on process startup.  If we can stop using CR0.TS
entirely, I think everything will be better.

--Andy

[toc] | [prev] | [next] | [standalone]

#1391420

On 04/29/2016 01:25 PM, Andy Lutomirski wrote:
> On Fri, Apr 29, 2016 at 1:07 PM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>> On Fri, Apr 29, 2016 at 01:03:43PM -0700, Dave Hansen wrote:
>>> That's not feasible.  Think of dynamic libraries or just-in-time
>>> compilers.  What instruction set does /usr/bin/java use, for instance? :)
>>
>> The java argument is true. In that case or when the bitmask is
>> missing, we can allocate for all supported features.
> 
> I actually want to see us moving in the direction of unconditionally
> allocating everything on process startup.  If we can stop using CR0.TS
> entirely, I think everything will be better.

We can absolutely allocate the worst-case XSAVE buffer at task startup
for folks that never want to see a latency spike in the life of the app
no matter what.

But I also think it would be pretty nice if 'ls' didn't pay the 2k cost
to have AVX-512 state if it's not using AVX-512.  We also don't have to
do this with CR0.TS.  We'd actually use a combination of out-of-line
(not appended to task_struct) XSAVE buffers and XGETBV1 to check the
size of our XSAVE buffer before we call XSAVE* and resize it when needed.

Maybe nobody will ever care enough about 2kbytes/thread, though.

[toc] | [prev] | [next] | [standalone]

#1391422

On Fri, Apr 29, 2016 at 1:40 PM, Dave Hansen
<dave.hansen@linux.intel.com> wrote:
> On 04/29/2016 01:25 PM, Andy Lutomirski wrote:
>> On Fri, Apr 29, 2016 at 1:07 PM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>>> On Fri, Apr 29, 2016 at 01:03:43PM -0700, Dave Hansen wrote:
>>>> That's not feasible.  Think of dynamic libraries or just-in-time
>>>> compilers.  What instruction set does /usr/bin/java use, for instance? :)
>>>
>>> The java argument is true. In that case or when the bitmask is
>>> missing, we can allocate for all supported features.
>>
>> I actually want to see us moving in the direction of unconditionally
>> allocating everything on process startup.  If we can stop using CR0.TS
>> entirely, I think everything will be better.
>
> We can absolutely allocate the worst-case XSAVE buffer at task startup
> for folks that never want to see a latency spike in the life of the app
> no matter what.
>
> But I also think it would be pretty nice if 'ls' didn't pay the 2k cost
> to have AVX-512 state if it's not using AVX-512.  We also don't have to
> do this with CR0.TS.  We'd actually use a combination of out-of-line
> (not appended to task_struct) XSAVE buffers and XGETBV1 to check the
> size of our XSAVE buffer before we call XSAVE* and resize it when needed.
>
> Maybe nobody will ever care enough about 2kbytes/thread, though.

I suspect we're so far about 2k/thread that no one cares.

That being said, when I wrote this email, I wasn't thinking about
compacted form at all.  I think we should allocate a viable xstate
area of some sort on startup and use saves/xrstors/xsaveopt/whatever
without fiddling with TS and eagerly save and restore even if no
extended state whatsoever has been used.  I'm certainly okay in
principle with reallocating.

However, what do we do if we run out when memory when trying to reallocate?

-- 
Andy Lutomirski
AMA Capital Management, LLC

[toc] | [prev] | [next] | [standalone]

#1391513

On 04/29/2016 01:49 PM, Andy Lutomirski wrote:
>> >
>> > But I also think it would be pretty nice if 'ls' didn't pay the 2k cost
>> > to have AVX-512 state if it's not using AVX-512.  We also don't have to
>> > do this with CR0.TS.  We'd actually use a combination of out-of-line
>> > (not appended to task_struct) XSAVE buffers and XGETBV1 to check the
>> > size of our XSAVE buffer before we call XSAVE* and resize it when needed.
>> >
>> > Maybe nobody will ever care enough about 2kbytes/thread, though.
> I suspect we're so far about 2k/thread that no one cares.
> 
...
> However, what do we do if we run out when memory when trying to reallocate?

The thread has to die a horrible death.  We can switch away from it, but
can never switch back to it.  Well we can switch to it, we just can't
return to userspace.

Actually, though...  On my laptop 1/3 of the task_struct is XSAVE state
(it's ~3k).  With AVX-512, ~3/5 of it will be XSAVE, and it will be ~5k
(>1 page).  Breaking it in to two pieces makes it overall less likely
that we'd have to fail an allocation.

We'd be in a situation where we probably can't fork *anyway* if that
happened.

[toc] | [prev] | [next] | [standalone]

#1391579

* Dave Hansen <dave.hansen@linux.intel.com> wrote:

> But I also think it would be pretty nice if 'ls' didn't pay the 2k cost to have 
> AVX-512 state if it's not using AVX-512. [...]

A C library might decide to use AVX-512 memset(). RAM is cheap, while allocation 
complexity, especially in the kernel, has various other costs.

I mean, we should not worry about per thread allocation sizes that can be compared 
to the kernel stack size.

We can still use the compacted area handling instructions, because presumably 
those are the fastest and are also the most optimized ones? But I wouldn't use 
them to do dynamic allocation: just allocate the maximum possible FPU save area at 
task creation time and never again worry about that detail.

Ok?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1392357

On 04/30/2016 12:53 AM, Ingo Molnar wrote:
> We can still use the compacted area handling instructions, because presumably 
> those are the fastest and are also the most optimized ones? But I wouldn't use 
> them to do dynamic allocation: just allocate the maximum possible FPU save area at 
> task creation time and never again worry about that detail.
> 
> Ok?

Sounds sane to me.

BTW, I hacked up your "fpu performance" to compare XSAVE vs. XSAVES:

> [    0.048347] x86/fpu: Cost of: XSAVE                       insn          :   127 cycles
> [    0.049134] x86/fpu: Cost of: XSAVES                      insn          :   113 cycles
> [    0.048492] x86/fpu: Cost of: XRSTOR                      insn          :   120 cycles
> [    0.049267] x86/fpu: Cost of: XRSTORS                     insn          :   102 cycles

So I guess we can add that to the list of things that XSAVES is good
for.  Granted, the real-world benefit is probably hard to measure
because the cache residency of the XSAVE buffer isn't as good when
_actually_ context switching, but this at least shows a small
theoretical advantage for XSAVES.

[toc] | [prev] | [next] | [standalone]

#1392442

* Dave Hansen <dave.hansen@linux.intel.com> wrote:

> On 04/30/2016 12:53 AM, Ingo Molnar wrote:
> > We can still use the compacted area handling instructions, because presumably 
> > those are the fastest and are also the most optimized ones? But I wouldn't use 
> > them to do dynamic allocation: just allocate the maximum possible FPU save area at 
> > task creation time and never again worry about that detail.
> > 
> > Ok?
> 
> Sounds sane to me.
> 
> BTW, I hacked up your "fpu performance" to compare XSAVE vs. XSAVES:
> 
> > [    0.048347] x86/fpu: Cost of: XSAVE                       insn          :   127 cycles
> > [    0.049134] x86/fpu: Cost of: XSAVES                      insn          :   113 cycles
> > [    0.048492] x86/fpu: Cost of: XRSTOR                      insn          :   120 cycles
> > [    0.049267] x86/fpu: Cost of: XRSTORS                     insn          :   102 cycles
> 
> So I guess we can add that to the list of things that XSAVES is good for.

Absolutely!

> [...]  Granted, the real-world benefit is probably hard to measure because the 
> cache residency of the XSAVE buffer isn't as good when _actually_ context 
> switching, but this at least shows a small theoretical advantage for XSAVES.

Yeah, and anything that was measured for real is far from being theoretical. It's 
simply a best-case microbenchmark figure, but it's still a nice 10+ cycles 
improvement overall - which might become bigger in future CPU generations.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1391410

On 04/29/2016 01:07 PM, Yu-cheng Yu wrote:
> On Fri, Apr 29, 2016 at 01:03:43PM -0700, Dave Hansen wrote:
>>> That's not feasible.  Think of dynamic libraries or just-in-time 
>>> compilers.  What instruction set does /usr/bin/java use, for
>>> instance? :)
> The java argument is true. In that case or when the bitmask is
> missing, we can allocate for all supported features.

Remember, execve() doesn't replace the task_struct.  How do we resize
the task_struct at execve() time?  If /bin/bash doesn't use AVX, then
fork()s and execve()s an AVX-using program, what do we do?

[toc] | [prev] | [next] | [standalone]

#1391377

On Fri, Apr 29, 2016 at 11:09:23AM -0700, Dave Hansen wrote:

> Once we *HAVE* XSAVES support, it also opens up the possibilities for
> doing things like dynamic XSAVE buffer allocation.  For instance, let
> threads that are not _using_ AVX-512 not waste the 2k of space for it.

If we can somehow modify exec* system call to scan the executable binary (in user space) and pass along a bitmask containing xfeatures used in the binary, and XSAVES is enabled in the kernel, we can easily save a lot of memory.  The kernel only needs to allocate space for tasks that actually use xstates; most of them do not. 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web