[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: objtool warning for next-20221118



On Tue, Nov 29, 2022 at 11:56:55AM -0800, Paul E. McKenney wrote:
> On Fri, Nov 25, 2022 at 06:30:35AM +0100, Juergen Gross wrote:
> > 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.
> 
> And in last night's -next run, that diagnostic was gone!
> 
> But of course another appeared in its place:
> 
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool: 
> check_relocations+0xd1: stack state mismatch: cfa1=4+32 cfa2=-1+0
> 
> The .config file is shown below.  Thoughts?

And it is back.  Which makes no sense, but there it is.

                                                        Thanx, Paul



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.