Date:   Sun, 5 Dec 2021 16:14:40 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Alexander Antonov <alexander.antonov@linux.intel.com>,
        Alexei Budankov <abudankov@huawei.com>,
        Riccardo Mancini <rickyman7@gmail.com>
Subject: Re: [PATCH v12 08/16] perf record: Introduce data file at mmap
 buffer object
Message-ID: <YazXYN9r68M/RH9T@krava>
References: <cover.1637675515.git.alexey.v.bayduraev@linux.intel.com>
 <06c298ce2f2f05964fb507524e1eb7080d57da4b.1637675515.git.alexey.v.bayduraev@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <06c298ce2f2f05964fb507524e1eb7080d57da4b.1637675515.git.alexey.v.bayduraev@linux.intel.com>
Precedence: bulk

On Tue, Nov 23, 2021 at 05:08:04PM +0300, Alexey Bayduraev wrote:

SNIP

> +static int record__threads_enabled(struct record *rec)
> +{
> +	return rec->opts.threads_spec;
> +}
> +
>  static bool switch_output_signal(struct record *rec)
>  {
>  	return rec->switch_output.signal &&
> @@ -197,12 +202,16 @@ static int record__write(struct record *rec, struct mmap *map __maybe_unused,
>  {
>  	struct perf_data_file *file = &rec->session->data->file;
>  
> +	if (map && map->file)
> +		file = map->file;
> +
>  	if (perf_data_file__write(file, bf, size) < 0) {
>  		pr_err("failed to write perf data, error: %m\n");
>  		return -1;
>  	}
>  
> -	rec->bytes_written += size;
> +	if (!(map && map->file))
> +		rec->bytes_written += size;

why is this incremented under that condition?

>  
>  	if (record__output_max_size_exceeded(rec) && !done) {
>  		fprintf(stderr, "[ perf record: perf size limit reached (%" PRIu64 " KB),"
> @@ -1102,7 +1111,7 @@ static int record__alloc_thread_data(struct record *rec, struct evlist *evlist)
>  static int record__mmap_evlist(struct record *rec,
>  			       struct evlist *evlist)
>  {

SNIP

> @@ -1574,6 +1601,7 @@ static void record__init_features(struct record *rec)
>  static void
>  record__finish_output(struct record *rec)
>  {
> +	int i;
>  	struct perf_data *data = &rec->data;
>  	int fd = perf_data__fd(data);
>  
> @@ -1582,8 +1610,14 @@ record__finish_output(struct record *rec)
>  
>  	rec->session->header.data_size += rec->bytes_written;
>  	data->file.size = lseek(perf_data__fd(data), 0, SEEK_CUR);
> +	if (record__threads_enabled(rec)) {
> +		for (i = 0; i < data->dir.nr; i++)
> +			data->dir.files[i].size = lseek(data->dir.files[i].fd, 0, SEEK_CUR);
> +	}
>  
>  	if (!rec->no_buildid) {
> +		/* this will be recalculated during process_buildids() */
> +		rec->samples = 0;
>  		process_buildids(rec);
>  
>  		if (rec->buildid_all)
> @@ -2490,8 +2524,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  		status = err;
>  
>  	record__synthesize(rec, true);
> -	/* this will be recalculated during process_buildids() */
> -	rec->samples = 0;

hm, why is this removed?

jirka

>  
>  	if (!err) {
>  		if (!rec->timestamp_filename) {
> @@ -3312,7 +3344,7 @@ int cmd_record(int argc, const char **argv)
>  		goto out_opts;
>  	}
>  
> -	if (rec->opts.kcore)
> +	if (rec->opts.kcore || record__threads_enabled(rec))
>  		rec->data.is_dir = true;
>  
>  	if (rec->opts.comp_level != 0) {
> diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
> index 8e259b9610f8..a3370a8bf307 100644
> --- a/tools/perf/util/mmap.h
> +++ b/tools/perf/util/mmap.h
> @@ -44,6 +44,7 @@ struct mmap {
>  	struct mmap_cpu_mask	affinity_mask;
>  	void		*data;
>  	int		comp_level;
> +	struct perf_data_file *file;
>  };
>  
>  struct mmap_params {
> diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
> index ef6c2715fdd9..ad08c092f3dd 100644
> --- a/tools/perf/util/record.h
> +++ b/tools/perf/util/record.h
> @@ -78,6 +78,7 @@ struct record_opts {
>  	int	      ctl_fd_ack;
>  	bool	      ctl_fd_close;
>  	int	      synth;
> +	int	      threads_spec;
>  };
>  
>  extern const char * const *record_usage;
> -- 
> 2.19.0
>