Messages in this thread Patch in this message | | | Subject | Re: [PATCH 3/4] uprobes/tracing: Kill the pointless local_save_flags/preempt_count calls | From | Steven Rostedt <> | Date | Mon, 08 Apr 2013 11:58:42 -0400 |
| |
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@redhat.com> > --- > 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; > }
| |