[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <65959200a747b_2384720814@john.notmuch>
Date: Wed, 03 Jan 2024 08:57:36 -0800
From: John Fastabend <john.fastabend@...il.com>
To: Barret Rhoden <brho@...gle.com>,
Andrii Nakryiko <andrii@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>
Cc: mattbobrowski@...gle.com,
bpf@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: RE: [PATCH bpf-next 1/2] libbpf: add helpers for mmapping maps
Barret Rhoden wrote:
> bpf_map__mmap_size() was internal to bpftool. Use that to make wrappers
> for mmap and munmap.
>
> Signed-off-by: Barret Rhoden <brho@...gle.com>
> ---
> tools/bpf/bpftool/gen.c | 16 +++-------------
> tools/lib/bpf/libbpf.c | 23 +++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 6 ++++++
> tools/lib/bpf/libbpf.map | 4 ++++
> 4 files changed, 36 insertions(+), 13 deletions(-)
>
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index ee3ce2b8000d..a328e960c141 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -453,16 +453,6 @@ static void print_hex(const char *data, int data_sz)
> }
> }
>
> -static size_t bpf_map_mmap_sz(const struct bpf_map *map)
> -{
> - long page_sz = sysconf(_SC_PAGE_SIZE);
> - size_t map_sz;
> -
> - map_sz = (size_t)roundup(bpf_map__value_size(map), 8) * bpf_map__max_entries(map);
> - map_sz = roundup(map_sz, page_sz);
> - return map_sz;
> -}
> -
> /* Emit type size asserts for all top-level fields in memory-mapped internal maps. */
> static void codegen_asserts(struct bpf_object *obj, const char *obj_name)
> {
> @@ -641,7 +631,7 @@ static void codegen_destroy(struct bpf_object *obj, const char *obj_name)
> if (bpf_map__is_internal(map) &&
> (bpf_map__map_flags(map) & BPF_F_MMAPABLE))
> printf("\tskel_free_map_data(skel->%1$s, skel->maps.%1$s.initial_value, %2$zd);\n",
> - ident, bpf_map_mmap_sz(map));
> + ident, bpf_map__mmap_size(map));
> codegen("\
> \n\
> skel_closenz(skel->maps.%1$s.map_fd); \n\
> @@ -723,7 +713,7 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
> goto cleanup; \n\
> skel->maps.%1$s.initial_value = (__u64) (long) skel->%1$s;\n\
> } \n\
> - ", ident, bpf_map_mmap_sz(map));
> + ", ident, bpf_map__mmap_size(map));
> }
> codegen("\
> \n\
> @@ -780,7 +770,7 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
> if (!skel->%1$s) \n\
> return -ENOMEM; \n\
> ",
> - ident, bpf_map_mmap_sz(map), mmap_flags);
> + ident, bpf_map__mmap_size(map), mmap_flags);
> }
> codegen("\
> \n\
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ebcfb2147fbd..171a977cb5fd 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9830,6 +9830,29 @@ void *bpf_map__initial_value(struct bpf_map *map, size_t *psize)
> return map->mmaped;
> }
It seems libbpf.c has its own bpf_map_mmap_sz() as well.
static size_t bpf_map_mmap_sz(unsigned int value_sz, unsigned int max_entries)
{
const long page_sz = sysconf(_SC_PAGE_SIZE);
size_t map_sz;
map_sz = (size_t)roundup(value_sz, 8) * max_entries;
map_sz = roundup(map_sz, page_sz);
return map_sz;
}
Can we consolidate these a bit. Seems we don't want/need to
have both bpf_map__mmap_size and bpf_map_mmap_sz() floating
around.
Should bpf_map__mmap_size just calls bpf_map_mmap_sz with
the correct sz and max_entries?
>
> +size_t bpf_map__mmap_size(const struct bpf_map *map)
> +{
> + long page_sz = sysconf(_SC_PAGE_SIZE);
> + size_t map_sz;
> +
> + map_sz = (size_t)roundup(bpf_map__value_size(map), 8) *
> + bpf_map__max_entries(map);
> + map_sz = roundup(map_sz, page_sz);
> + return map_sz;
> +}
> +
> +void *bpf_map__mmap(const struct bpf_map *map)
> +{
> + return mmap(NULL, bpf_map__mmap_size(map),
> + PROT_READ | PROT_WRITE, MAP_SHARED,
> + bpf_map__fd(map), 0);
> +}
> +
> +int bpf_map__munmap(const struct bpf_map *map, void *addr)
> +{
> + return munmap(addr, bpf_map__mmap_size(map));
> +}
> +
> bool bpf_map__is_internal(const struct bpf_map *map)
> {
> return map->libbpf_type != LIBBPF_MAP_UNSPEC;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 6cd9c501624f..148f4c783ca7 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -996,6 +996,12 @@ LIBBPF_API int bpf_map__set_map_extra(struct bpf_map *map, __u64 map_extra);
> LIBBPF_API int bpf_map__set_initial_value(struct bpf_map *map,
> const void *data, size_t size);
> LIBBPF_API void *bpf_map__initial_value(struct bpf_map *map, size_t *psize);
> +/* get the mmappable size of the map */
> +LIBBPF_API size_t bpf_map__mmap_size(const struct bpf_map *map);
> +/* mmap the map */
> +LIBBPF_API void *bpf_map__mmap(const struct bpf_map *map);
> +/* munmap the map at addr */
> +LIBBPF_API int bpf_map__munmap(const struct bpf_map *map, void *addr);
>
> /**
> * @brief **bpf_map__is_internal()** tells the caller whether or not the
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 91c5aef7dae7..9e44de4fbf39 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -411,4 +411,8 @@ LIBBPF_1.3.0 {
> } LIBBPF_1.2.0;
>
> LIBBPF_1.4.0 {
> + global:
> + bpf_map__mmap_size;
> + bpf_map__mmap;
> + bpf_map__munmap;
> } LIBBPF_1.3.0;
> --
> 2.43.0.472.g3155946c3a-goog
>
>
Powered by blists - more mailing lists