All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Andy Lutomirski <luto@amacapital.net>,
	Dave Hansen <dave@sr71.net>, Brian Gerst <brgerst@gmail.com>,
	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>,
	"Chang S . Bae" <chang.seok.bae@intel.com>
Subject: Re: [PATCH -v5 0/8] sched: Make task_struct::thread constant size, x86/fpu: Remove thread::fpu
Date: Tue, 22 Apr 2025 18:11:08 +0200	[thread overview]
Message-ID: <20250422161108.GA1676@redhat.com> (raw)
In-Reply-To: <20250409211127.3544993-1-mingo@kernel.org>

Ingo, sorry for delay.

So just in case, the whole series looks good to me. I am going to send a
couple of minor cleanups on top of it, but let me ask first if I missed
something or not.

- x86_init_fpu is not really used after 4/8, it can be killed

- DEFINE_EVENT(x86_fpu, x86_fpu_copy_src) can be killed after 7/8

- arch_dup_task_struct() still does

	/* init_task is not dynamically sized (incomplete FPU state) */
	if (unlikely(src == &init_task))
		memcpy_and_pad(dst, arch_task_struct_size, src, sizeof(init_task), 0);
	else
		memcpy(dst, src, arch_task_struct_size);

  and I don't understand why do we need to check src == &init_task. It seems
  that we can always do

	memcpy_and_pad(dst, arch_task_struct_size, src, sizeof(struct task_struct), 0);

  or even just

	memcpy(dst, src, sizeof(struct task_struct));

  fpu_clone() will initialize the "dst_fpu" memory correctly.

- fpu__drop() does

	/* PF_KTHREAD tasks do not use the FPU context area: */
	if (tsk->flags & (PF_KTHREAD | PF_USER_WORKER))
		return;

  and this is correct. But perhaps

	if (test_tsk_thread_flag(tsk, TIF_NEED_FPU_LOAD))
		return;

  makes more sense? PF_KTHREAD's should never clear TIF_NEED_FPU_LOAD,
  and this way we can avoid the unnecessary "fwait" if, say, the exiting
  task does context_switch() at least once on its way to exit_thread().

- Finally, with or without these changes, it seems that the
  switch_fpu_prepare() + switch_fpu_finish() logic can be simplified,
  I'll write another email.


  parent reply	other threads:[~2025-04-22 16:11 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-09 21:11 [PATCH -v5 0/8] sched: Make task_struct::thread constant size, x86/fpu: Remove thread::fpu Ingo Molnar
2025-04-09 21:11 ` [PATCH 1/8] x86/fpu: Introduce the x86_task_fpu() helper method Ingo Molnar
2025-04-14  7:34   ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-09 21:11 ` [PATCH 2/8] x86/fpu: Convert task_struct::thread.fpu accesses to use x86_task_fpu() Ingo Molnar
2025-04-14  7:34   ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-09 21:11 ` [PATCH 3/8] x86/fpu: Make task_struct::thread constant size Ingo Molnar
2025-04-14  7:34   ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-09 21:11 ` [PATCH 4/8] x86/fpu: Remove the thread::fpu pointer Ingo Molnar
2025-04-10  7:39   ` Peter Zijlstra
2025-04-10 10:10     ` Ingo Molnar
2025-04-10 10:30       ` Peter Zijlstra
2025-04-10 10:54         ` [PATCH] x86/fpu: Clarify FPU context cacheline alignment Ingo Molnar
2025-04-14  7:34           ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-10 10:51       ` [PATCH 4/8] x86/fpu: Remove the thread::fpu pointer Ingo Molnar
2025-04-10 14:04       ` Oleg Nesterov
2025-04-14  7:34   ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-09 21:11 ` [PATCH 5/8] x86/fpu: Push 'fpu' pointer calculation into the fpu__drop() call Ingo Molnar
2025-04-14  7:34   ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-09 21:11 ` [PATCH 6/8] x86/fpu: Make sure x86_task_fpu() doesn't get called for PF_KTHREAD|PF_USER_WORKER tasks during exit Ingo Molnar
2025-04-11 15:22   ` Chang S. Bae
2025-04-12  8:37     ` Ingo Molnar
2025-04-14  7:34   ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-09 21:11 ` [PATCH 7/8] x86/fpu: Remove init_task FPU state dependencies, add debugging warning for PF_KTHREAD tasks Ingo Molnar
2025-04-14  7:34   ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-09 21:11 ` [PATCH 8/8] x86/fpu: Use 'fpstate' variable names consistently Ingo Molnar
2025-04-14  7:34   ` [tip: x86/merge] " tip-bot2 for Ingo Molnar
2025-04-22 16:11 ` Oleg Nesterov [this message]
2025-04-22 20:09   ` [PATCH -v5 0/8] sched: Make task_struct::thread constant size, x86/fpu: Remove thread::fpu Ingo Molnar
2025-04-22 17:01 ` question about switch_fpu_prepare/switch_fpu_finish Oleg Nesterov
2025-04-22 20:11   ` Ingo Molnar
2025-05-03 14:38 ` [PATCH tip/x86/fpu 1/6] x86/fpu: simplify the switch_fpu_prepare() + switch_fpu_finish() logic Oleg Nesterov
2025-05-04  8:54   ` [tip: x86/fpu] x86/fpu: Simplify " tip-bot2 for Oleg Nesterov
2025-05-03 14:38 ` [PATCH tip/x86/fpu 2/6] x86/fpu: kill x86_init_fpu Oleg Nesterov
2025-05-04  8:54   ` [tip: x86/fpu] x86/fpu: Remove x86_init_fpu tip-bot2 for Oleg Nesterov
2025-05-03 14:38 ` [PATCH tip/x86/fpu 3/6] x86/fpu: kill DEFINE_EVENT(x86_fpu, x86_fpu_copy_src) Oleg Nesterov
2025-05-04  8:54   ` [tip: x86/fpu] x86/fpu: Remove " tip-bot2 for Oleg Nesterov
2025-05-03 14:38 ` [PATCH tip/x86/fpu 4/6] x86/fpu: arch_dup_task_struct: always use memcpy_and_pad() Oleg Nesterov
2025-05-04  8:54   ` [tip: x86/fpu] x86/fpu: Always use memcpy_and_pad() in arch_dup_task_struct() tip-bot2 for Oleg Nesterov
2025-05-03 14:38 ` [PATCH tip/x86/fpu 5/6] x86/fpu: fpu__drop: check TIF_NEED_FPU_LOAD instead of PF_KTHREAD|PF_USER_WORKER Oleg Nesterov
2025-05-04  8:54   ` [tip: x86/fpu] x86/fpu: Check TIF_NEED_FPU_LOAD instead of PF_KTHREAD|PF_USER_WORKER in fpu__drop() tip-bot2 for Oleg Nesterov
2025-05-03 14:39 ` [PATCH tip/x86/fpu 6/6] x86/fpu: shift fpregs_assert_state_consistent() from arch_exit_work() to its caller Oleg Nesterov
2025-05-04  8:36   ` Ingo Molnar
2025-05-04  8:54   ` [tip: x86/fpu] x86/fpu: Shift " tip-bot2 for Oleg Nesterov

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=20250422161108.GA1676@redhat.com \
    --to=oleg@redhat.com \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=chang.seok.bae@intel.com \
    --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 \
    /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.