x86/smpboot: Question regarding native_play_dead() __noreturn warning

x86/smpboot: Question regarding native_play_dead() __noreturn warning

[Thorsten Blum]

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.

Thanks,
Thorsten

Re: x86/smpboot: Question regarding native_play_dead() __noreturn warning

[Peter Zijlstra]

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.

Re: x86/smpboot: Question regarding native_play_dead() __noreturn warning

[Waiman Long]

On 10/27/25 8:50 AM, Peter Zijlstra wrote:
> 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.

I don't remember exactly how I got the warning when __noreturn is added 
to native_play_dead(). It may be a limitation of the objtool or gcc that 
I was using at that time. If Thorsten doesn't have problem adding 
__noreturn, I won't mind him doing that and taking out the the comment. 
We can see if there is other issue coming up in the future.

Cheers,
Longman

Re: x86/smpboot: Question regarding native_play_dead() __noreturn warning

[Thorsten Blum]

On 27. Oct 2025, at 16:48, Waiman Long wrote:
> On 10/27/25 8:50 AM, Peter Zijlstra wrote:
>> On Mon, Oct 27, 2025 at 01:23:02PM +0100, Thorsten Blum wrote:
>>> [...]
>> 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.
> 
> I don't remember exactly how I got the warning when __noreturn is added to native_play_dead(). It may be a limitation of the objtool or gcc that I was using at that time. If Thorsten doesn't have problem adding __noreturn, I won't mind him doing that and taking out the the comment. We can see if there is other issue coming up in the future.

Thanks for the quick replies!

I submitted the patch adding __noreturn here:

https://lore.kernel.org/lkml/20251027155107.183136-1-thorsten.blum@linux.dev/