Re: [PATCH v12 08/16] perf record: Introduce data file at mmap buffer object

Jiri Olsa
2021-12-05T15:14:53Z

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
>