Re: [PATCH] tracing: uninline trace_trigger_soft_disabled()

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 10/02/2022 à 15:26, Steven Rostedt a écrit :
> On Thu, 10 Feb 2022 09:47:52 +0100
> Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> 
>> On a ppc32 build with CONFIG_CC_OPTIMISE_FOR_SIZE,
>> trace_trigger_soft_disabled() appears more than 50 times in vmlinux.
>>
>> That function is rather big for an inlined function, and
>> it doesn't benefit much from inlining as its only parameter
>> is a pointer to a struct in memory:
> 
> The number of parameters is not the reason for it being inlined. It's in a
> *very* hot path, and a function call causes a noticeable performance hit.

Fair enough

> 
> 
>>
>>
>> It doesn't benefit much from inlining as its only parameter is a
>> pointer to a struct in memory so no constant folding is involved.
>>
>> Uninline it and move it into kernel/trace/trace_events_trigger.c
>>
>> It reduces the size of vmlinux by approximately 10 kbytes.
> 
> If you have an issue with the size, perhaps the function can be modified to
> condense it. I'm happy to have a size reduction, but I will NACK making it
> into a function call.
> 

Well, my first issue is that I get it duplicated 50 times because GCC 
find it too big to get inlined. So it is a function call anyway.

For instance, when building arch/powerpc/kernel/irq.o which -Winline, I get:

./include/linux/perf_event.h:1169:20: error: inlining failed in call to 
'perf_fetch_caller_regs': call is unlikely and code size would grow 
[-Werror=inline]
./include/linux/perf_event.h:1169:20: error: inlining failed in call to 
'perf_fetch_caller_regs': call is unlikely and code size would grow 
[-Werror=inline]
./include/linux/perf_event.h:1169:20: error: inlining failed in call to 
'perf_fetch_caller_regs': call is unlikely and code size would grow 
[-Werror=inline]
./include/linux/perf_event.h:1169:20: error: inlining failed in call to 
'perf_fetch_caller_regs': call is unlikely and code size would grow 
[-Werror=inline]
./include/linux/trace_events.h:712:1: error: inlining failed in call to 
'trace_trigger_soft_disabled': call is unlikely and code size would grow 
[-Werror=inline]
./include/linux/trace_events.h:712:1: error: inlining failed in call to 
'trace_trigger_soft_disabled': call is unlikely and code size would grow 
[-Werror=inline]
./include/linux/trace_events.h:712:1: error: inlining failed in call to 
'trace_trigger_soft_disabled': call is unlikely and code size would grow 
[-Werror=inline]
./include/linux/trace_events.h:712:1: error: inlining failed in call to 
'trace_trigger_soft_disabled': call is unlikely and code size would grow 
[-Werror=inline]



If having it a function call is really an issue, then it should be 
__always_inline

I will see the impact on size when we do so.


What is in the hot path really ? It is the entire function or only the 
first test ?

What about:

static inline bool
trace_trigger_soft_disabled(struct trace_event_file *file)
{
	unsigned long eflags = file->flags;

	if (!(eflags & EVENT_FILE_FL_TRIGGER_COND))
		return outlined_trace_trigger_soft_disabled(...);
	return false;
}


Or is there more in the hot path ?

Thanks
Christophe