Re: [RFC][PATCH] x86, fpu: dynamically allocate 'struct fpu'

[Ingo Molnar <mingo@kernel.org>]

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 07/16/2015 12:14 PM, Dave Hansen wrote:
> > The FPU rewrite removed the dynamic allocations of 'struct fpu'.
> > But, this potentially wastes massive amounts of memory (2k per
> > task on systems that do not have AVX-512 for instance).
> > 
> > Instead of having a separate slab, this patch just appends the
> > space that we need to the 'task_struct' which we dynamically
> > allocate already.  This saves from doing an extra slab allocation
> > at fork().  The only real downside here is that we have to stick
> > everything and the end of the task_struct.  But, I think the
> > BUILD_BUG_ON()s I stuck in there should keep that from being too
> > fragile.
> > 
> > This survives a quick build and boot in a VM.  Does anyone see any
> > real downsides to this?
> 
> No.  I have also long advocated for merging task_struct and thread_info into a 
> common structure and get it off the stack; it would improve security and avoid 
> weird corner cases in the irqstack handling.

Note that we have 3 related 'task state' data structures with overlapping purpose:

  task_struct
   thread_struct
  thread_info

where thread_struct is embedded in task_struct currently.

So to turn it all into a single structure we'd have to merge thread_info into 
thread_struct. thread_info was put on the kernel stack due to the ESP trick we 
played long ago - but that is moot these days.

So I think what we want is not some common structure, but to actually merge all of 
thread_info into thread_struct for arch details and into task_struct for generic 
fields, and only have:

  task_struct                /* generic fields */
   thread_struct             /* arch details */

this can be done gradually, field by field, and in the end thread_info can be 
eliminated altogether.

The only real complication is that it affects every architecture. The good news is 
that most of the thread_info layout details are wrapped in various constructs like 
test_ti_thread_flag() and task_thread_info().

While at it we might as well rename 'thread_struct' to 'arch_task_struct':

  task_struct                  /* generic fields */
  arch_task_struct             /* arch details */

to make it really clear and easy to understand at a glance - as the current naming 
is has become ambiguous and slightly confusing the moment we introduced threading.

Thanks,

	Ingo