Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 22 Sep 2021 23:16:55 +0200
From:   Jiri Olsa <jolsa@redhat.com>
To:     Ian Rogers <irogers@google.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Namhyung Kim <namhyung@kernel.org>,
        linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
        eranian@google.com
Subject: Re: [PATCH v2] perf evlist: Remove group option.
Message-ID: <YUudR8ST7qGYdFQE@krava>
References: <20210921181349.199780-1-irogers@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210921181349.199780-1-irogers@google.com>
Precedence: bulk

On Tue, Sep 21, 2021 at 11:13:49AM -0700, Ian Rogers wrote:

SNIP

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 8feef3a05af7..9cd79513eebb 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -1109,14 +1109,6 @@ static PyObject *pyrf_evlist__open(struct pyrf_evlist *pevlist,
>  				   PyObject *args, PyObject *kwargs)
>  {
>  	struct evlist *evlist = &pevlist->evlist;
> -	int group = 0;
> -	static char *kwlist[] = { "group", NULL };
> -
> -	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|OOii", kwlist, &group))
> -		return NULL;
> -
> -	if (group)
> -		evlist__set_leader(evlist);
>  
>  	if (evlist__open(evlist) < 0) {
>  		PyErr_SetFromErrno(PyExc_OSError);
> diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> index bff669b615ee..9e694db7c7ee 100644
> --- a/tools/perf/util/record.c
> +++ b/tools/perf/util/record.c
> @@ -99,13 +99,6 @@ void evlist__config(struct evlist *evlist, struct record_opts *opts, struct call
>  	bool use_comm_exec;
>  	bool sample_id = opts->sample_id;
>  
> -	/*
> -	 * Set the evsel leader links before we configure attributes,
> -	 * since some might depend on this info.
> -	 */
> -	if (opts->group)
> -		evlist__set_leader(evlist);
> -

I don't mind erasing that, but just curious if you're going
to add something which would clash with this? it does not
look too complex for the code.. but still, let's remove it
if it's in 'legacy mode' for this long ;-)

thanks,
jirka


>  	if (evlist->core.cpus->map[0] < 0)
>  		opts->no_inherit = true;
>  
> diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
> index 68f471d9a88b..d71dee9ce41c 100644
> --- a/tools/perf/util/record.h
> +++ b/tools/perf/util/record.h
> @@ -13,7 +13,6 @@ struct option;
>  
>  struct record_opts {
>  	struct target target;
> -	bool	      group;
>  	bool	      inherit_stat;
>  	bool	      no_buffering;
>  	bool	      no_inherit;
> -- 
> 2.33.0.464.g1972c5931b-goog
>