All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Thorsten Blum <thorsten.blum@linux.dev>
Cc: Waiman Long <longman@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: x86/smpboot: Question regarding native_play_dead() __noreturn warning
Date: Mon, 27 Oct 2025 13:50:45 +0100	[thread overview]
Message-ID: <20251027125045.GX4067720@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <47A8B84B-2685-4DA2-B39B-E55812374426@linux.dev>

On Mon, Oct 27, 2025 at 01:23:02PM +0100, Thorsten Blum wrote:
> Hi,
> 
> I just came across this comment in arch/x86/kernel/smpboot.c:
> 
> /*
> * native_play_dead() is essentially a __noreturn function, but it can't
> * be marked as such as the compiler may complain about it.
> */
> void native_play_dead(void) {
> 	...
> }
> 
> and when I mark native_play_dead() as __noreturn, neither gcc nor clang
> complain about it.
> 
> The commit message 2743fe89d4d4 ("x86/idle: Disable IBRS when CPU is
> offline to improve single-threaded performance") says:
> 
> "Add a comment to say that native_play_dead() is a __noreturn function,
> but it can't be marked as such to avoid confusion about the missing
> MSR restoration code."
> 
> Unfortunately, that doesn't really help me either. Can someone explain
> what the issue was and if the comment is still valid? Otherwise, I'd
> like to submit a patch adding __noreturn and removing the comment.

I'm not sure either, it wasn't there in v2 but appeared in v3.

v2: 20230620140625.1001886-3-longman@redhat.com
v3: 20230622003603.1188364-2-longman@redhat.com

The difference is that v2 tried to restore the msr after 'play_dead'
which is silly, since it would never reach that code. v3 removed that
dead restore code and added the confusing comment.

There is a clue here though:

  20230622054053.uy577qezu5a65buc@treble

Josh suggests play_dead() should be marked noreturn (which it is in
current kernels).

Waiman then replies:

  921e1b98-af36-1f51-5abe-dea36425b706@redhat.com

which is utterly confused again.

  reply	other threads:[~2025-10-27 12:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-27 12:23 x86/smpboot: Question regarding native_play_dead() __noreturn warning Thorsten Blum
2025-10-27 12:50 ` Peter Zijlstra [this message]
2025-10-27 15:48   ` Waiman Long
2025-10-27 16:23     ` Thorsten Blum

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=20251027125045.GX4067720@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=thorsten.blum@linux.dev \
    --cc=x86@kernel.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.