From: Steven Rostedt <rostedt@goodmis.org>
To: Andi Kleen <andi@firstfloor.org>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
Andi Kleen <ak@linux.intel.com>
Subject: Re: [PATCH 1/7] trace: Move trace_seq_overflowed out of line
Date: Wed, 15 Mar 2017 23:20:30 -0400 [thread overview]
Message-ID: <20170315232030.19d9ffe2@gandalf.local.home> (raw)
In-Reply-To: <20170316022756.GF14380@two.firstfloor.org>
On Wed, 15 Mar 2017 19:27:57 -0700
Andi Kleen <andi@firstfloor.org> wrote:
> On Wed, Mar 15, 2017 at 08:54:20PM -0400, Steven Rostedt wrote:
> > On Tue, 14 Mar 2017 19:14:25 -0700
> > Andi Kleen <andi@firstfloor.org> wrote:
> >
> > > From: Andi Kleen <ak@linux.intel.com>
> > >
> > > Inlining trace_seq_overflowed takes ~17k in text size in my kernel.
> > > The function doesn't seem to be time critical, so we can just out of line it.
> >
> > Instead of out of lining trace_seq_has_overflowed(), have you tried to
> > out of line the function that's called by tracepoints (one per
> > tracepoint). That is, trace_handle_return()?
>
> This is a data driven approach so I always went for the largest savings.
>
> >
> > The trace_seq_handle_overflow() is used in not reproduced places that I
> > would like to keep it as an inline. If the issue is size of the kernel,
>
> I cannot parse this sentence. What advantage has it being inline?
Because you don't understand the problem. And why I'm against your
patch!
>
> > please just out of line the one place that calls it that is duplicated
> > for every tracepoint. Which happens to be trace_handle_return().
>
> It is used in lots of places outside trace_handle_return, so that would
> give far less savings.
Have you actually looked at what trace_seq_has_overflowed() is?
static inline bool trace_seq_has_overflowed(struct trace_seq *s)
{
return s->full || seq_buf_has_overflowed(&s->seq);
}
static inline bool
seq_buf_has_overflowed(struct seq_buf *s)
{
return s->len > s->size;
}
Basically trace_seq_has_overflowed() is the same as:
return s->full || s->seq->len > s->seq->size
You really think the above in 24 locations would cause 17k difference??
>
> -Andi
>
> include/linux/trace_events.h:143: return trace_seq_has_overflowed(s) ?
Every thing below is negligible. The above which is called in
trace_handle_return() is your problem.
Let me explain it to you.
The above is part of the TRACE_EVENT() logic. It is duplicated for
*every* tracepoint in the system.
Looking at a current kernel:
# ls /debug/tracing/events/*/*/enable | wc -l
1267
There's 1267 events. That means the function trace_handle_return() is
called 1267 times! THAT IS THE PROBLEM!!!!
Look at include/trace/trace_events.h for
trace_raw_output_##call()
That's the macro that creates over a thousand functions calling
trace_handle_return().
So please, fix where the issue is and not the other function, as 23
callers is not going to be noticed.
-- Steve
> include/linux/trace_seq.h:60: * trace_seq_has_overflowed - return true if the trace_seq took too much
> include/linux/trace_seq.h:66:static inline bool trace_seq_has_overflowed(struct trace_seq *s)
> kernel/trace/ring_buffer.c:47: return !trace_seq_has_overflowed(s);
> kernel/trace/ring_buffer.c:390: return !trace_seq_has_overflowed(s);
> kernel/trace/trace.c:3268: if (trace_seq_has_overflowed(s))
> kernel/trace/trace.c:3292: if (trace_seq_has_overflowed(s))
> kernel/trace/trace.c:3318: if (trace_seq_has_overflowed(s))
> kernel/trace/trace.c:3347: if (trace_seq_has_overflowed(s))
> kernel/trace/trace.c:3399: if (trace_seq_has_overflowed(&iter->seq))
> kernel/trace/trace.c:5490: if (trace_seq_has_overflowed(&iter->seq)) {
> kernel/trace/trace_functions_graph.c:910: if (trace_seq_has_overflowed(s))
> kernel/trace/trace_functions_graph.c:1221: if (trace_seq_has_overflowed(s))
> kernel/trace/trace_output.c:354: return !trace_seq_has_overflowed(s);
> kernel/trace/trace_output.c:374: return !trace_seq_has_overflowed(s);
> kernel/trace/trace_output.c:435: return !trace_seq_has_overflowed(s);
> kernel/trace/trace_output.c:522: return !trace_seq_has_overflowed(s);
> kernel/trace/trace_output.c:550: return !trace_seq_has_overflowed(s);
> kernel/trace/trace_output.c:586: return !trace_seq_has_overflowed(s);
> kernel/trace/trace_output.c:1021: if (trace_seq_has_overflowed(s))
> kernel/trace/trace_output.c:1071: if (ip == ULONG_MAX || trace_seq_has_overflowed(s))
> kernel/trace/trace_probe.c:44: return !trace_seq_has_overflowed(s); \
> kernel/trace/trace_probe.c:73: return !trace_seq_has_overflowed(s);
> kernel/trace/trace_syscalls.c:147: if (trace_seq_has_overflowed(s))
>
next prev parent reply other threads:[~2017-03-16 3:22 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-15 2:14 Some inline debloating, 4.11 edition Andi Kleen
2017-03-15 2:14 ` [PATCH 1/7] trace: Move trace_seq_overflowed out of line Andi Kleen
2017-03-16 0:54 ` Steven Rostedt
2017-03-16 2:27 ` Andi Kleen
2017-03-16 3:20 ` Steven Rostedt [this message]
2017-03-16 3:41 ` Steven Rostedt
2017-03-15 2:14 ` [PATCH 2/7] x86/atomic: Move __atomic_add_unless " Andi Kleen
2017-03-15 2:14 ` [PATCH 3/7] sched: Out of line __update_load_avg Andi Kleen
2017-03-15 2:14 ` [PATCH 4/7] kref: Remove WARN_ON for NULL release functions Andi Kleen
2017-03-15 2:46 ` Greg KH
2017-03-15 12:27 ` Peter Zijlstra
2017-03-15 2:14 ` [PATCH 5/7] Out of line dma_alloc/free_attrs Andi Kleen
2017-03-15 2:14 ` [PATCH 6/7] megasas: Remove expensive inline from megasas_return_cmd Andi Kleen
2017-03-15 12:17 ` Sumit Saxena
2017-03-15 2:14 ` [PATCH 7/7] Remove expensive WARN_ON in pagefault_disabled_dec Andi Kleen
2017-03-15 21:49 ` Andrew Morton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170315232030.19d9ffe2@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).