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

[Jiri Olsa]

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@xxxxxxxxxx>
> Reviewed-by: Riccardo Mancini <rickyman7@xxxxxxxxx>
> Tested-by: Riccardo Mancini <rickyman7@xxxxxxxxx>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@xxxxxxxxxxxxxxx>
> ---
>  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
>