Re: [PATCH v8 08/58] perf data: Add open flag

Namhyung Kim
2026-04-29T07:08:39Z

On Tue, Apr 28, 2026 at 12:18:13AM -0700, Ian Rogers wrote:
> Avoid double opens and ensure only open files are closed. This
> addresses some issues with python integration where the data file
> wants to be opened before being given to a session.
>
> Assisted-by: Gemini:gemini-3.1-pro-preview
> Signed-off-by: Ian Rogers <irogers@google.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
> Changes in v2:
>
> 1. Fixed File Rotation: In perf_data__switch() , I added data->open =
> false; after the file is closed. This ensures that the subsequent
> perf_data__open() call will not exit early and will successfully
> open the new file.
>
> 2. Fixed Memory Leak: In open_dir() , I added a call to
> zfree(&data->file.path) if mkdir() fails, preventing the leak of
> the path string.
> ---
> tools/perf/util/data.c | 26 ++++++++++++++++++++++----
> tools/perf/util/data.h | 4 +++-
> 2 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> index 94dc534a7386..17baf71897d1 100644
> --- a/tools/perf/util/data.c
> +++ b/tools/perf/util/data.c
> @@ -346,8 +346,10 @@ static int open_dir(struct perf_data *data)
> return -1;
>
> if (perf_data__is_write(data) &&
> - mkdir(data->path, S_IRWXU) < 0)
> + mkdir(data->path, S_IRWXU) < 0) {
> + zfree(&data->file.path);
> return -1;
> + }
>
> ret = open_file(data);
>
> @@ -360,9 +362,16 @@ static int open_dir(struct perf_data *data)
>
> int perf_data__open(struct perf_data *data)
> {
> - if (check_pipe(data))
> + int ret;
> +
> + if (data->open)
> return 0;
>
> + if (check_pipe(data)) {
> + data->open = true;
> + return 0;
> + }
> +
> /* currently it allows stdio for pipe only */
> data->file.use_stdio = false;
>
> @@ -375,16 +384,24 @@ int perf_data__open(struct perf_data *data)
> if (perf_data__is_read(data))
> data->is_dir = is_dir(data);
>
> - return perf_data__is_dir(data) ?
> - open_dir(data) : open_file_dup(data);
> + ret = perf_data__is_dir(data) ? open_dir(data) : open_file_dup(data);
> +
> + if (!ret)
> + data->open = true;
> +
> + return ret;
> }
>
> void perf_data__close(struct perf_data *data)
> {
> + if (!data->open)
> + return;
> +
> if (perf_data__is_dir(data))
> perf_data__close_dir(data);
>
> perf_data_file__close(&data->file);
> + data->open = false;
> }
>
> static ssize_t perf_data_file__read(struct perf_data_file *file, void *buf, size_t size)
> @@ -457,6 +474,7 @@ int perf_data__switch(struct perf_data *data,
>
> if (!at_exit) {
> perf_data_file__close(&data->file);
> + data->open = false;
> ret = perf_data__open(data);
> if (ret < 0)
> goto out;
> diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
> index 8299fb5fa7da..76f57f60361f 100644
> --- a/tools/perf/util/data.h
> +++ b/tools/perf/util/data.h
> @@ -50,6 +50,8 @@ struct perf_data {
> const char *path;
> /** @file: Underlying file to be used. */
> struct perf_data_file file;
> + /** @open: Has the file or directory been opened. */
> + bool open;
> /** @is_pipe: Underlying file is a pipe. */
> bool is_pipe;
> /** @is_dir: Underlying file is a directory. */
> @@ -59,7 +61,7 @@ struct perf_data {
> /** @in_place_update: A file opened for reading but will be written to. */
> bool in_place_update;
> /** @mode: Read or write mode. */
> - enum perf_data_mode mode;
> + enum perf_data_mode mode:8;
>
> struct {
> /** @version: perf_dir_version. */
> --
> 2.54.0.545.g6539524ca2-goog
>