[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221124163928.dof5ldfer6rswlnx@treble>
Date: Thu, 24 Nov 2022 08:39:28 -0800
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Juergen Gross <jgross@...e.com>
Cc: Andrew Cooper <Andrew.Cooper3@...rix.com>,
Peter Zijlstra <peterz@...radead.org>,
"Paul E. McKenney" <paulmck@...nel.org>,
"sfr@...b.auug.org.au" <sfr@...b.auug.org.au>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"sstabellini@...nel.org" <sstabellini@...nel.org>,
"boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
Thomas Gleixner <tglx@...utronix.de>
Subject: Re: 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
Powered by blists - more mailing lists