| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 4 Jul 2008 09:57:02 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To: Masami Hiramatsu <mhiramat@...hat.com>
Cc: akpm@...ux-foundation.org, Ingo Molnar <mingo@...e.hu>,
linux-kernel@...r.kernel.org,
Peter Zijlstra <peterz@...radead.org>,
"Frank Ch. Eigler" <fche@...hat.com>,
Hideo AOKI <haoki@...hat.com>,
Takashi Nishiie <t-nishiie@...css.fujitsu.com>,
Steven Rostedt <rostedt@...dmis.org>,
Alexander Viro <viro@...iv.linux.org.uk>
Subject: Re: [RFC patch 1/3] Kernel Tracepoints
* Masami Hiramatsu (mhiramat@...hat.com) wrote:
> Hi Mathieu,
>
> I couldn't apply this patch against 2.6.26-rc8, and have
> some trivial comments.
>
> Mathieu Desnoyers wrote:
> > Implementation of kernel tracepoints. Inspired from the Linux Kernel Markers.
> >
> > Allows complete typing verification. No format string required.
> >
> > TODO : Documentation/tracepoint.txt
> >
> > Changelog :
> > - Use #name ":" #proto as string to identify the tracepoint in the
> > tracepoint table. This will make sure not type mismatch happens due to
> > connexion of a probe with the wrong type to a tracepoint declared with
> > the same name in a different header.
> >
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
> > CC: 'Peter Zijlstra' <peterz@...radead.org>
> > CC: "Frank Ch. Eigler" <fche@...hat.com>
> > CC: 'Ingo Molnar' <mingo@...e.hu>
> > CC: 'Hideo AOKI' <haoki@...hat.com>
> > CC: Takashi Nishiie <t-nishiie@...css.fujitsu.com>
> > CC: 'Steven Rostedt' <rostedt@...dmis.org>
> > CC: Alexander Viro <viro@...iv.linux.org.uk>
> > ---
> > include/asm-generic/vmlinux.lds.h | 6
> > include/linux/module.h | 17 +
> > include/linux/tracepoint.h | 139 ++++++++++
> > init/Kconfig | 16 +
> > kernel/Makefile | 1
> > kernel/module.c | 66 ++++-
> > kernel/tracepoint.c | 498 ++++++++++++++++++++++++++++++++++++++
> > 7 files changed, 741 insertions(+), 2 deletions(-)
> >
> > Index: linux-2.6-lttng/init/Kconfig
> > ===================================================================
> > --- linux-2.6-lttng.orig/init/Kconfig 2008-07-03 11:47:15.000000000 -0400
> > +++ linux-2.6-lttng/init/Kconfig 2008-07-03 11:49:54.000000000 -0400
> > @@ -782,12 +782,28 @@ config PROFILING
> > Say Y here to enable the extended profiling support mechanisms used
> > by profilers such as OProfile.
> >
> > +config TRACEPOINTS
> > + bool "Activate tracepoints"
> > + default y
> > + help
> > + Place an empty function call at each tracepoint site. Can be
> > + dynamically changed for a probe function.
> > +
> > config MARKERS
> > bool "Activate markers"
> > help
> > Place an empty function call at each marker site. Can be
> > dynamically changed for a probe function.
> >
> > +config TRACEPROBES
> > + tristate "Compile generic tracing probes"
> > + depends on MARKERS
> > + default y
> > + help
> > + Compile generic tracing probes, which connect to the tracepoints when
> > + loaded and format the information collected by the tracepoints with
> > + the Markers.
> > +
> > source "arch/Kconfig"
>
> might this part better go to PATCH 3/3?
>
Will fix.
> [...]
> > +
> > +
> > +#define TPPROTO(args...) args
> > +#define TPARGS(args...) args
> > +
> > +#ifdef CONFIG_TRACEPOINTS
> > +
> > +/*
> > + * Note : the empty asm volatile with read constraint is used here instead of a
> > + * "used" attribute to fix a gcc 4.1.x bug.
>
> There seems no empty asm...
>
True, should be removed from markers too.
> > + * Make sure the alignment of the structure in the __tracepoints section will
> > + * not add unwanted padding between the beginning of the section and the
> > + * structure. Force alignment to the same alignment as the section start.
> > + */
> > +#define DEFINE_TRACE(name, proto, args) \
> > + static inline void _do_trace_##name(struct tracepoint *tp, proto) \
> > + { \
> > + int i; \
> > + struct tracepoint_probe_closure *multi; \
> > + preempt_disable(); \
> > + multi = tp->multi; \
> > + smp_read_barrier_depends(); \
> > + if (multi) { \
> > + for (i = 0; multi[i].func; i++) { \
> > + ((void(*)(void *private_data, proto)) \
> > + (multi[i].func))(multi[i].probe_private, args);\
> > + } \
> > + } \
> > + preempt_enable(); \
> > + } \
> > + static inline void trace_##name(proto) \
> > + { \
> > + static const char __tpstrtab_##name[] \
> > + __attribute__((section("__tracepoints_strings"))) \
> > + = #name ":" #proto; \
> > + static struct tracepoint __tracepoint_##name \
> > + __attribute__((section("__tracepoints"), aligned(8))) = \
> > + { __tpstrtab_##name, 0, NULL }; \
> > + if (unlikely(__tracepoint_##name.state)) \
> > + _do_trace_##name(&__tracepoint_##name, args); \
> > + } \
> > + static inline int register_trace_##name( \
> > + void (*probe)(void *private_data, proto), \
> > + void *private_data) \
> > + { \
> > + return tracepoint_probe_register(#name ":" #proto, \
> > + (void *)probe, private_data); \
> > + } \
> > + static inline void unregister_trace_##name( \
> > + void (*probe)(void *private_data, proto), \
> > + void *private_data) \
> > + { \
> > + tracepoint_probe_unregister(#name ":" #proto, \
> > + (void *)probe, private_data); \
> > + }
> > +
>
> As we are discussing on another thread, this macro can't handle
> non-argument tracepoint.
>
Will fix.
> [...]
> > Index: linux-2.6-lttng/kernel/tracepoint.c
> > ===================================================================
> > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6-lttng/kernel/tracepoint.c 2008-07-03 11:54:30.000000000 -0400
> > @@ -0,0 +1,498 @@
> > +/*
> > + * Copyright (C) 2007 Mathieu Desnoyers
>
> trivial: it should be 2008? :-)
>
I always have difficulty remembering the day of the week. Now years... :)
> [...]
> > Index: linux-2.6-lttng/kernel/module.c
> > ===================================================================
> > --- linux-2.6-lttng.orig/kernel/module.c 2008-07-03 11:49:54.000000000 -0400
> > +++ linux-2.6-lttng/kernel/module.c 2008-07-03 11:54:01.000000000 -0400
> > @@ -46,6 +46,7 @@
> > #include <asm/cacheflush.h>
> > #include <linux/license.h>
> > #include <asm/sections.h>
> > +#include <linux/tracepoint.h>
> >
> > #if 0
> > #define DEBUGP printk
> > @@ -1770,6 +1771,8 @@ static struct module *load_module(void _
> > unsigned int unusedgplcrcindex;
> > unsigned int markersindex;
> > unsigned int markersstringsindex;
> > + unsigned int tracepointsindex;
> > + unsigned int tracepointsstringsindex;
> > struct module *mod;
> > long err = 0;
> > void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
> > @@ -2049,6 +2052,9 @@ static struct module *load_module(void _
> > markersindex = find_sec(hdr, sechdrs, secstrings, "__markers");
> > markersstringsindex = find_sec(hdr, sechdrs, secstrings,
> > "__markers_strings");
> > + tracepointsindex = find_sec(hdr, sechdrs, secstrings, "__tracepoints");
> > + tracepointsstringsindex = find_sec(hdr, sechdrs, secstrings,
> > + "__tracepoints_strings");
> >
> > /* Now do relocations. */
> > for (i = 1; i < hdr->e_shnum; i++) {
> > @@ -2076,6 +2082,12 @@ static struct module *load_module(void _
> > mod->num_markers =
> > sechdrs[markersindex].sh_size / sizeof(*mod->markers);
> > #endif
> > +#ifdef CONFIG_TRACEPOINTS
> > + mod->tracepoints = (void *)sechdrs[tracepointsindex].sh_addr;
> > + mod->num_tracepoints =
> > + sechdrs[tracepointsindex].sh_size / sizeof(*mod->tracepoints);
> > +#endif
> > +
> >
> > /* Find duplicate symbols */
> > err = verify_export_symbols(mod);
> > @@ -2094,11 +2106,16 @@ static struct module *load_module(void _
> >
> > add_kallsyms(mod, sechdrs, symindex, strindex, secstrings);
> >
> > + if (!(mod->taints & TAINT_FORCED_MODULE)) {
> > #ifdef CONFIG_MARKERS
> > - if (!(mod->taints & TAINT_FORCED_MODULE))
> > marker_update_probe_range(mod->markers,
> > mod->markers + mod->num_markers);
>
> Here, this hunk was rejected, because "Markers Support for
> Proprierary Modules" patch doesn't merged yet.
>
Ok, I'll change the patch order.
> > #endif
> > +#ifdef CONFIG_TRACEPOINTS
> > + tracepoint_update_probe_range(mod->tracepoints,
> > + mod->tracepoints + mod->num_tracepoints);
> > +#endif
> > + }
> > err = module_finalize(hdr, sechdrs, mod);
> > if (err < 0)
> > goto cleanup;
>
> Thank you,
>
Thanks for the review,
Mathieu
> --
> Masami Hiramatsu
>
> Software Engineer
> Hitachi Computer Products (America) Inc.
> Software Solutions Division
>
> e-mail: mhiramat@...hat.com
>
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists