Re: [PATCH 2/2] powerpc, ftrace: use create_branch lib function

From: Michael Ellerman
Date: Sat Feb 14 2009 - 08:49:43 EST


On Fri, 2009-02-13 at 10:00 -0500, Steven Rostedt wrote:
> plain text document attachment
> (0002-powerpc-ftrace-use-create_branch-lib-function.patch)
> From: Steven Rostedt <srostedt@xxxxxxxxxx>
>
> Impact: clean up, remove duplicate code
>
> When ftrace was first ported to PowerPC, there existed a
> create_function_call that would create the instruction to make a call
> to a given address. Unfortunately, this call expected to write to
> the address it was given, and since it used the address to calculate
> the offset, it could not be faked.
>
> ftrace needed a way to create the instruction without actually writing
> that instruction to the text section. So ftrace had to implement its
> own code.
>
> Now we have create_branch in the code patching library, which does
> exactly what ftrace needs. This patch replaces ftrace's implementation
> with the library function.

Thanks for doing this, I was going to once the ftrace code had settled a
little but you beat me to it.

> @@ -46,17 +41,10 @@ ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> {
> unsigned int op;
>
> - /*
> - * It would be nice to just use create_function_call, but that will
> - * update the code itself. Here we need to just return the
> - * instruction that is going to be modified, without modifying the
> - * code.
> - */
> addr = GET_ADDR(addr);
>
> /* if (link) set op to 'bl' else 'b' */
> - op = 0x48000000 | (link ? 1 : 0);
> - op |= (ftrace_calc_offset(ip, addr) & 0x03fffffc);
> + op = create_branch((unsigned int *)ip, addr, link ? 1 : 0);

If I was feeling nit-picky I'd say you should use:

op = create_branch((unsigned int *)ip, addr, link ? BRANCH_SET_LINK : 0);


But admittedly we're unlikely to ever change the flag handling, so it's
probably not worth the effort of a respin (or this email :).

cheers

--
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

Attachment: signature.asc
Description: This is a digitally signed message part