lkml.org 
[lkml]   [2020]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH] objtool,ftrace: Implement UNWIND_HINT_RET_OFFSET
On Tue, Mar 31, 2020 at 01:16:52PM +0200, Peter Zijlstra wrote:
> Subject: objtool,ftrace: Implement UNWIND_HINT_RET_OFFSET
>
> This replaces the SAVE/RESTORE hints with a RET_OFFSET hint that applies
> to the following instructions:
>
> - any instruction that terminates a function, like: RETURN and sibling
> calls. It allows the stack-frame to be off by @sp_offset, ie. it
> allows stuffing the return stack.
>
> - EXCEPTION_RETURN (a new INSN_type that splits IRET out of
> CONTEXT_SWITCH) and here it denotes a @sp_offset sized POP and makes
> the instruction continue.

Looking closer, I see how my UNWIND_HINT_ADJUST idea doesn't work for
the ftrace_regs_caller() case. The ORC data is actually correct there.
So basically we need a way to tell objtool to be quiet.

I now understand what you're trying to do with the RET_TAIL thing, and I
guess it's ok for the ftrace case. But I'd rather an UNWIND_HINT_IGNORE
before the tail cail, which would tell objtool to just silence the tail
call warning. It's simpler for the user to understand, it's simpler
logic in objtool, and I think an "ignore warnings for the next insn"
hint would be more generally applicable anyway.

But also... the RET_OFFSET usage for sync_core() *really* bugs me.

I know you said it's like an indirect tail call with a bigger frame, but
that's kind of stretching it because the function frame is still there.

And objtool doesn't treat it like a tail call at all. In fact, it
handles it *completely* differently from the normal ret-tail-call case.
Instead of silencing a tail call warning, it adjusts the stack offset
and continues the code path.

This basically adds *two* new hint types, while trying to call them the
same thing. There's no overlapping functionality between them in
objtool, other than the use of the same insn->ret_offset variable. But
it's two distinct functionalities, depending on the context (return/tail
vs IRETQ).

I'll try to work up some patches with a different approach in a bit.

--
Josh

\
 
 \ /
  Last update: 2020-03-31 22:24    [W:0.148 / U:0.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site