Messages in this thread | | | Date | Fri, 25 Nov 2022 06:30:35 +0100 | Subject | Re: objtool warning for next-20221118 | From | Juergen Gross <> |
| |
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 [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |