Re: [PATCH 3/4] uprobes/tracing: Kill the pointlesslocal_save_flags/preempt_count calls

[Steven Rostedt]

On Fri, 2013-03-29 at 19:15 +0100, Oleg Nesterov wrote:
> uprobe_trace_func() is never called with irqs or preemption
> disabled, no need to ask preempt_count() or local_save_flags().
> 
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> ---
>  kernel/trace/trace_uprobe.c |   10 +++-------
>  1 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index 8e00901..43d258d 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -492,17 +492,13 @@ static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
>  	struct ring_buffer_event *event;
>  	struct ring_buffer *buffer;
>  	u8 *data;
> -	int size, i, pc;
> -	unsigned long irq_flags;
> +	int size, i;
>  	struct ftrace_event_call *call = &tu->call;
>  
> -	local_save_flags(irq_flags);
> -	pc = preempt_count();

How about instead, just change the above two and have:

	/* uprobes are never called with preemption disabled */
	pc = 0;
	irq_flags = 0;

and leave the rest the same. This will help in future reviewers of the
code to not have to look up what that "0, 0" is for, and then wonder if
it should be that way. gcc should optimize it to be exactly the same as
this patch.

-- Steve

> -
>  	size = sizeof(*entry) + tu->size;
>  
>  	event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
> -						  size, irq_flags, pc);
> +						  size, 0, 0);
>  	if (!event)
>  		return 0;
>  
> @@ -513,7 +509,7 @@ static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
>  		call_fetch(&tu->args[i].fetch, regs, data + tu->args[i].offset);
>  
>  	if (!filter_current_check_discard(buffer, call, entry, event))
> -		trace_buffer_unlock_commit(buffer, event, irq_flags, pc);
> +		trace_buffer_unlock_commit(buffer, event, 0, 0);
>  
>  	return 0;
>  }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/