Re: [PATCH 1/3] tracing: Merge irqflags + preempt counter.

[Steven Rostedt]

On Mon, 25 Jan 2021 15:05:51 +0100
Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote:

> > Then all you need to do is implement the tracing_gen_ctx_irq_test() without
> > adding and #ifdefs in it, and just or in the "irq_status" to trace_flags,
> > without any conditionals.  
> 
> You just moved that from one place to another. I had to move enum
> trace_flag_type just before tracing_gen_ctx_irq_test() so it is
> available by tracing_gen_ctx_flags(). I don't know if you earn by
> inlining much, the gcc numbers for vmlinux:
> 
>     text      data      bss      dec filename
>  24306853 21869070 14205180 60381103 vmlinux-rc5
>  24306482 21869070 14205180 60380732 vmlinux-rc5-merge-irqflags
>  24306852 21869070 14205180 60381102 vmlinux-rc5-merge-irqflags-inline
> 
> I know that a reduction by ~300 byte isn't exactly breath taking. I run
> the test twice because the reduction in text by one byte looked odd but
> it is what it is. Inlining only tracing_gen_ctx_dec() probably makes
> sense especially since there is one user.

I was thinking about the inlining for two reasons. One was to consolidate
the logic in the header file, as they are small functions. And two, inlined
functions tend to be faster than non-inlined functions. Thus, I wasn't
looking at this from a size point of view, but since this is called by all
events, including function tracer, being efficient is a requirement.

> 
> I can post the irqflags-merge and the inlinining as two seprate patches
> and you can then decide if you merge the two patches or drop the
> inlining.

Feel free to send it as separate patches. I'd like to have the inlining.

Thanks!

-- Steve