[PATCH v5 02/50] libperf: Lazily allocate/size mmap event copy

Ian Rogers posted 50 patches 2 years ago
[PATCH v5 02/50] libperf: Lazily allocate/size mmap event copy
Posted by Ian Rogers 2 years ago
The event copy in the mmap is used to have storage to read an
event. Not all users of mmaps read the events, such as perf
record. The amount of buffer was also statically set to
PERF_SAMPLE_MAX_SIZE rather than the amount necessary from the
header's event size. Switch to a model where the event_copy is
reallocated if too small to the event's size. This adds the potential
for the event to move, so if a copy of the event pointer were stored
it could be broken. All the current users do:

  while(event = perf_mmap__read_event()) { ... }

and so they would be broken due to the event being overwritten if they
had stored the pointer. Manual inspection and address sanitizer
testing also shows the event pointer not being stored.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/perf/include/internal/mmap.h |  3 ++-
 tools/lib/perf/mmap.c                  | 21 ++++++++++++++++++---
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/lib/perf/include/internal/mmap.h b/tools/lib/perf/include/internal/mmap.h
index 5a062af8e9d8..5f08cab61ece 100644
--- a/tools/lib/perf/include/internal/mmap.h
+++ b/tools/lib/perf/include/internal/mmap.h
@@ -33,7 +33,8 @@ struct perf_mmap {
 	bool			 overwrite;
 	u64			 flush;
 	libperf_unmap_cb_t	 unmap_cb;
-	char			 event_copy[PERF_SAMPLE_MAX_SIZE] __aligned(8);
+	void			*event_copy;
+	size_t			 event_copy_sz;
 	struct perf_mmap	*next;
 };
 
diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
index 2184814b37dd..c829db7bf1fa 100644
--- a/tools/lib/perf/mmap.c
+++ b/tools/lib/perf/mmap.c
@@ -19,6 +19,7 @@
 void perf_mmap__init(struct perf_mmap *map, struct perf_mmap *prev,
 		     bool overwrite, libperf_unmap_cb_t unmap_cb)
 {
+	/* Assume fields were zero initialized. */
 	map->fd = -1;
 	map->overwrite = overwrite;
 	map->unmap_cb  = unmap_cb;
@@ -51,13 +52,19 @@ int perf_mmap__mmap(struct perf_mmap *map, struct perf_mmap_param *mp,
 
 void perf_mmap__munmap(struct perf_mmap *map)
 {
-	if (map && map->base != NULL) {
+	if (!map)
+		return;
+
+	free(map->event_copy);
+	map->event_copy = NULL;
+	map->event_copy_sz = 0;
+	if (map->base) {
 		munmap(map->base, perf_mmap__mmap_len(map));
 		map->base = NULL;
 		map->fd = -1;
 		refcount_set(&map->refcnt, 0);
 	}
-	if (map && map->unmap_cb)
+	if (map->unmap_cb)
 		map->unmap_cb(map);
 }
 
@@ -223,9 +230,17 @@ static union perf_event *perf_mmap__read(struct perf_mmap *map,
 		 */
 		if ((*startp & map->mask) + size != ((*startp + size) & map->mask)) {
 			unsigned int offset = *startp;
-			unsigned int len = min(sizeof(*event), size), cpy;
+			unsigned int len = size, cpy;
 			void *dst = map->event_copy;
 
+			if (size > map->event_copy_sz) {
+				dst = realloc(map->event_copy, size);
+				if (!dst)
+					return NULL;
+				map->event_copy = dst;
+				map->event_copy_sz = size;
+			}
+
 			do {
 				cpy = min(map->mask + 1 - (offset & map->mask), len);
 				memcpy(dst, &data[offset & map->mask], cpy);
-- 
2.43.0.rc1.413.gea7ed67945-goog
Re: [PATCH v5 02/50] libperf: Lazily allocate/size mmap event copy
Posted by Arnaldo Carvalho de Melo 2 years ago
Em Mon, Nov 27, 2023 at 02:08:14PM -0800, Ian Rogers escreveu:
> The event copy in the mmap is used to have storage to read an
> event. Not all users of mmaps read the events, such as perf
> record. The amount of buffer was also statically set to
> PERF_SAMPLE_MAX_SIZE rather than the amount necessary from the
> header's event size. Switch to a model where the event_copy is
> reallocated if too small to the event's size. This adds the potential
> for the event to move, so if a copy of the event pointer were stored
> it could be broken. All the current users do:
> 
>   while(event = perf_mmap__read_event()) { ... }
> 
> and so they would be broken due to the event being overwritten if they
> had stored the pointer. Manual inspection and address sanitizer
> testing also shows the event pointer not being stored.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/lib/perf/include/internal/mmap.h |  3 ++-
>  tools/lib/perf/mmap.c                  | 21 ++++++++++++++++++---
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/lib/perf/include/internal/mmap.h b/tools/lib/perf/include/internal/mmap.h
> index 5a062af8e9d8..5f08cab61ece 100644
> --- a/tools/lib/perf/include/internal/mmap.h
> +++ b/tools/lib/perf/include/internal/mmap.h
> @@ -33,7 +33,8 @@ struct perf_mmap {
>  	bool			 overwrite;
>  	u64			 flush;
>  	libperf_unmap_cb_t	 unmap_cb;
> -	char			 event_copy[PERF_SAMPLE_MAX_SIZE] __aligned(8);
> +	void			*event_copy;
> +	size_t			 event_copy_sz;
>  	struct perf_mmap	*next;
>  };
>  
> diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
> index 2184814b37dd..c829db7bf1fa 100644
> --- a/tools/lib/perf/mmap.c
> +++ b/tools/lib/perf/mmap.c
> @@ -19,6 +19,7 @@
>  void perf_mmap__init(struct perf_mmap *map, struct perf_mmap *prev,
>  		     bool overwrite, libperf_unmap_cb_t unmap_cb)
>  {
> +	/* Assume fields were zero initialized. */
>  	map->fd = -1;
>  	map->overwrite = overwrite;
>  	map->unmap_cb  = unmap_cb;
> @@ -51,13 +52,19 @@ int perf_mmap__mmap(struct perf_mmap *map, struct perf_mmap_param *mp,
>  
>  void perf_mmap__munmap(struct perf_mmap *map)
>  {
> -	if (map && map->base != NULL) {
> +	if (!map)
> +		return;
> +
> +	free(map->event_copy);
> +	map->event_copy = NULL;

I´m converting this to:

	zfree(&map->event_copy);

Hopefully we'll find some tool to flag these before submitting patches,
does clang-tidy do these kinds of things?

Also, applied:

 b4 am -P2,4-7 -ctsl --cc-trailers 20231127220902.1315692-1-irogers@google.com

The ones that Namhyung acked and that applied cleanly in order.

The first one, as Namhyung noted, is already merged.

- Arnaldo
Re: [PATCH v5 02/50] libperf: Lazily allocate/size mmap event copy
Posted by Arnaldo Carvalho de Melo 2 years ago
Em Thu, Nov 30, 2023 at 10:15:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Nov 27, 2023 at 02:08:14PM -0800, Ian Rogers escreveu:
> >  void perf_mmap__munmap(struct perf_mmap *map)
> >  {
> > -	if (map && map->base != NULL) {
> > +	if (!map)
> > +		return;
> > +
> > +	free(map->event_copy);
> > +	map->event_copy = NULL;
> 
> I´m converting this to:
> 
> 	zfree(&map->event_copy);
> 
> Hopefully we'll find some tool to flag these before submitting patches,
> does clang-tidy do these kinds of things?
> 
> Also, applied:
> 
>  b4 am -P2,4-7 -ctsl --cc-trailers 20231127220902.1315692-1-irogers@google.com
> 
> The ones that Namhyung acked and that applied cleanly in order.
> 
> The first one, as Namhyung noted, is already merged.

I'll fix this later:

[perfbuilder@five ~]$ export BUILD_TARBALL=http://192.168.86.5/perf/perf-6.6.0-rc1.tar.xz
[perfbuilder@five ~]$ time dm
   1    16.97 almalinux:8                   : FAIL gcc version 8.5.0 20210514 (Red Hat 8.5.0-20) (GCC)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
     static inline int io__get_hex(struct io *io, __u64 *hex)
                                                  ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
     static inline int io__get_dec(struct io *io, __u64 *dec)
                                                  ^~~~~
      INSTALL libbpf_headers
      INSTALL libperf_headers
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      INSTALL /tmp/build/perf/libperf/include/perf/event.h
      CC      /tmp/build/perf/libperf/mmap.o
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf_legacy.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_helpers.h
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
     static inline int io__get_hex(struct io *io, __u64 *hex)
                                                  ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
     static inline int io__get_dec(struct io *io, __u64 *dec)
                                                  ^~~~~
      INSTALL /tmp/build/perf/libperf/include/perf/mmap.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_tracing.h
      CC      /tmp/build/perf/libperf/zalloc.o
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_endian.h
   2    17.28 almalinux:9                   : FAIL gcc version 11.4.1 20230605 (Red Hat 11.4.1-2) (GCC)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
      INSTALL libperf_headers
      INSTALL libbpf_headers
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      INSTALL /tmp/build/perf/libperf/include/perf/evlist.h
      INSTALL /tmp/build/perf/libperf/include/perf/evsel.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf_legacy.h
      INSTALL /tmp/build/perf/libperf/include/perf/event.h
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_helpers.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_tracing.h
      INSTALL /tmp/build/perf/libperf/include/perf/mmap.h
      CC      /tmp/build/perf/libperf/evlist.o
   3    20.71 alpine:3.15                   : FAIL gcc version 10.3.1 20211027 (Alpine 10.3.1_git20211027)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      CC      /tmp/build/perf/libperf/xyarray.o
      CC      /tmp/build/perf/libperf/lib.o
      LINK    /tmp/build/perf/dlfilters/dlfilter-test-api-v0.so
      LINK    /tmp/build/perf/dlfilters/dlfilter-test-api-v2.so
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[5]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:106: /tmp/build/perf/libapi/fs/fs.o] Error 1
    make[5]: *** Waiting for unfinished jobs....
      LD      /tmp/build/perf/libapi/fd/libapi-in.o
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
   4    20.52 alpine:3.16                   : FAIL gcc version 11.2.1 20220219 (Alpine 11.2.1_git20220219)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      LINK    /tmp/build/perf/dlfilters/dlfilter-test-api-v2.so
      CC      /tmp/build/perf/libperf/xyarray.o
      INSTALL libperf_headers
      CC      /tmp/build/perf/libperf/lib.o
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[5]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:106: /tmp/build/perf/libapi/fs/fs.o] Error 1
    make[5]: *** Waiting for unfinished jobs....
      LD      /tmp/build/perf/libapi/fd/libapi-in.o
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
   5    19.77 alpine:3.17                   : FAIL gcc version 12.2.1 20220924 (Alpine 12.2.1_git20220924-r4)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
      INSTALL libperf_headers
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      INSTALL /tmp/build/perf/libperf/include/internal/threadmap.h
      CC      /tmp/build/perf/libperf/xyarray.o
      CC      /tmp/build/perf/libperf/lib.o
      LINK    /tmp/build/perf/dlfilters/dlfilter-test-api-v0.so
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
      LINK    /tmp/build/perf/dlfilters/dlfilter-test-api-v2.so
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf_version.h
      INSTALL /tmp/build/perf/libperf/include/internal/xyarray.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/usdt.bpf.h
   6    17.79 alpine:3.18                   : FAIL gcc version 12.2.1 20220924 (Alpine 12.2.1_git20220924-r10)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_core_read.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/skel_internal.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf_version.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/usdt.bpf.h
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[5]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:106: /tmp/build/perf/libapi/fs/fs.o] Error 1
    make[5]: *** Waiting for unfinished jobs....
      LD      /tmp/build/perf/libapi/fd/libapi-in.o
      LD      /tmp/build/perf/libsymbol/libsymbol-in.o
   7    19.74 alpine:edge                   : FAIL gcc version 13.1.1 20230722 (Alpine 13.1.1_git20230722)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      CC      /tmp/build/perf/libperf/zalloc.o
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf.h
      CC      /tmp/build/perf/libperf/xyarray.o
      CC      /tmp/build/perf/libperf/lib.o
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
      LINK    /tmp/build/perf/dlfilters/dlfilter-show-cycles.so
      LINK    /tmp/build/perf/dlfilters/dlfilter-test-api-v0.so
      LINK    /tmp/build/perf/dlfilters/dlfilter-test-api-v2.so
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf.h
   8    11.61 amazonlinux:2                 : FAIL gcc version 7.3.1 20180712 (Red Hat 7.3.1-17) (GCC)
    In file included from fs/fs.c:19:0:
    fs/../io.h:94:46: error: unknown type name '__u64'; did you mean '_LP64'?
     static inline int io__get_hex(struct io *io, __u64 *hex)
                                                  ^~~~~
                                                  _LP64
    fs/../io.h:123:46: error: unknown type name '__u64'; did you mean '_LP64'?
     static inline int io__get_dec(struct io *io, __u64 *dec)
                                                  ^~~~~
                                                  _LP64
      INSTALL libapi_headers
      INSTALL libsymbol_headers
    make[4]: *** [fs] Error 2
      CC      /tmp/build/perf/libsubcmd/subcmd-config.o
      GEN     /tmp/build/perf/libbpf/bpf_helper_defs.h
      INSTALL /tmp/build/perf/libperf/include/perf/cpumap.h
      CC      /tmp/build/perf/libperf/cpumap.o
    In file included from fs/fs.c:19:0:
    fs/../io.h:94:46: error: unknown type name '__u64'; did you mean '_LP64'?
     static inline int io__get_hex(struct io *io, __u64 *hex)
                                                  ^~~~~
                                                  _LP64
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf.h
    fs/../io.h:123:46: error: unknown type name '__u64'; did you mean '_LP64'?
     static inline int io__get_dec(struct io *io, __u64 *dec)
                                                  ^~~~~
                                                  _LP64
      INSTALL /tmp/build/perf/libperf/include/perf/threadmap.h
   9    12.89 amazonlinux:2023              : FAIL gcc version 11.4.1 20230605 (Red Hat 11.4.1-2) (GCC)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_core_read.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf_version.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/skel_internal.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/usdt.bpf.h
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[5]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:106: /tmp/build/perf/libapi/fs/fs.o] Error 1
    make[5]: *** Waiting for unfinished jobs....
      LD      /tmp/build/perf/libapi/fd/libapi-in.o
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
  10    12.84 amazonlinux:devel             : FAIL gcc version 11.3.1 20221121 (Red Hat 11.3.1-4) (GCC)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      CC      /tmp/build/perf/libperf/lib.o
      INSTALL /tmp/build/perf/libbpf/include/bpf/skel_internal.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf_version.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/usdt.bpf.h
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
    make[5]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:106: /tmp/build/perf/libapi/fs/fs.o] Error 1
    make[5]: *** Waiting for unfinished jobs....
      LD      /tmp/build/perf/libapi/fd/libapi-in.o
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
  11   147.52 archlinux:base                : Ok   gcc (GCC) 13.2.1 20230801 , clang version 16.0.6 flex 2.6.4
  12    13.41 centos:stream                 : FAIL gcc version 8.5.0 20210514 (Red Hat 8.5.0-21) (GCC)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
     static inline int io__get_hex(struct io *io, __u64 *hex)
                                                  ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
     static inline int io__get_dec(struct io *io, __u64 *dec)
                                                  ^~~~~
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_tracing.h
      CC      /tmp/build/perf/libperf/xyarray.o
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_endian.h
      CC      /tmp/build/perf/libperf/lib.o
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
     static inline int io__get_hex(struct io *io, __u64 *hex)
                                                  ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
     static inline int io__get_dec(struct io *io, __u64 *dec)
                                                  ^~~~~
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_core_read.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/skel_internal.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf_version.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/usdt.bpf.h
  13   108.26 clearlinux:latest             : Ok   gcc (Clear Linux OS for Intel Architecture) 13.2.1 20231115 releases/gcc-13.2.0-463-g5d6f62c9b2 , clang version 16.0.6 flex 2.6.4
  14    10.87 debian:10                     : FAIL gcc version 8.3.0 (Debian 8.3.0-6)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
     static inline int io__get_hex(struct io *io, __u64 *hex)
                                                  ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
     static inline int io__get_dec(struct io *io, __u64 *dec)
                                                  ^~~~~
      INSTALL libbpf_headers
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_tracing.h
      CC      /tmp/build/perf/libapi/cpu.o
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_endian.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/bpf_core_read.h
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
     static inline int io__get_hex(struct io *io, __u64 *hex)
                                                  ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
     static inline int io__get_dec(struct io *io, __u64 *dec)
                                                  ^~~~~
      INSTALL /tmp/build/perf/libbpf/include/bpf/skel_internal.h
      INSTALL /tmp/build/perf/libbpf/include/bpf/libbpf_version.h
      CC      /tmp/build/perf/libapi/debug.o
      CC      /tmp/build/perf/libapi/str_error_r.o
  15    12.70 debian:11                     : FAIL gcc version 10.2.1 20210110 (Debian 10.2.1-6)
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
      INSTALL libperf_headers
    make[4]: *** [/git/perf-6.6.0-rc1/tools/build/Makefile.build:158: fs] Error 2
      INSTALL /tmp/build/perf/libbpf/include/bpf/usdt.bpf.h
      INSTALL /tmp/build/perf/libperf/include/internal/cpumap.h
      INSTALL /tmp/build/perf/libperf/include/internal/evlist.h
      INSTALL /tmp/build/perf/libperf/include/internal/evsel.h
    In file included from fs/fs.c:19:
    fs/../io.h:94:46: error: unknown type name '__u64'
       94 | static inline int io__get_hex(struct io *io, __u64 *hex)
          |                                              ^~~~~
    fs/../io.h:123:46: error: unknown type name '__u64'
      123 | static inline int io__get_dec(struct io *io, __u64 *dec)
          |                                              ^~~~~
      CC      /tmp/build/perf/libperf/zalloc.o
      INSTALL /tmp/build/perf/libperf/include/internal/lib.h
      INSTALL /tmp/build/perf/libperf/include/internal/mmap.h
      CC      /tmp/build/perf/libperf/xyarray.o
  16    85.50 debian:12                     : Ok   gcc (Debian 12.2.0-14) 12.2.0 , Debian clang version 14.0.6 flex 2.6.4
  17: debian:experimental
Re: [PATCH v5 02/50] libperf: Lazily allocate/size mmap event copy
Posted by Arnaldo Carvalho de Melo 2 years ago
Em Thu, Nov 30, 2023 at 11:19:51AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Nov 30, 2023 at 10:15:43AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Nov 27, 2023 at 02:08:14PM -0800, Ian Rogers escreveu:
> > >  void perf_mmap__munmap(struct perf_mmap *map)
> > >  {
> > > -	if (map && map->base != NULL) {
> > > +	if (!map)
> > > +		return;
> > > +
> > > +	free(map->event_copy);
> > > +	map->event_copy = NULL;
> > 
> > I´m converting this to:
> > 
> > 	zfree(&map->event_copy);
> > 
> > Hopefully we'll find some tool to flag these before submitting patches,
> > does clang-tidy do these kinds of things?
> > 
> > Also, applied:
> > 
> >  b4 am -P2,4-7 -ctsl --cc-trailers 20231127220902.1315692-1-irogers@google.com
> > 
> > The ones that Namhyung acked and that applied cleanly in order.
> > 
> > The first one, as Namhyung noted, is already merged.
> 
> I'll fix this later:
> 
> [perfbuilder@five ~]$ export BUILD_TARBALL=http://192.168.86.5/perf/perf-6.6.0-rc1.tar.xz
> [perfbuilder@five ~]$ time dm
>    1    16.97 almalinux:8                   : FAIL gcc version 8.5.0 20210514 (Red Hat 8.5.0-20) (GCC)
>     In file included from fs/fs.c:19:
>     fs/../io.h:94:46: error: unknown type name '__u64'
>      static inline int io__get_hex(struct io *io, __u64 *hex)

Added the following patch to just before your patches.

https://lore.kernel.org/all/ZWjDPL+IzPPsuC3X@kernel.org/T/#u

tldr; missing linux/types.h for __u64 in api/io.h.

- Arnaldo
Re: [PATCH v5 02/50] libperf: Lazily allocate/size mmap event copy
Posted by Namhyung Kim 2 years ago
On Mon, Nov 27, 2023 at 2:09 PM Ian Rogers <irogers@google.com> wrote:
>
> The event copy in the mmap is used to have storage to read an
> event. Not all users of mmaps read the events, such as perf
> record. The amount of buffer was also statically set to
> PERF_SAMPLE_MAX_SIZE rather than the amount necessary from the
> header's event size. Switch to a model where the event_copy is
> reallocated if too small to the event's size. This adds the potential
> for the event to move, so if a copy of the event pointer were stored
> it could be broken. All the current users do:
>
>   while(event = perf_mmap__read_event()) { ... }
>
> and so they would be broken due to the event being overwritten if they
> had stored the pointer. Manual inspection and address sanitizer
> testing also shows the event pointer not being stored.
>
> Signed-off-by: Ian Rogers <irogers@google.com>

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

Thanks,
Namhyung


> ---
>  tools/lib/perf/include/internal/mmap.h |  3 ++-
>  tools/lib/perf/mmap.c                  | 21 ++++++++++++++++++---
>  2 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/perf/include/internal/mmap.h b/tools/lib/perf/include/internal/mmap.h
> index 5a062af8e9d8..5f08cab61ece 100644
> --- a/tools/lib/perf/include/internal/mmap.h
> +++ b/tools/lib/perf/include/internal/mmap.h
> @@ -33,7 +33,8 @@ struct perf_mmap {
>         bool                     overwrite;
>         u64                      flush;
>         libperf_unmap_cb_t       unmap_cb;
> -       char                     event_copy[PERF_SAMPLE_MAX_SIZE] __aligned(8);
> +       void                    *event_copy;
> +       size_t                   event_copy_sz;
>         struct perf_mmap        *next;
>  };
>
> diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
> index 2184814b37dd..c829db7bf1fa 100644
> --- a/tools/lib/perf/mmap.c
> +++ b/tools/lib/perf/mmap.c
> @@ -19,6 +19,7 @@
>  void perf_mmap__init(struct perf_mmap *map, struct perf_mmap *prev,
>                      bool overwrite, libperf_unmap_cb_t unmap_cb)
>  {
> +       /* Assume fields were zero initialized. */
>         map->fd = -1;
>         map->overwrite = overwrite;
>         map->unmap_cb  = unmap_cb;
> @@ -51,13 +52,19 @@ int perf_mmap__mmap(struct perf_mmap *map, struct perf_mmap_param *mp,
>
>  void perf_mmap__munmap(struct perf_mmap *map)
>  {
> -       if (map && map->base != NULL) {
> +       if (!map)
> +               return;
> +
> +       free(map->event_copy);
> +       map->event_copy = NULL;
> +       map->event_copy_sz = 0;
> +       if (map->base) {
>                 munmap(map->base, perf_mmap__mmap_len(map));
>                 map->base = NULL;
>                 map->fd = -1;
>                 refcount_set(&map->refcnt, 0);
>         }
> -       if (map && map->unmap_cb)
> +       if (map->unmap_cb)
>                 map->unmap_cb(map);
>  }
>
> @@ -223,9 +230,17 @@ static union perf_event *perf_mmap__read(struct perf_mmap *map,
>                  */
>                 if ((*startp & map->mask) + size != ((*startp + size) & map->mask)) {
>                         unsigned int offset = *startp;
> -                       unsigned int len = min(sizeof(*event), size), cpy;
> +                       unsigned int len = size, cpy;
>                         void *dst = map->event_copy;
>
> +                       if (size > map->event_copy_sz) {
> +                               dst = realloc(map->event_copy, size);
> +                               if (!dst)
> +                                       return NULL;
> +                               map->event_copy = dst;
> +                               map->event_copy_sz = size;
> +                       }
> +
>                         do {
>                                 cpy = min(map->mask + 1 - (offset & map->mask), len);
>                                 memcpy(dst, &data[offset & map->mask], cpy);
> --
> 2.43.0.rc1.413.gea7ed67945-goog
>