Messages in this thread | | | Date | Sat, 18 Jul 2015 05:40:24 +0200 | From | Ingo Molnar <> | Subject | Re: [RFC][PATCH] x86, fpu: dynamically allocate 'struct fpu' |
| |
* 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
| |