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:   Sun, 12 Sep 2021 22:46:43 +0200
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 v11 09/24] perf record: Introduce bytes written stats to
 support --max-size option
Message-ID: <YT5nMzMPwy5/zXeh@krava>
References: <cover.1629186429.git.alexey.v.bayduraev@linux.intel.com>
 <c5c886094b1657328c7ec030da140b329282ebb7.1629186429.git.alexey.v.bayduraev@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c5c886094b1657328c7ec030da140b329282ebb7.1629186429.git.alexey.v.bayduraev@linux.intel.com>
Precedence: bulk

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@kernel.org>
> Reviewed-by: Riccardo Mancini <rickyman7@gmail.com>
> Tested-by: Riccardo Mancini <rickyman7@gmail.com>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@linux.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
>