Markers - remove extra format argument
Denys Vlasenko <vda.linux@googlemail.com> :
> Not in this patch, but I noticed:
>
> #define __trace_mark(name, call_private, format, args...) \
> do { \
> static const char __mstrtab_##name[] \
> __attribute__((section("__markers_strings"))) \
> = #name "\0" format; \
> static struct marker __mark_##name \
> __attribute__((section("__markers"), aligned(8))) = \
> { __mstrtab_##name, &__mstrtab_##name[sizeof(#name)], \
> 0, 0, marker_probe_cb, \
> { __mark_empty_function, NULL}, NULL }; \
> __mark_check_format(format, ## args); \
> if (unlikely(__mark_##name.state)) { \
> (*__mark_##name.call) \
> (&__mark_##name, call_private, \
> format, ## args); \
> } \
> } while (0)
>
> In this call:
>
> (*__mark_##name.call) \
> (&__mark_##name, call_private, \
> format, ## args); \
>
> you make gcc allocate duplicate format string. You can use
> &__mstrtab_##name[sizeof(#name)] instead since it holds the same string,
> or drop ", format," above and "const char *fmt" from here:
>
> void (*call)(const struct marker *mdata, /* Probe wrapper */
> void *call_private, const char *fmt, ...);
>
> since mdata->format is the same and all callees which need it can take it there.
Very good point. I actually thought about dropping it, since it would
remove an unnecessary argument from the stack. And actually, since I now
have the marker_probe_cb sitting between the marker site and the
callbacks, there is no API change required. Thanks :)
Mathieu
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: Denys Vlasenko <vda.linux@googlemail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
diff --git a/kernel/marker.c b/kernel/marker.c
index b5a9fe1..1abfb92 100644
--- a/kernel/marker.c
+++ b/kernel/marker.c
@@ -55,8 +55,8 @@
struct marker_entry {
struct hlist_node hlist;
char *format;
- void (*call)(const struct marker *mdata, /* Probe wrapper */
- void *call_private, const char *fmt, ...);
+ /* Probe wrapper */
+ void (*call)(const struct marker *mdata, void *call_private, ...);
struct marker_probe_closure single;
struct marker_probe_closure *multi;
int refcount; /* Number of times armed. 0 if disarmed. */
@@ -91,15 +91,13 @@
* marker_probe_cb Callback that prepares the variable argument list for probes.
* @mdata: pointer of type struct marker
* @call_private: caller site private data
- * @fmt: format string
* @...: Variable argument list.
*
* Since we do not use "typical" pointer based RCU in the 1 argument case, we
* need to put a full smp_rmb() in this branch. This is why we do not use
* rcu_dereference() for the pointer read.
*/
-void marker_probe_cb(const struct marker *mdata, void *call_private,
- const char *fmt, ...)
+void marker_probe_cb(const struct marker *mdata, void *call_private, ...)
{
va_list args;
char ptype;
@@ -120,8 +118,9 @@
/* Must read the ptr before private data. They are not data
* dependant, so we put an explicit smp_rmb() here. */
smp_rmb();
- va_start(args, fmt);
- func(mdata->single.probe_private, call_private, fmt, &args);
+ va_start(args, call_private);
+ func(mdata->single.probe_private, call_private, mdata->format,
+ &args);
va_end(args);
} else {
struct marker_probe_closure *multi;
@@ -136,9 +135,9 @@
smp_read_barrier_depends();
multi = mdata->multi;
for (i = 0; multi[i].func; i++) {
- va_start(args, fmt);
- multi[i].func(multi[i].probe_private, call_private, fmt,
- &args);
+ va_start(args, call_private);
+ multi[i].func(multi[i].probe_private, call_private,
+ mdata->format, &args);
va_end(args);
}
}
@@ -150,13 +149,11 @@
* marker_probe_cb Callback that does not prepare the variable argument list.
* @mdata: pointer of type struct marker
* @call_private: caller site private data
- * @fmt: format string
* @...: Variable argument list.
*
* Should be connected to markers "MARK_NOARGS".
*/
-void marker_probe_cb_noarg(const struct marker *mdata,
- void *call_private, const char *fmt, ...)
+void marker_probe_cb_noarg(const struct marker *mdata, void *call_private, ...)
{
va_list args; /* not initialized */
char ptype;
@@ -172,7 +169,8 @@
/* Must read the ptr before private data. They are not data
* dependant, so we put an explicit smp_rmb() here. */
smp_rmb();
- func(mdata->single.probe_private, call_private, fmt, &args);
+ func(mdata->single.probe_private, call_private, mdata->format,
+ &args);
} else {
struct marker_probe_closure *multi;
int i;
@@ -186,8 +184,8 @@
smp_read_barrier_depends();
multi = mdata->multi;
for (i = 0; multi[i].func; i++)
- multi[i].func(multi[i].probe_private, call_private, fmt,
- &args);
+ multi[i].func(multi[i].probe_private, call_private,
+ mdata->format, &args);
}
preempt_enable();
}
