From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org,
Andy Lutomirski <luto@amacapital.net>,
Andrew Morton <akpm@linux-foundation.org>,
Dave Hansen <dave@sr71.net>,
Peter Zijlstra <peterz@infradead.org>,
Borislav Petkov <bp@alien8.de>, "H . Peter Anvin" <hpa@zytor.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
Uros Bizjak <ubizjak@gmail.com>
Subject: Re: [PATCH 1/1] headers/deps: x86/fpu: Make task_struct::thread constant size
Date: Tue, 26 Mar 2024 22:54:36 +0100 [thread overview]
Message-ID: <20240326215435.GB4539@redhat.com> (raw)
In-Reply-To: <ZgMeH9p0BTnMfmOM@gmail.com>
On 03/26, Ingo Molnar wrote:
>
> * Oleg Nesterov <oleg@redhat.com> wrote:
>
> > > +static struct fpu x86_init_fpu __read_mostly;
> > > +
> > > static void __init fpu__init_system_early_generic(void)
> > > {
> > > + {
> > > + int this_cpu = smp_processor_id();
> > > +
> > > + fpstate_reset(&x86_init_fpu);
> > > + current->thread.fpu = &x86_init_fpu;
> > > + per_cpu(fpu_fpregs_owner_ctx, this_cpu) = &x86_init_fpu;
> > > + x86_init_fpu.last_cpu = this_cpu;
> > > + }
> >
> > Can't x86_init_fpu be declared inside the block above?
>
> As a function-local static? I think globals are better defined in a visible
> fashion, not hidden among local variables where they are easy to overlook.
OK, but please see below.
> Yeah. Something like the delta patch below?
LGTM. And the whole patch too, although I don't think I can review it ;)
but... Ingo, let me ask 2 stupid questions.
1. Can you split this patch? I mean, 1/2 should add something like
struct fpu *x86_task_fpu(struct task_struct *task)
{
return &target->thread.fpu;
}
somewhere in arch/x86/include and update all the users of ->thread.fpu
to use the new helper. No functional changes.
This way 2/2 which actually turns .fpu into a pointer will be MUCH simpler.
2. Now, 2/2 should change the new helper above to
return target->thread.fpu;
But! why do we need the thread_struct.fpu pointer at all? Can't we simply have
struct fpu *x86_task_fpu(struct task_struct *task)
{
return (void *)task + sizeof(*task);
}
? This is what fpu_clone() (with your patch) does anyway.
Yes, this needs some changes:
- exit_thread() - trivial
- arch_dup_task_struct() clears thread.fpu, but I guess this is unnecessary
- fpu_clone() initializes dst->thread.fpu, no longer needed
- finally, fpu__init_system_early_generic() which initializes the
init_task.thread.fpu pointer.
But this doesn't look difficult? Although I don't understand the magic in
arch/x86/kernel/vmlinux.lds.S ...
Oleg.
next prev parent reply other threads:[~2024-03-26 21:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-20 13:19 [PATCH 0/1] Fast headers: Make task_struct::thread constant size Ingo Molnar
2024-03-20 13:19 ` [PATCH 1/1] headers/deps: x86/fpu: " Ingo Molnar
2024-03-25 6:00 ` kernel test robot
2024-03-26 17:49 ` Oleg Nesterov
2024-03-26 19:12 ` Ingo Molnar
2024-03-26 19:13 ` [PATCH 1/1 -v2] " Ingo Molnar
2024-03-26 21:54 ` Oleg Nesterov [this message]
2024-03-29 9:10 ` [PATCH 2/1] headers/deps: x86/fpu: Remove the thread::fpu pointer Ingo Molnar
2024-03-29 9:45 ` [PATCH 3/1] x86/fpu: Remove init_task FPU state dependencies, add debugging warning Ingo Molnar
2024-03-29 13:41 ` [tip: x86/fpu] x86/vm86: Make sure the free_vm86(task) definition uses its parameter even in the !CONFIG_VM86 case tip-bot2 for Ingo Molnar
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=20240326215435.GB4539@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=bp@alien8.de \
--cc=dave@sr71.net \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=ubizjak@gmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.