From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933480AbbFWOFK (ORCPT ); Tue, 23 Jun 2015 10:05:10 -0400 Received: from mail.kernel.org ([198.145.29.136]:56503 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932925AbbFWOFF (ORCPT ); Tue, 23 Jun 2015 10:05:05 -0400 Date: Tue, 23 Jun 2015 11:05:01 -0300 From: Arnaldo Carvalho de Melo To: Jiri Olsa Cc: Jiri Olsa , lkml , Adrian Hunter , Andi Kleen , David Ahern , Ingo Molnar , Namhyung Kim , Peter Zijlstra , Stephane Eranian Subject: Re: [PATCHv4 00/27] perf stat: Introduce --per-thread option Message-ID: <20150623140501.GB3489@kernel.org> References: <1435012588-9007-1-git-send-email-jolsa@kernel.org> <20150622230600.GB8510@kernel.org> <20150623072200.GB749@krava.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150623072200.GB749@krava.redhat.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Jun 23, 2015 at 09:22:00AM +0200, Jiri Olsa escreveu: > On Mon, Jun 22, 2015 at 08:06:00PM -0300, Arnaldo Carvalho de Melo wrote: > > Em Tue, Jun 23, 2015 at 12:36:01AM +0200, Jiri Olsa escreveu: > > > adding the possibility to display stat data per thread. > > > Allowing following commands and output: > > > $ perf stat -e cycles,instructions --per-thread -p 30190,30242 > > While testing Adrian's Intel PT patchkit I realised we have --per-thread > > in 'record', wonder if using a long option with the exact same name but > > different meanings for 'stat' and 'record' would cause confusion... > I think the name fits for both stat and record.. and both are doing different For record it is vague, for stat, it seems to fit. For record it really should be --mmap-per-thread, but then we start getting what may seem overly long options, but then, its an oddball 'record' option... We have perf_evsel__open_per_thread(), that is used in builtin-stat.c, and that means: Open a descriptor per thread, but then, perf_target.per_thread only means, hey optimize things when setting up the perf_event_attr, which so far means just use it as one of the things to decide if PERF_SAMPLE_TIME should be set. Just raising some concerns about consistency... > thing, so both --per-thread are also different.. and well documented ;-) The fact that something is documented doesn't avoid the initial expectation that since it has the same name, and that its part of a collection of tools, that it should have the same meaning, which means maybe the existing option, being the oddball, should be changed to better reflect what it asks the tool to do. In fact, when we don't have to read documentation and things make sense, that is the ideal, huh? We should be more careful with this to avoid breaking too many expectations when switching from one tool to another in the same collection. What others have to say? Ingo? - Arnaldo