Re: [PATCH] powerpc/ftrace: Handle large kernel configs

["Naveen N. Rao" <naveen.n.rao@linux.ibm.com>]

Hi Christophe,
Sorry for the delay, catching up with some of the earlier emails now..


Christophe Leroy wrote:
> Hi Naveen,
> 
> Le 16/10/2018 à 22:25, Naveen N. Rao a écrit :
> ...
> 
>> +/*
>> + * If this is a compiler generated long_branch trampoline (essentially, a
>> + * trampoline that has a branch to _mcount()), we re-write the branch to
>> + * instead go to ftrace_[regs_]caller() and note down the location of this
>> + * trampoline.
>> + */
>> +static int setup_mcount_compiler_tramp(unsigned long tramp)
>> +{
>> +	int i, op;
>> +	unsigned long ptr;
>> +	static unsigned long ftrace_plt_tramps[NUM_FTRACE_TRAMPS];
>> +
>> +	/* Is this a known long jump tramp? */
>> +	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
>> +		if (!ftrace_tramps[i])
>> +			break;
>> +		else if (ftrace_tramps[i] == tramp)
>> +			return 0;
>> +
>> +	/* Is this a known plt tramp? */
>> +	for (i = 0; i < NUM_FTRACE_TRAMPS; i++)
>> +		if (!ftrace_plt_tramps[i])
>> +			break;
>> +		else if (ftrace_plt_tramps[i] == tramp)
>> +			return -1;
> 
> I don't understand how this is supposed to work.
> ftrace_plt_tramps[] being a static table, it is set to 0s at startup.
> So the above loop breaks at first round.
> 
> Then ftrace_plt_tramps[i] is never/nowhere set.
> 
> So I just see it as useless.
> 
> Am I missing something ?

No, that's correct. I had posted a cleanup of this a year back as part 
of the ftrace_direct enablement. I have updated that series and will be 
posting it out soon (though I should rebase it atop your livepatch 
series):
http://lkml.kernel.org/r/bdc3710137c4bda8393532a789558bed22507cfe.1606412433.git.naveen.n.rao@linux.vnet.ibm.com


- Naveen