From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754660AbZGAQde (ORCPT ); Wed, 1 Jul 2009 12:33:34 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752295AbZGAQd0 (ORCPT ); Wed, 1 Jul 2009 12:33:26 -0400 Received: from mail-ew0-f210.google.com ([209.85.219.210]:59649 "EHLO mail-ew0-f210.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751727AbZGAQdZ (ORCPT ); Wed, 1 Jul 2009 12:33:25 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=bkwxbPO6LZD+hjZab7DfdR/4+4+WK8JucU+erUbE77j2yIuOaHN8//U0kQpmeRK5FV s9giKjbTyNZ5sOV77W1samkIrc0iDHKeb6PsYUT/FNKAKzCkYPRUC6MhEVZXdOOOE3rb Xkq5y+8kt8/OAZNgPTPpeI1g39o5ZsoUXHsus= Date: Wed, 1 Jul 2009 18:33:25 +0200 From: Frederic Weisbecker To: Ingo Molnar Cc: LKML , Peter Zijlstra , Mike Galbraith , Paul Mackerras , Anton Blanchard , Arnaldo Carvalho de Melo Subject: Re: [PATCH 0/3] perfcounter: callchain symbol resolving and fixes Message-ID: <20090701163324.GD5097@nowhere> References: <1246419315-9968-1-git-send-email-fweisbec@gmail.com> <20090701081814.GA25593@elte.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090701081814.GA25593@elte.hu> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 01, 2009 at 10:18:14AM +0200, Ingo Molnar wrote: > > * Frederic Weisbecker wrote: > > > Hi, > > > > This patchset provides the symbol resolving for callchains. > > Example: > > > > perf report -s sym -c > > > > 5.40% [k] __d_lookup > > 3.60% > > __d_lookup > > perf_callchain > > perf_counter_overflow > > intel_pmu_handle_irq > > perf_counter_nmi_handler > > notifier_call_chain > > atomic_notifier_call_chain > > notify_die > > do_nmi > > nmi > > do_lookup > > __link_path_walk > > path_walk > > do_path_lookup > > user_path_at > > vfs_fstatat > > vfs_lstat > > sys_newlstat > > system_call_fastpath > > __lxstat > > 0x406fb1 > > nice! > > > Sorry about the third patch, it's a kind of all-in-one monolithic > > thing which gathers various fixes. I should have granulate it... > > No problem, it's good enough - it's all about the same topic. > > > > > Still in my plans: > > > > - profit we have a tree to display a better graph hierarchy > > - let the user provide a limit for hit percentage, depth, number of > > backtraces, etc... > > - better output > > - colors > > > > And another one: > > > > - remove the perfcounter internal nmi call frame (ie: every nmi frame) > > so that we drop this header from each callchain: > > > > perf_callchain > > perf_counter_overflow > > intel_pmu_handle_irq > > perf_counter_nmi_handler > > notifier_call_chain > > atomic_notifier_call_chain > > notify_die > > do_nmi > > nmi > > Sounds good. I suspect this latter one is the most important one > because right now the backtrace output screen real estate is > dominated by the repetitive nmi entries, making it hard to interpret > the result 'at a glance'. > > I think we should skip those NMI entries right in the kernel - that > will also make call-chain event records quite a bit smaller, by > about 72 bytes per call-chain record. > > We can do the skipping by using this backtrace-generator callback in > arch/x86/kernel/cpu/perf_counter.c: > > static int backtrace_stack(void *data, char *name) > { > /* Process all stacks: */ > return 0; > } > > The 'name' parameter passed in signals the type of stack frame we > are processing. If you look into arch/x86/kernel/dumpstack_64.c, it > can be one of these strings: > > static char ids[][8] = { > [DEBUG_STACK - 1] = "#DB", > [NMI_STACK - 1] = "NMI", > [DOUBLEFAULT_STACK - 1] = "#DF", > [STACKFAULT_STACK - 1] = "#SS", > [MCE_STACK - 1] = "#MC", > > A quick check to see whether this concept works would be expose the > ids array and do: > > static int PER_CPU(int, is_nmi_frame); > > static int backtrace_stack(void *data, char *name) > { > if (name == x86_stack_ids[NMI_STACK-1]) IIRC, gcc manages to factorize the string table in the elf format right? So that a simple == should indeed work here. Because if you look at dumpstack_64.c, the calls to ->stack() use plain const string for some of them: ops->stack(data, "IRQ") But "NMI" is always passed by its real address in the ids so that should work without problem here. (I just feared about using strcmp is such a fastpath). > per_cpu(is_nmi_frame, raw_processor_id()) = 1; > else > per_cpu(is_nmi_frame, raw_processor_id()) = 0; > > /* Process all stacks: */ > return 0; > } > > and to add something like this to backtrace_address(): > > if (per_cpu(is_nmi_frame, raw_processor_id()) > return; > > Ingo Heh, looks like I'll almost only have to copy-paste this mail :) Another solution would be to handle an IGNORE return value from dump_trace() instead of always terminate the trace when ->stack() < 0 Would it be useful for other kind of uses? For now I just asssume ignoring a stack is not a known pattern so I'll just implement your solution. Thanks.