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;
Message-ID: <2039de9b117b11131b7ecc054cca800a3a71831f.camel@gmail.com>
Subject: Re: [PATCH v10 08/24] perf record: Introduce data file at mmap
 buffer object
From:   Riccardo Mancini <rickyman7@gmail.com>
To:     Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     Jiri Olsa <jolsa@redhat.com>, 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>
Date:   Wed, 11 Aug 2021 19:24:42 +0200
In-Reply-To: <9f112977fb46418f6275fef45612c662d03db66d.1626072009.git.alexey.v.bayduraev@linux.intel.com>
References: <cover.1626072008.git.alexey.v.bayduraev@linux.intel.com>
         <9f112977fb46418f6275fef45612c662d03db66d.1626072009.git.alexey.v.bayduraev@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.40.3 (3.40.3-1.fc34) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

On Mon, 2021-07-12 at 09:46 +0300, Alexey Bayduraev wrote:
> Introduce data file and compressor objects into mmap object so
> they could be used to process and store data stream from the
> corresponding kernel data buffer. Make use of the introduced
> per mmap file and compressor when they are initialized and
> available.
> 
> Acked-by: Andi Kleen <ak@linux.intel.com>
> Acked-by: Namhyung Kim <namhyung@gmail.com>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
> ---
>  tools/perf/builtin-record.c | 3 +++
>  tools/perf/util/mmap.c      | 6 ++++++
>  tools/perf/util/mmap.h      | 3 +++
>  3 files changed, 12 insertions(+)

Testing this patch I encounter an issue with the perf test 19 ("import python"):
$ ./perf test 19 -vvv
Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps,
etc
19: 'import perf' in python                                         :
--- start ---
test child forked, pid 147201
python usage test: "echo "import sys ; sys.path.append('python'); import perf" |
'/usr/bin/python2' "
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: python/perf.so: undefined symbol: zstd_fini
test child finished with -1
---- end ----
'import perf' in python: FAILED!

The problem is caused by the fact that the perf.so library for Python is not
compiled with -lzstd, but it includes mmap.c.
Since I don't think zstd_* functionality is used by Python, we can provide stub
functions instead of adding the zstd dependency to the Python library.

Thanks,
Riccardo

> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index cb8d320d20a9..d67a09e26792 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -197,6 +197,9 @@ 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;
> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
> index ab7108d22428..a2c5e4237592 100644
> --- a/tools/perf/util/mmap.c
> +++ b/tools/perf/util/mmap.c
> @@ -230,6 +230,8 @@ void mmap__munmap(struct mmap *map)
>  {
>         bitmap_free(map->affinity_mask.bits);
>  
> +       zstd_fini(&map->zstd_data);
> +
>         perf_mmap__aio_munmap(map);
>         if (map->data != NULL) {
>                 munmap(map->data, mmap__mmap_len(map));
> @@ -291,6 +293,10 @@ int mmap__mmap(struct mmap *map, struct mmap_params *mp,
> int fd, int cpu)
>         map->core.flush = mp->flush;
>  
>         map->comp_level = mp->comp_level;
> +       if (zstd_init(&map->zstd_data, map->comp_level)) {
> +               pr_debug2("failed to init mmap commpressor, error %d\n", errno);
> +               return -1;
> +       }
>  
>         if (map->comp_level && !perf_mmap__aio_enabled(map)) {
>                 map->data = mmap(NULL, mmap__mmap_len(map),
> PROT_READ|PROT_WRITE,
> diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
> index 9d5f589f02ae..c4aed6e89549 100644
> --- a/tools/perf/util/mmap.h
> +++ b/tools/perf/util/mmap.h
> @@ -13,6 +13,7 @@
>  #endif
>  #include "auxtrace.h"
>  #include "event.h"
> +#include "util/compress.h"
>  
>  struct aiocb;
>  
> @@ -43,6 +44,8 @@ struct mmap {
>         struct mmap_cpu_mask    affinity_mask;
>         void            *data;
>         int             comp_level;
> +       struct perf_data_file *file;
> +       struct zstd_data      zstd_data;
>  };
>  
>  struct mmap_params {