From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965981AbbLPPzz (ORCPT ); Wed, 16 Dec 2015 10:55:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60364 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965780AbbLPPzx (ORCPT ); Wed, 16 Dec 2015 10:55:53 -0500 Date: Wed, 16 Dec 2015 16:55:49 +0100 From: Jiri Olsa To: Adrian Hunter Cc: Arnaldo Carvalho de Melo , Jiri Olsa , lkml , David Ahern , Ingo Molnar , Namhyung Kim , Peter Zijlstra Subject: Re: [PATCH 0/7] perf stat: Change event enable code Message-ID: <20151216155549.GD6882@krava.brq.redhat.com> References: <1449133606-14429-1-git-send-email-jolsa@kernel.org> <20151207210908.GC11564@kernel.org> <566686EF.3040505@intel.com> <20151208135336.GE11564@kernel.org> <5668302C.3030004@intel.com> <566AC4AE.2040304@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <566AC4AE.2040304@intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Dec 11, 2015 at 02:42:22PM +0200, Adrian Hunter wrote: SNIP > > I was wrong about there being any problem using evsel->threads. While the > patch "perf evlist: Factor perf_evlist__(enable|disable) function" changes > the number of threads (from perf_evlist__nr_threads() to thread_map__nr()), > the system_wide check is still done in perf_evsel__run_ioctl(), so > everything is fine. > > WRT "[RFC 7/7] perf tools: Remove perf_evlist__(enable|disable)_event > functions" it might be worth putting the evsel->fd checks that > perf_evlist__[enable|disable]_event() have into perf_evsel__[enable|disable](). > But otherwise it looks fine. > > The gap in testing that I was thinking of is below: > > From: Adrian Hunter > Date: Fri, 11 Dec 2015 11:05:11 +0200 > Subject: [PATCH] perf tools: Make perf_evlist__open() open evsels with their > cpus and threads (like perf record does) > > 'perf record' uses perf_evsel__open() to open events and passes the evsel->cpus > and evsel->threads. Many tests and some tools instead use perf_evlist__open() > which passes instead evlist->cpus and evlist->threads. > > Make perf_evlist__open() follow the 'perf record' behaviour so that a consistent > approach is taken. coo, I'll queue this one for Arnaldo to pick up together with: [RFC 7/7] perf tools: Remove perf_evlist__(enable|disable)_event thanks, jirka > > Signed-off-by: Adrian Hunter > --- > tools/perf/util/evlist.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index d1b6c206bb93..306dacb33d8e 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -1470,7 +1470,7 @@ int perf_evlist__open(struct perf_evlist *evlist) > perf_evlist__update_id_pos(evlist); > > evlist__for_each(evlist, evsel) { > - err = perf_evsel__open(evsel, evlist->cpus, evlist->threads); > + err = perf_evsel__open(evsel, evsel->cpus, evsel->threads); > if (err < 0) > goto out_err; > } > -- > 1.9.1 > >