Re: objtool warning for next-20221118

From: Juergen Gross
Date: Fri Nov 25 2022 - 00:31:29 EST


On 24.11.22 17:39, Josh Poimboeuf wrote:
On Thu, Nov 24, 2022 at 08:47:47AM +0100, Juergen Gross wrote:
+++ b/arch/x86/xen/smp_pv.c
@@ -385,17 +385,9 @@ static void xen_pv_play_dead(void) /* used only
with HOTPLUG_CPU */
  {
      play_dead_common();
      HYPERVISOR_vcpu_op(VCPUOP_down, xen_vcpu_nr(smp_processor_id()), NULL);
-    cpu_bringup();
-    /*
-     * commit 4b0c0f294 (tick: Cleanup NOHZ per cpu data on cpu down)
-     * clears certain data that the cpu_idle loop (which called us
-     * and that we return from) expects. The only way to get that
-     * data back is to call:
-     */
-    tick_nohz_idle_enter();
-    tick_nohz_idle_stop_tick_protected();
-    cpuhp_online_idle(CPUHP_AP_ONLINE_IDLE);
+    /* FIXME: converge cpu_bringup_and_idle() and start_secondary() */
+    cpu_bringup_and_idle();

I think this will leak stack memory. Multiple cpu offline/online cycles of
the same cpu will finally exhaust the idle stack.

Doh! Of course...

I was actually thinking ahead, to where eventually xen_pv_play_dead()
can call start_cpu0(), which can be changed to automatically reset the
stack pointer like this:

SYM_CODE_START(start_cpu0)
ANNOTATE_NOENDBR
UNWIND_HINT_EMPTY
movq PER_CPU_VAR(pcpu_hot + X86_top_of_stack), %rax
leaq -PTREGS_SIZE(%rax), %rsp
jmp .Ljump_to_C_code
SYM_CODE_END(start_cpu0)

but that would only be possible be after more cleanups which converge
cpu_bringup_and_idle() with start_secondary().

The attached patch seems to work fine.

The patch looks good to me.

It doesn't solve Paul's original issue where arch_cpu_idle_dead() needs
to be __noreturn. But that should probably be a separate patch anyway.

Okay, I'll split this off.


The __noreturn annotation seems to trigger an objtool warning, though, in
spite of the added BUG() at the end of xen_pv_play_dead():

arch/x86/xen/smp_pv.o: warning: objtool: xen_pv_play_dead() falls through to
next function xen_pv_cpu_die()

You'll need to tell objtool that xen_cpu_bringup_again() is noreturn by
adding "xen_cpu_bringup_again" to global_noreturns[] in
tools/objtool/check.c.

Ah, okay. Will do that.

(Yes it's a pain, I'll be working an improved solution to the noreturn
thing...)

Should be fairly easy, no?

"Just" extend the __noreturn macro to put the function into a ".text.noreturn"
section, which can be handled in a special way by objtool. This would need
an __init_noreturn macro, of course, for a ".init.text.noreturn" section.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature