From: Ingo Molnar <mingo@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
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 20:12:31 +0100 [thread overview]
Message-ID: <ZgMeH9p0BTnMfmOM@gmail.com> (raw)
In-Reply-To: <20240326174903.GA4539@redhat.com>
* Oleg Nesterov <oleg@redhat.com> wrote:
> On 03/20, Ingo Molnar wrote:
> >
> > --- a/arch/x86/kernel/fpu/init.c
> > +++ b/arch/x86/kernel/fpu/init.c
> > @@ -38,7 +38,7 @@ static void fpu__init_cpu_generic(void)
> > /* Flush out any pending x87 state: */
> > #ifdef CONFIG_MATH_EMULATION
> > if (!boot_cpu_has(X86_FEATURE_FPU))
> > - fpstate_init_soft(¤t->thread.fpu.fpstate->regs.soft);
> > + fpstate_init_soft(current->thread.fpu->fpstate->regs.soft);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Typo? it should be
>
> ¤t->thread.fpu->fpstate->regs.soft
Thx, fixed.
>
> > +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.
But the extra block is unnecessary in any case.
> > void __init fpu__init_system(void)
> > {
> > - fpstate_reset(¤t->thread.fpu);
> > fpu__init_system_early_generic();
> > + fpstate_reset(current->thread.fpu);
>
> It seems that fpstate_reset(current->thread.fpu) is not needed after the
> change in fpu__init_system_early_generic() above.
Yeah. Something like the delta patch below?
Thanks,
Ingo
=================>
arch/x86/kernel/fpu/init.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index f9412d012191..de618ec509aa 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -38,7 +38,7 @@ static void fpu__init_cpu_generic(void)
/* Flush out any pending x87 state: */
#ifdef CONFIG_MATH_EMULATION
if (!boot_cpu_has(X86_FEATURE_FPU))
- fpstate_init_soft(current->thread.fpu->fpstate->regs.soft);
+ fpstate_init_soft(¤t->thread.fpu->fpstate->regs.soft);
else
#endif
asm volatile ("fninit");
@@ -75,14 +75,12 @@ static struct fpu x86_init_fpu __read_mostly;
static void __init fpu__init_system_early_generic(void)
{
- {
- int this_cpu = smp_processor_id();
+ 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;
- }
+ 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;
if (!boot_cpu_has(X86_FEATURE_CPUID) &&
!test_bit(X86_FEATURE_FPU, (unsigned long *)cpu_caps_cleared)) {
@@ -222,7 +220,6 @@ static void __init fpu__init_system_xstate_size_legacy(void)
void __init fpu__init_system(void)
{
fpu__init_system_early_generic();
- fpstate_reset(current->thread.fpu);
/*
* The FPU has to be operational for some of the
next prev parent reply other threads:[~2024-03-26 19:12 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 [this message]
2024-03-26 19:13 ` [PATCH 1/1 -v2] " Ingo Molnar
2024-03-26 21:54 ` [PATCH 1/1] " Oleg Nesterov
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=ZgMeH9p0BTnMfmOM@gmail.com \
--to=mingo@kernel.org \
--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=oleg@redhat.com \
--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.