Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752637Ab0G1CvJ (ORCPT ); Tue, 27 Jul 2010 22:51:09 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:61812 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1751804Ab0G1CvC (ORCPT ); Tue, 27 Jul 2010 22:51:02 -0400 Message-ID: <4C4F9C3A.6020406@cn.fujitsu.com> Date: Wed, 28 Jul 2010 10:55:54 +0800 From: Li Zefan User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20090513 Fedora/3.0-2.3.beta2.fc11 Thunderbird/3.0b2 MIME-Version: 1.0 To: Ian Munsie CC: linux-kernel@vger.kernel.org, Steven Rostedt , Frederic Weisbecker , Ingo Molnar , Masami Hiramatsu , Jiri Olsa , Tejun Heo Subject: Re: [PATCH 1/2] ftrace: record command lines at more appropriate moment References: <1280284180-17863-1-git-send-email-imunsie@au1.ibm.com> In-Reply-To: <1280284180-17863-1-git-send-email-imunsie@au1.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3992 Lines: 88 Ian Munsie wrote: > From: Ian Munsie > > Previously, when tracing was activated through debugfs, regardless of > which tracing plugin (if any) were activated, the probe_sched_switch and > probe_sched_wakeup probes from the sched_switch plugin would be > activated. This appears to have been a hack to use them to record the > command lines of active processes as they were scheduled. > > That approach would suffer if many processes were being scheduled that > were not generating events as they would consume entries in the > saved_cmdlines buffer that could otherwise have been used by other > processes that were actually generating events. > > It also had the problem that events could be mis-attributed - in the > common situation of a process forking then execing a new process, the > change of the process command would not be noticed for some time after > the exec until the process was next scheduled. > > If the trace was read after the fact this would generally go unnoticed > because at some point the process would be scheduled and the entry in > the saved_cmdlines buffer would be updated so that the new command would > be reported when the trace was eventually read. However, if the events > were being read live (e.g. through trace_pipe), the events just after > the exec and before the process was next scheduled would show the > incorrect command (though the PID would be correct). > > This patch removes the sched_switch hack altogether and instead records > the commands at a more appropriate moment - at the same time the PID of > the process is recorded (i.e. when an entry on the ring buffer is > reserved). This means that the recorded command line is much more likely > to be correct when the trace is read, either live or after the fact, so > long as the command line still resides in the saved_cmdlines buffer. > > It is still not guaranteed to be correct in all situations. For instance > if the trace is read after the fact rather than live (consider events > generated by a process before an exec - in the below example they would > be attributed to sleep rather than stealpid since the entry in > saved_cmdlines would have changed before the event was read), but this > is no different to the current situation and the alternative would be to > store the command line with each and every event. > ... > > Signed-off-by: Ian Munsie > --- > kernel/trace/trace.c | 3 +-- > kernel/trace/trace_events.c | 11 ----------- > kernel/trace/trace_functions.c | 2 -- > kernel/trace/trace_functions_graph.c | 2 -- > kernel/trace/trace_sched_switch.c | 10 ---------- > 5 files changed, 1 insertions(+), 27 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index 4b1122d..f8458c3 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -1023,8 +1023,6 @@ void tracing_stop(void) > spin_unlock_irqrestore(&tracing_start_lock, flags); > } > > -void trace_stop_cmdline_recording(void); > - > static void trace_save_cmdline(struct task_struct *tsk) > { > unsigned pid, idx; > @@ -1112,6 +1110,7 @@ tracing_generic_entry_update(struct trace_entry *entry, unsigned long flags, > { > struct task_struct *tsk = current; > > + tracing_record_cmdline(tsk); Now this function is called everytime a tracepoint is triggered, so did you run some benchmarks to see if the performance is improved or even worse? Another problem in this patch is, tracing_generic_entry_update() is also called by perf, but cmdline recoding is not needed in perf. > entry->preempt_count = pc & 0xff; > entry->pid = (tsk) ? tsk->pid : 0; > entry->lock_depth = (tsk) ? tsk->lock_depth : 0; ... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/