On Thu, May 12, 2011 at 07:00:13PM +0530, Rabin Vincent wrote:

On Thu, May 12, 2011 at 14:54, Martin Schwidefsky
[off-list ref] wrote:

quoted

On Wed, 11 May 2011 22:53:55 +0530 Rabin Vincent [off-list ref] wrote:

quoted

On Tue, May 10, 2011 at 13:40, Martin Schwidefsky [off-list ref] wrote:
Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
relocation (R_ARM_ABS32) that gets used for the assembly file
that recordmcount.pl generates and assembles dictates that the lsb be
set if the target symbol is Thumb/Thumb-2 function.  mcount_adjust would
not help here since the ORing is done later, when the relocation is
applied.

Hmm, from what I can make out the C version of recordmcount uses R_ARM_ABS32
as well.

Right.  It worked when I initially implemented ARM support there because
recordmcount.c always found the STT_SECTION symbol as a base and not a
STT_FUNC symbol.  However, I noticed yesterday that this does not happen
in some cases, so I sent a patch to avoid STT_FUNC symbol as bases on
ARM, not because of this relocation, but because of a slightly different
oddity of Thumb symbols:

http://lkml.org/lkml/2011/5/11/304

(The relocation problem alone could be solved by using R_ARM_ABS32_NOI
 instead.)

quoted

quoted

Thumb-2 via recordmcount.c does not need the clearing of the lsb in
ftrace_call_adjust.

So the clearing of the lsb is only required if the recordmcount.pl script
is used?

Yes.

quoted

quoted

Building with the ARM instruction set also does not need the clearing
of the lsb.

Who does the ORing? I can't find anything in recordmount.pl/recordmcount.c
which looks like doing an OR, does the assembler do that based on the
symbol type?

The lsb is set to 1 by the linker, when it applies the relocations as it
links vmlinux.

quoted

quoted

quoted

Thumb-2 the offset is -1, correct? If there is a way to distinguish
the two targets in recordmcount at compile time we could convert arm
as well. Which would allow us to remove the ftrace_call_adjust function.

To remove ftrace_call_adjust, we could either deprecate the
recordmcount.pl usage for ARM (you already have to edit the Kconfig to
use it) or modify it to generate specific relocations explicitly instead
of using the assembler data directives.

Hmm, it would be a desirable property if the C version and the pearl
version of recordmcount would do the same. Or we could remove the arm
support from the pearl script, the C version is faster anyway.

I'm OK with removing the ARM support from recordmcount.pl; it doesn't
seem needed to make significant modifications to it for ARM when we
don't use it anyway.

Is there any reason why the recordmcount.pl would ever be used now that the
C implementation exists?

I notice that arch/arm/Kconfig has:

config ARM
...
        select HAVE_C_RECORDMCOUNT

so deprecating ARM support from recordmcount.pl seems unlikely to hurt
anyone.

The C implementation seems to have worked fine when I was testing dynamic
ftrace with Thumb-2 recently.

Cheers
---Dave