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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALCETrUfyOWErpQgjyKdmbdJy4RhF3Kz5VtieUQP1vxduox4rw@mail.gmail.com>
Date:	Thu, 16 Jul 2015 12:25:14 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Dave Hansen <dave@...1.net>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, X86 ML <x86@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Borislav Petkov <bp@...en8.de>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH] x86, fpu: dynamically allocate 'struct fpu'

On Thu, Jul 16, 2015 at 12:14 PM, Dave Hansen <dave@...1.net> 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?

Looks generally sensible.  Minor nitpicking below.

> +#define CHECK_MEMBER_AT_END_OF(TYPE, MEMBER)   \
> +       BUILD_BUG_ON((sizeof(TYPE) -                    \
> +                       offsetof(TYPE, MEMBER) -        \
> +                       sizeof(((TYPE *)0)->MEMBER)) >  \
> +                       0)                              \
> +

You could save a bit of typing by using offsetofend here.  Something
along the lines of BUILD_BUG_ON(sizeof(TYPE) != offsetofend(TYPE,
MEMBER));

>  #endif /* _ASM_X86_FPU_H */
> diff -puN arch/x86/kernel/process.c~dynamically-allocate-struct-fpu arch/x86/kernel/process.c
> --- a/arch/x86/kernel/process.c~dynamically-allocate-struct-fpu 2015-07-16 10:50:42.360571875 -0700
> +++ b/arch/x86/kernel/process.c 2015-07-16 12:00:59.204808551 -0700
> @@ -81,7 +81,7 @@ EXPORT_SYMBOL_GPL(idle_notifier_unregist
>   */
>  int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
>  {
> -       *dst = *src;
> +       memcpy(dst, src, arch_task_struct_size());

This is actually vaguely performance-critical, which makes me thing
that using some kind of inline or other real way (config macro, ifdef,
etc) to detect whether there's an arch override would be better than a
weak function.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/