Re: [PATCH v4 01/11] tracing: Add support for SOFT_DISABLE to syscall events

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

(2013/07/30 1:40), Tom Zanussi wrote:
> The original SOFT_DISABLE patches didn't add support for soft disable
> of syscall events; this adds it and paves the way for future patches
> allowing triggers to be added to syscall events, since triggers are
> built on top of SOFT_DISABLE.
> 
> Add an array of ftrace_event_file pointers indexed by syscall number
> to the trace array and remove the existing enabled bitmaps, which as a
> result are now redundant.  The ftrace_event_file structs in turn
> contain the soft disable flags we need for per-syscall soft disable
> accounting; later patches add additional 'trigger' flags and
> per-syscall triggers and filters.

At this point, this looks OK for me.

Note, this could be racy after introducing event-trigger which makes
some other event soft-disabled. Currently only function tracer makes
soft-disabled and it works only on the default instance.
However introducing trigger interface, there can be events in soft-mode,
and  ftrace_event_enable_disable(file, set = 0) to those events always fail.

> 
> Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
> ---
>  kernel/trace/trace.h          |  4 ++--
>  kernel/trace/trace_syscalls.c | 34 ++++++++++++++++++++++++++++------
>  2 files changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index afaae41..3dd994e8 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -192,8 +192,8 @@ struct trace_array {
>  #ifdef CONFIG_FTRACE_SYSCALLS
>  	int			sys_refcount_enter;
>  	int			sys_refcount_exit;
> -	DECLARE_BITMAP(enabled_enter_syscalls, NR_syscalls);
> -	DECLARE_BITMAP(enabled_exit_syscalls, NR_syscalls);
> +	struct ftrace_event_file *enter_syscall_files[NR_syscalls];
> +	struct ftrace_event_file *exit_syscall_files[NR_syscalls];
>  #endif
>  	int			stop_count;
>  	int			clock_id;
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index 8fd0365..294cfe8 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -302,6 +302,7 @@ static int __init syscall_exit_define_fields(struct ftrace_event_call *call)
>  static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id)
>  {
>  	struct trace_array *tr = data;
> +	struct ftrace_event_file *ftrace_file;
>  	struct syscall_trace_enter *entry;
>  	struct syscall_metadata *sys_data;
>  	struct ring_buffer_event *event;
> @@ -314,7 +315,12 @@ static void ftrace_syscall_enter(void *data, struct pt_regs *regs, long id)
>  	syscall_nr = trace_get_syscall_nr(current, regs);
>  	if (syscall_nr < 0)
>  		return;
> -	if (!test_bit(syscall_nr, tr->enabled_enter_syscalls))
> +

OK, since all tracepoint handlers run under rcu_read_lock, we can safely
use rcu_dereference_raw() here. :)
To avoid misread, I think you'd better add a comment here.

> +	ftrace_file = rcu_dereference_raw(tr->enter_syscall_files[syscall_nr]);
> +	if (!ftrace_file)
> +		return;
> +
> +	if (test_bit(FTRACE_EVENT_FL_SOFT_DISABLED_BIT, &ftrace_file->flags))
>  		return;
>  

Thank you,


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com