* Josh Poimboeuf <jpoim...@redhat.com> wrote:
> On Mon, Jul 20, 2015 at 09:56:11AM +0200, Ingo Molnar wrote:
> > >
> > > The reason I suggested to put FRAME in the macro name is to try to
> > > prevent it
> > > from being accidentally used for leaf functions, where it isn't needed.
> >
> > Well, we could use LEAF_FUNCTION to mark that fact.
> >
> > Wether a function written in assembly is a leaf function or not is a higher
> > level
> > (and thus more valuable) piece of information whether we generate frame
> > pointer
> > debuginfo or not.
> >
> > > Also the naming of FUNCTION_ENTRY and FUNCTION_RETURN doesn't do anything
> > > to
> > > distinguish them from the already ubiquitous ENTRY and ENDPROC. So as a
> > > kernel
> > > developer it seems confusing to me, e.g. how do I remember when to use
> > > FUNCTION_ENTRY vs ENTRY?
> >
> > 'ENDPROC' is really leftover from older debuginfo cruft, it's not a
> > valuable
> > construct IMHO, even if it's (sadly) ubiquitious.
> >
> > We want to create new, clean, as minimal as possible and as clearly named
> > as
> > possible debuginfo constructs from first principles.
>
> Ok. So if I understand right, the proposal is:
>
> Replace *all* x86 usage of ENTRY/ENDPROC with either:
>
> FUNCTION_ENTRY(func)
> FUNCTION_RETURN(func)
>
> or
>
> LEAF_FUNCTION_ENTRY(func)
> LEAF_FUNCTION_RETURN(func)
>
> Those sound fine to me.
Yeah - but keep the old constructs as well and don't necessarily do the full
migration straight away, only once the dust has settled - to reduce churn.
> I should point out that there are still a few cases where the more granular
> FRAME/ENDFRAME and ENTRY/ENDPROC macros would still be needed.
>
> For example, if the function ends with a jump instead of a ret. If the
> jump is a sibling call, the code would look like:
>
> FUNCTION_ENTRY(func)
> ...
> ENDFRAME
> jmp another_func
> ENDPROC(func)
>
>
> Or if it's a jump within the function to an internal ret:
>
> FUNCTION_ENTRY(func)
> ...
> 1: ...
> ENDFRAME
> ret
> 2: ...
> jmp 1b
> ENDPROC(func)
>
>
> Or if it jumps to some shared code before returning:
>
> FUNCTION_ENTRY(func_1)
> ...
> jmp common_return
> ENDPROC(func_1)
>
> FUNCTION_ENTRY(func_2)
> ...
> jmp common_return
> ENDPROC(func_2)
>
> common_return:
> ...
> ENDFRAME
> ret
>
>
> So in some cases we'd still need the more granular macros, unless we
> decided to make special macros for these cases as well.
Ok, I see how the naming scheme I proposed won't work with all that very well,
but
I'd still suggest using consistently named patterns.
Let me suggest yet another approach. How about open-coding something like this:
FUNCTION_START(func)
push_bp
mov_sp_bp
...
pop_bp
ret
FUNCTION_END(func)
This is just two easy things:
- a redefine of the FUNCTION_ENTRY and ENDPROC names
- the introduction of three quasi-mnemonics: push_bp, mov_sp_bp, pop_bp -
which
all look very similar to a real frame setup sequence, except that we can
easily
make them go away in the !CONFIG_FRAME_POINTERS case.
The advantage of this approach would be:
- it looks pretty 'natural' and very close to how the real disassembly looks
like in CONFIG_FRAME_POINTERS=y kernels. So while it's not as compact as
some
of the other variants, it's close to what the real instruction sequence
looks
like and that is a positive quality in itself.
- it also makes it apparent 'on sight' that it's probably a bug to have
unbalanced push/pop sequences in a regular function, to any reasonably alert
assembly coder.
- if we ever unsupport framepointer kernels in the (far far) future, we can get
rid of all lines with those 3 mnemonics and be done with it.
- it's finegrained enough so that we can express all the special function/tail
variants you listed above.
What do you think?
I'd still keep existing frame setup functionality and names and only use these
in
fixes, new code and new annotations - and do a full rename and cleanup once the
dust has settled.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
