linux-kernel - Re: [PATCH v11 09/24] perf record: Introduce bytes written stats to support --max-size option

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YT5nMzMPwy5/zXeh@krava>
Date:   Sun, 12 Sep 2021 22:46:43 +0200
From:   Jiri Olsa <jolsa@...hat.com>
To:     Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Alexander Antonov <alexander.antonov@...ux.intel.com>,
        Alexei Budankov <abudankov@...wei.com>,
        Riccardo Mancini <rickyman7@...il.com>
Subject: Re: [PATCH v11 09/24] perf record: Introduce bytes written stats to
 support --max-size option

On Tue, Aug 17, 2021 at 11:23:12AM +0300, Alexey Bayduraev wrote:
> Adding a function to calculate the total amount of data written
> and using it to support the --max-size option.
> 
> Acked-by: Namhyung Kim <namhyung@...nel.org>
> Reviewed-by: Riccardo Mancini <rickyman7@...il.com>
> Tested-by: Riccardo Mancini <rickyman7@...il.com>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>
> ---
>  tools/perf/builtin-record.c | 30 +++++++++++++++++++++++++++---
>  tools/perf/util/mmap.h      |  1 +
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index eff6f8db60b2..cb155f1ba979 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -186,10 +186,28 @@ static bool switch_output_time(struct record *rec)
>  	       trigger_is_ready(&switch_output_trigger);
>  }
>  
> +static u64 record__bytes_written(struct record *rec)
> +{
> +	int t, tm;
> +	struct record_thread *thread_data = rec->thread_data;
> +	u64 bytes_written = rec->bytes_written;
> +
> +	for (t = 0; t < rec->nr_threads; t++) {
> +		for (tm = 0; tm < thread_data[t].nr_mmaps; tm++) {
> +			if (thread_data[t].maps)
> +				bytes_written += thread_data[t].maps[tm]->bytes_written;
> +			if (thread_data[t].overwrite_maps)
> +				bytes_written += thread_data[t].overwrite_maps[tm]->bytes_written;
> +		}
> +	}
> +
> +	return bytes_written;
> +}
> +
>  static bool record__output_max_size_exceeded(struct record *rec)
>  {
>  	return rec->output_max_size &&
> -	       (rec->bytes_written >= rec->output_max_size);
> +	       (record__bytes_written(rec) >= rec->output_max_size);
>  }
>  
>  static int record__write(struct record *rec, struct mmap *map __maybe_unused,
> @@ -205,15 +223,21 @@ static int record__write(struct record *rec, struct mmap *map __maybe_unused,
>  		return -1;
>  	}
>  
> -	rec->bytes_written += size;
> +	if (map && map->file)
> +		map->bytes_written += size;
> +	else
> +		rec->bytes_written += size;
>  
>  	if (record__output_max_size_exceeded(rec) && !done) {

also instead of slowing down each write with calling
record__output_max_size_exceeded and risking the race with done,
I think this should be called as part of trigger framework

we could call this check periodically every second or such,
we don't mind the size overhead.. IMO it's still better than
caling this each time we write to the file

jirka

>  		fprintf(stderr, "[ perf record: perf size limit reached (%" PRIu64 " KB),"
>  				" stopping session ]\n",
> -				rec->bytes_written >> 10);
> +				record__bytes_written(rec) >> 10);
>  		done = 1;
>  	}
>  
> +	if (map && map->file)
> +		return 0;
> +
>  	if (switch_output_size(rec))
>  		trigger_hit(&switch_output_trigger);
>  
> diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
> index c4aed6e89549..67d41003d82e 100644
> --- a/tools/perf/util/mmap.h
> +++ b/tools/perf/util/mmap.h
> @@ -46,6 +46,7 @@ struct mmap {
>  	int		comp_level;
>  	struct perf_data_file *file;
>  	struct zstd_data      zstd_data;
> +	u64		      bytes_written;
>  };
>  
>  struct mmap_params {
> -- 
> 2.19.0
>