linux-kernel - Re: [PATCH] tracing: create events only when configed

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090525231845.GE7879@nowhere>
Date:	Tue, 26 May 2009 01:18:47 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Lai Jiangshan <laijs@...fujitsu.com>
Cc:	Ingo Molnar <mingo@...e.hu>, Steven Rostedt <rostedt@...dmis.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tracing: create events only when configed

On Mon, May 25, 2009 at 09:03:36AM +0800, Lai Jiangshan wrote:
> Frederic Weisbecker wrote:
> > On Fri, May 22, 2009 at 05:40:23PM +0800, Lai Jiangshan wrote:
> >> There some unneeded events in debugfs/tracing/ftrace/ when the
> >> corresponding CONFIG_XXXX=n.
> >>
> >> And when CONFIG_ENABLE_EVENT_TRACING=n, various events
> >> are still created.
> >>
> >> Now when CONFIG_ENABLE_EVENT_TRACING=n, various events are
> >> not created, trace_export.c is compiled only for creating
> >> TRACEPOINTs.
> 
> I'm sorry for this incorrect changelog.
> The TRACEPOINTs are not defined in trace_export.c.


Thanks Lai.
The patch is fine and fixes a real waste of memory.

Well, the problem is that it fixes two wastes of memory:

- the creation of tracer events when the corresponding tracers are
  not configured

- the creation of events when CONFIG_ENABLE_EVENT_TRACING is not set
  but CONFIG_EVENT_TRACING=y


It would be nice if you could split it up in two patches.

Also you seem to have missed some tracers in the CPP checks,
such as function tracer, sched switch tracer, ...

Thanks.

Frederic.

 
> >>
> >> [Impact:] cleanup
> >>
> >> Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com>
> > 
> > 
> > Looks good.
> > 
> > Acked-by: Frederic Weisbecker <fweisbec@...il.com>
> > 
> > 
> 
> Thanx, changelog is rewrote
> 
> Frome: Lai Jiangshan <laijs@...fujitsu.com>
> 
> There some unneeded events in debugfs/tracing/ftrace/ when the
> corresponding CONFIG_XXXX=n.
> 
> And when CONFIG_ENABLE_EVENT_TRACING=n, various events
> are still created.
> 
> Now when CONFIG_ENABLE_EVENT_TRACING=n, various events are
> not created. And TRACEPOINTs are created.
> 
> [Impact:] cleanup
> 
> Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com>
> ---
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index f7a7ae1..e4e0e7a 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -52,7 +52,7 @@
>  
>  #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
>  
> -#ifdef CONFIG_EVENT_TRACING
> +#ifdef CONFIG_ENABLE_EVENT_TRACING
>  #include <trace/ftrace.h>
>  #endif
>  
> diff --git a/kernel/trace/trace_event_types.h b/kernel/trace/trace_event_types.h
> index 5e32e37..5c9813e 100644
> --- a/kernel/trace/trace_event_types.h
> +++ b/kernel/trace/trace_event_types.h
> @@ -13,6 +13,7 @@ TRACE_EVENT_FORMAT(function, TRACE_FN, ftrace_entry, ignore,
>  	TP_RAW_FMT(" %lx <-- %lx")
>  );
>  
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  TRACE_EVENT_FORMAT(funcgraph_entry, TRACE_GRAPH_ENT,
>  		   ftrace_graph_ent_entry, ignore,
>  	TRACE_STRUCT(
> @@ -30,6 +31,7 @@ TRACE_EVENT_FORMAT(funcgraph_exit, TRACE_GRAPH_RET,
>  	),
>  	TP_RAW_FMT("<-- %lx (%d)")
>  );
> +#endif
>  
>  TRACE_EVENT_FORMAT(wakeup, TRACE_WAKE, ctx_switch_entry, ignore,
>  	TRACE_STRUCT(
> @@ -66,6 +68,7 @@ TRACE_EVENT_FORMAT_NOFILTER(special, TRACE_SPECIAL, special_entry, ignore,
>  	TP_RAW_FMT("(%08lx) (%08lx) (%08lx)")
>  );
>  
> +#ifdef CONFIG_STACKTRACE
>  /*
>   * Stack-trace entry:
>   */
> @@ -101,6 +104,7 @@ TRACE_EVENT_FORMAT(user_stack, TRACE_USER_STACK, userstack_entry, ignore,
>  	TP_RAW_FMT("\t=> (%08lx)\n\t=> (%08lx)\n\t=> (%08lx)\n\t=> (%08lx)\n"
>  		 "\t=> (%08lx)\n\t=> (%08lx)\n\t=> (%08lx)\n\t=> (%08lx)\n")
>  );
> +#endif
>  
>  TRACE_EVENT_FORMAT(bprint, TRACE_BPRINT, bprint_entry, ignore,
>  	TRACE_STRUCT(
> @@ -119,6 +123,7 @@ TRACE_EVENT_FORMAT(print, TRACE_PRINT, print_entry, ignore,
>  	TP_RAW_FMT("%08lx (%d) fmt:%p %s")
>  );
>  
> +#ifdef CONFIG_BRANCH_TRACER
>  TRACE_EVENT_FORMAT(branch, TRACE_BRANCH, trace_branch, ignore,
>  	TRACE_STRUCT(
>  		TRACE_FIELD(unsigned int, line, line)
> @@ -130,7 +135,9 @@ TRACE_EVENT_FORMAT(branch, TRACE_BRANCH, trace_branch, ignore,
>  	),
>  	TP_RAW_FMT("%u:%s:%s (%u)")
>  );
> +#endif
>  
> +#ifdef CONFIG_HW_BRANCH_TRACER
>  TRACE_EVENT_FORMAT(hw_branch, TRACE_HW_BRANCHES, hw_branch_entry, ignore,
>  	TRACE_STRUCT(
>  		TRACE_FIELD(u64, from, from)
> @@ -138,7 +145,9 @@ TRACE_EVENT_FORMAT(hw_branch, TRACE_HW_BRANCHES, hw_branch_entry, ignore,
>  	),
>  	TP_RAW_FMT("from: %llx to: %llx")
>  );
> +#endif
>  
> +#ifdef CONFIG_POWER_TRACER
>  TRACE_EVENT_FORMAT(power, TRACE_POWER, trace_power, ignore,
>  	TRACE_STRUCT(
>  		TRACE_FIELD_SIGN(ktime_t, state_data.stamp, stamp, 1)
> @@ -148,7 +157,9 @@ TRACE_EVENT_FORMAT(power, TRACE_POWER, trace_power, ignore,
>  	),
>  	TP_RAW_FMT("%llx->%llx type:%u state:%u")
>  );
> +#endif
>  
> +#ifdef CONFIG_KMEMTRACE
>  TRACE_EVENT_FORMAT(kmem_alloc, TRACE_KMEM_ALLOC, kmemtrace_alloc_entry, ignore,
>  	TRACE_STRUCT(
>  		TRACE_FIELD(enum kmemtrace_type_id, type_id, type_id)
> @@ -171,5 +182,6 @@ TRACE_EVENT_FORMAT(kmem_free, TRACE_KMEM_FREE, kmemtrace_free_entry, ignore,
>  	),
>  	TP_RAW_FMT("type:%u call_site:%lx ptr:%p")
>  );
> +#endif
>  
>  #undef TRACE_SYSTEM
> 

--
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/