From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932165AbbLQXbd (ORCPT ); Thu, 17 Dec 2015 18:31:33 -0500 Received: from mail-wm0-f54.google.com ([74.125.82.54]:37014 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751845AbbLQXbc (ORCPT ); Thu, 17 Dec 2015 18:31:32 -0500 MIME-Version: 1.0 In-Reply-To: <20151217140123.GA15533@two.firstfloor.org> References: <1450227266-2501-1-git-send-email-andi@firstfloor.org> <20151217140123.GA15533@two.firstfloor.org> Date: Thu, 17 Dec 2015 15:31:30 -0800 Message-ID: Subject: Re: Add top down metrics to perf stat v2 From: Stephane Eranian To: Andi Kleen Cc: Arnaldo Carvalho de Melo , Peter Zijlstra , Jiri Olsa , Ingo Molnar , LKML , Namhyung Kim Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 17, 2015 at 6:01 AM, Andi Kleen wrote: > On Thu, Dec 17, 2015 at 02:27:58AM -0800, Stephane Eranian wrote: >> > S0-C1 2 4175583320.00 topdown-slots-retired (100.00%) >> > S0-C1 2 1743329246 topdown-recovery-bubbles # 22.22% bad speculation (100.00%) >> > S0-C1 2 6138901193.50 topdown-slots-issued # 46.99% backend bound >> > >> I don't see how this output could be very useful. What matters is the >> percentage in the comments >> and not so much the raw counts because what is the unit? Same remark >> holds for the percentage. >> I think you need to explain or show that this is % of issue slots and >> not cycles. > > The events already say slots, not cycles. Except for recovery-bubbles. Could add > -slots there too if you think it's helpful, although it would make the > name very long and may not fit into the column anymore. > I would drop the default output, it is not useful. I would not add a --topdown option but instead a --metric option with arguments such that other metrics could be added later: $ perf stat --metrics topdown -I 1000 -a sleep 100 If you do this, you do not need the --metric-only option The double --topdown is confusing. Why force --per-core when HT is on. I know you you need to aggregate per core, but you could still display globally. And then if user requests --per-core, then display per core. Same if user specifies --per-socket. I know this requires some more plumbing inside perf but it would be clearer and simpler to interpret to users. One bug I found when testing is that if you do with HT-on: $ perf stat -a --topdown -I 1000 --metric-only sleep 100 Then you get data for frontend and backend but nothing for retiring or bad speculation. I suspect it is because you expect --metric-only to be used only when you have the double --topdown. That's why I think this double topdown is confusing. If you do as I suggest, it will be much simpler. >> >> > 1.535832673 seconds time elapsed >> > >> > $ perf stat --topdown --topdown --metric-only -I 100 ./BC1s >> >> When I tried from your git tree the --metric-only option was not recognized. > > See below. >> >> > 0.100576098 frontend bound retiring bad speculation backend bound >> > 0.100576098 8.83% 48.93% 35.24% 7.00% >> > 0.200800845 8.84% 48.49% 35.53% 7.13% >> > 0.300905983 8.73% 48.64% 35.58% 7.05% >> > ... >> > >> This kind of output is more meaningful and clearer for end-users based >> on my experience >> and you'd like it per-core possibly. > > Yes --metric-only is a lot clearer. > > per-core is supported and automatically enabled with SMT on. > >> > Full tree available in >> > git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc perf/top-down-11 >> >> That is in the top-down-2 branch instead, I think. > > Sorry, typo > > The correct branch is perf/top-down-10 > > I also updated it now with the latest review feedback changes. > > top-down-2 is an really old branch that indeed didn't have metric-only. > > -Andi > > -- > ak@linux.intel.com -- Speaking for myself only.