lkml.org 
[lkml]   [2022]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: objtool warning for next-20221118
    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.

    > 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.

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

    --
    Josh

    \
     
     \ /
      Last update: 2022-11-24 17:40    [W:3.107 / U:0.136 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site