From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@redhat.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>,
linux-kernel@vger.kernel.org
Subject: [PATCH] tracing: eliminate const char[] auto variables
Date: Fri, 2 Nov 2018 21:54:42 +0100 [thread overview]
Message-ID: <20181102205442.7758-1-linux@rasmusvillemoes.dk> (raw)
Automatic const char[] variables cause unnecessary code
generation. For example, the this_mod variable leads to
3f04: 48 b8 5f 5f 74 68 69 73 5f 6d movabs $0x6d5f736968745f5f,%rax # __this_m
3f0e: 4c 8d 44 24 02 lea 0x2(%rsp),%r8
3f13: 48 8d 7c 24 10 lea 0x10(%rsp),%rdi
3f18: 48 89 44 24 02 mov %rax,0x2(%rsp)
3f1d: 4c 89 e9 mov %r13,%rcx
3f20: b8 65 00 00 00 mov $0x65,%eax # e
3f25: 48 c7 c2 00 00 00 00 mov $0x0,%rdx
3f28: R_X86_64_32S .rodata.str1.1+0x18d
3f2c: be 48 00 00 00 mov $0x48,%esi
3f31: c7 44 24 0a 6f 64 75 6c movl $0x6c75646f,0xa(%rsp) # odul
3f39: 66 89 44 24 0e mov %ax,0xe(%rsp)
i.e., the string gets built on the stack at runtime. Similar code can be
found for the other instances I'm replacing here. Putting the string
in .rodata reduces the combined .text+.rodata size and saves time and
stack space at runtime.
The simplest fix, and what I've done for the this_mod case, is to just
make the variable static.
However, for the "<faulted>" case where the same string is used twice,
that prevents the linker from merging those two literals, so instead use
a macro - that's also slightly cleaner, since FAULTED_SIZE otherwise has
no direct relation to the faulted variable used in
tracing_mark_raw_write().
Finally, for the two runs of spaces, just use variables initialized with
string literals; the linker (at least for x86) will reuse the tail of
the longer for the shorter string.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
kernel/trace/ftrace.c | 2 +-
kernel/trace/trace.c | 13 ++++++-------
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index f536f601bd46..4b79d4ae9635 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3862,7 +3862,7 @@ static int ftrace_hash_move_and_update_ops(struct ftrace_ops *ops,
static bool module_exists(const char *module)
{
/* All modules have the symbol __this_module */
- const char this_mod[] = "__this_module";
+ static const char this_mod[] = "__this_module";
char modname[MAX_PARAM_PREFIX_LEN + sizeof(this_mod) + 2];
unsigned long val;
int n;
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ff1c4b20cd0a..ffcceb33a9b2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3381,8 +3381,8 @@ static void print_func_help_header_irq(struct trace_buffer *buf, struct seq_file
unsigned int flags)
{
bool tgid = flags & TRACE_ITER_RECORD_TGID;
- const char tgid_space[] = " ";
- const char space[] = " ";
+ const char *tgid_space = " ";
+ const char *space = " ";
seq_printf(m, "# %s _-----=> irqs-off\n",
tgid ? tgid_space : space);
@@ -6089,13 +6089,13 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
struct ring_buffer *buffer;
struct print_entry *entry;
unsigned long irq_flags;
- const char faulted[] = "<faulted>";
ssize_t written;
int size;
int len;
/* Used in tracing_mark_raw_write() as well */
-#define FAULTED_SIZE (sizeof(faulted) - 1) /* '\0' is already accounted for */
+#define FAULTED_STR "<faulted>"
+#define FAULTED_SIZE (sizeof(FAULTED_STR) - 1) /* '\0' is already accounted for */
if (tracing_disabled)
return -EINVAL;
@@ -6127,7 +6127,7 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
len = __copy_from_user_inatomic(&entry->buf, ubuf, cnt);
if (len) {
- memcpy(&entry->buf, faulted, FAULTED_SIZE);
+ memcpy(&entry->buf, FAULTED_STR, FAULTED_SIZE);
cnt = FAULTED_SIZE;
written = -EFAULT;
} else
@@ -6168,7 +6168,6 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
struct ring_buffer_event *event;
struct ring_buffer *buffer;
struct raw_data_entry *entry;
- const char faulted[] = "<faulted>";
unsigned long irq_flags;
ssize_t written;
int size;
@@ -6208,7 +6207,7 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
len = __copy_from_user_inatomic(&entry->id, ubuf, cnt);
if (len) {
entry->id = -1;
- memcpy(&entry->buf, faulted, FAULTED_SIZE);
+ memcpy(&entry->buf, FAULTED_STR, FAULTED_SIZE);
written = -EFAULT;
} else
written = cnt;
--
2.19.1.6.gbde171bbf5
next reply other threads:[~2018-11-02 20:54 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-02 20:54 Rasmus Villemoes [this message]
2019-03-18 21:34 ` [PATCH v2/resend] tracing: eliminate const char[] auto variables Rasmus Villemoes
2019-03-18 21:53 ` Al Viro
2019-03-19 16:39 ` Steven Rostedt
2019-03-20 8:17 ` [PATCH v3] " Rasmus Villemoes
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=20181102205442.7758-1-linux@rasmusvillemoes.dk \
--to=linux@rasmusvillemoes.dk \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=rostedt@goodmis.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).