From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752707AbbJWNvq (ORCPT ); Fri, 23 Oct 2015 09:51:46 -0400 Received: from mail.kernel.org ([198.145.29.136]:59262 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751256AbbJWNvp (ORCPT ); Fri, 23 Oct 2015 09:51:45 -0400 Date: Fri, 23 Oct 2015 10:51:41 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: linux-kernel@vger.kernel.org, Alexei Starovoitov , Peter Zijlstra , Li Zefan , pi3orama@163.com Subject: Re: [RFC PATCH] perf tools: Don't set inherit bit for system wide evsel Message-ID: <20151023135141.GD27006@kernel.org> References: <1445597029-133332-1-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1445597029-133332-1-git-send-email-wangnan0@huawei.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 Fri, Oct 23, 2015 at 10:43:49AM +0000, Wang Nan escreveu: > Inherit bit is useless for a system wide evsel [1]. Further kernel > improvements are giving more constrain [2] on inherit events. This > patch set inherit bit to 0 to avoid potential constrains. > > [1] http://lkml.kernel.org/r/20151022124142.GQ17308@twins.programming.kicks-ass.net > [2] http://lkml.kernel.org/r/1445559014-4667-1-git-send-email-ast@kernel.org > > Signed-off-by: Wang Nan > Cc: Arnaldo Carvalho de Melo > Cc: Alexei Starovoitov > Cc: Peter Zijlstra > Cc: Li Zefan > Cc: pi3orama@163.com > Link: http://lkml.kernel.org/n/ebpf-0tgilipxoo6fiebcxu3ft866@git.kernel.org > --- > > evsel->system_wide doesn't correct reflect whether this evsel is system > wide or not, so checks pid when invoking perf_event_open, and it is > always correct. Can't we do this at perf_evlist__config() or perf_evsel__config() time? We have record_opts at perf_evsel__config() time and I think we should leave changing the attr at perf_evsel__open() time for feature fallbacks, i.e. something we will only know when trying to use, which is different from this inherit-on-syswide case, that we know far in advance we will not need. - Arnaldo > --- > tools/perf/util/evsel.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 5566b16..e2d6c9a 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -1337,6 +1337,7 @@ retry_sample_id: > > for (thread = 0; thread < nthreads; thread++) { > int group_fd; > + struct perf_event_attr attr; > > if (!evsel->cgrp && !evsel->system_wide) > pid = thread_map__pid(threads, thread); > @@ -1346,7 +1347,10 @@ retry_open: > pr_debug2("sys_perf_event_open: pid %d cpu %d group_fd %d flags %#lx\n", > pid, cpus->map[cpu], group_fd, flags); > > - FD(evsel, cpu, thread) = sys_perf_event_open(&evsel->attr, > + attr = evsel->attr; > + if (pid == -1) > + attr.inherit = 0; > + FD(evsel, cpu, thread) = sys_perf_event_open(&attr, > pid, > cpus->map[cpu], > group_fd, flags); > -- > 1.8.3.4