From: Leon Hwang <leon.hwang@linux.dev>
To: Emil Tsalapatis <emil@etsalapatis.com>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Martin KaFai Lau <martin.lau@linux.dev>,
Eduard Zingerman <eddyz87@gmail.com>,
Kumar Kartikeya Dwivedi <memxor@gmail.com>,
Song Liu <song@kernel.org>,
Yonghong Song <yonghong.song@linux.dev>,
Jiri Olsa <jolsa@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
Quentin Monnet <qmo@kernel.org>, Shuah Khan <shuah@kernel.org>,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
kernel-patches-bot@fb.com
Subject: Re: [PATCH bpf-next v10 5/9] bpftool: Generate skeleton for global percpu data
Date: Mon, 20 Jul 2026 13:00:57 +0800 [thread overview]
Message-ID: <149c6e74-fe9e-4b7e-a782-015cc44bec18@linux.dev> (raw)
In-Reply-To: <DK16GZ1CVRYZ.1ZKXOISH3846Z@etsalapatis.com>
On 18/7/26 05:52, Emil Tsalapatis wrote:
> On Wed Jul 15, 2026 at 11:32 AM EDT, Leon Hwang wrote:
[...]
>>
>> static bool get_datasec_ident(const char *sec_name, char *buf, size_t buf_sz)
>> {
>> - static const char *pfxs[] = { ".data", ".rodata", ".bss", ".kconfig" };
>> + static const char *pfxs[] = { ".data", ".rodata", ".bss", ".percpu", ".kconfig" };
>> int i, n;
>>
>> /* recognize hard coded LLVM section name */
>> @@ -254,7 +260,7 @@ static const struct btf_type *find_type_for_map(struct btf *btf, const char *map
>> return NULL;
>> }
>>
>> -static bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
>> +static bool is_skel_data(const struct bpf_map *map, char *buf, size_t sz)
>
> Here we change the function to take PERCPU_ARRAY into account, but then
> immediately turn around and add checks of the form is_skel_data && type
> != BPF_MAP_TYPE_PERCPU_ARRAY. If we keep both the existing mmapable_map
> and add a separate is_skel_data map (possibly using is_mappable_map),
> we can remove those additional checks.
Agreed.
I'd like to keep this new is_skel_data(), then update the
is_mmapable_map() to:
static bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
{
return is_skel_data(map, buf, sz) && bpf_map__type(map) !=
BPF_MAP_TYPE_PERCPU_ARRAY;
}
Yep, it is not a good choice to use '!=' here. But it can simplify the code.
>
>> {
>> size_t tmp_sz;
>>
>> @@ -263,13 +269,19 @@ static bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
>> return true;
>> }
>>
>> - if (!bpf_map__is_internal(map) || !(bpf_map__map_flags(map) & BPF_F_MMAPABLE))
>> + if (!bpf_map__is_internal(map))
>> return false;
>>
>> if (!get_map_ident(map, buf, sz))
>> return false;
>>
>> - return true;
>> + if (bpf_map__map_flags(map) & BPF_F_MMAPABLE)
>> + return true;
>> +
>> + if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>> + return true;
>> +
>> + return false;
>> }
>>
>> static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
>> @@ -286,8 +298,11 @@ static int codegen_datasecs(struct bpf_object *obj, const char *obj_name)
>> return -errno;
>>
>> bpf_object__for_each_map(map, obj) {
>> - /* only generate definitions for memory-mapped internal maps */
>> - if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
>> + /*
>> + * Only generate definitions for internal maps that have
>> + * mmapped data.
>> + */
>> + if (!is_skel_data(map, map_ident, sizeof(map_ident)))
>> continue;
>>
>> sec = find_type_for_map(btf, map_ident);
>> @@ -339,8 +354,14 @@ static int codegen_subskel_datasecs(struct bpf_object *obj, const char *obj_name
>> return -errno;
>>
>> bpf_object__for_each_map(map, obj) {
>> - /* only generate definitions for memory-mapped internal maps */
>> - if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
>> + /*
>> + * Only generate definitions for internal maps that have
>> + * mmapped data.
>> + */
>> + if (!is_skel_data(map, map_ident, sizeof(map_ident)))
>> + continue;
>> +
>> + if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>> continue;
>
> Case in point, the old is_mmapable_map would work fine here.
Yes.
>
>>
>> sec = find_type_for_map(btf, map_ident);
>> @@ -493,7 +514,10 @@ static size_t bpf_map_mmap_sz(const struct bpf_map *map)
>> return map_sz;
>> }
>>
>> -/* Emit type size asserts for all top-level fields in memory-mapped internal maps. */
>> +/*
>> + * Emit type size asserts for all top-level fields in internal maps that
>> + * have mmaped data.
>
> I do not think the new comment makes it any clearer, the difference is
> "memory-mapped internal maps" and "maps that are memory-mapped initially
> but actually inaccessible after loading because there's no way to
> represent them as a mapping of the skeleton" AFAICT which is not easy
> to infer currently.
Hmm, it is hard to explain the new case that the mmaped data is visible
in user-space, but is invisible in kernel-space.
Will drop this change.
>
>> + */
>> static void codegen_asserts(struct bpf_object *obj, const char *obj_name)
>> {
>> struct btf *btf = bpf_object__btf(obj);
>> @@ -517,7 +541,7 @@ static void codegen_asserts(struct bpf_object *obj, const char *obj_name)
>> ", obj_name);
>>
>> bpf_object__for_each_map(map, obj) {
>> - if (!is_mmapable_map(map, map_ident, sizeof(map_ident)))
>> + if (!is_skel_data(map, map_ident, sizeof(map_ident)))
>> continue;
>>
>> sec = find_type_for_map(btf, map_ident);
>> @@ -669,7 +693,8 @@ static void codegen_destroy(struct bpf_object *obj, const char *obj_name)
>> if (!get_map_ident(map, ident, sizeof(ident)))
>> continue;
>> if (bpf_map__is_internal(map) &&
>> - (bpf_map__map_flags(map) & BPF_F_MMAPABLE))
>> + ((bpf_map__map_flags(map) & BPF_F_MMAPABLE) ||
>> + bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY))
>
> I think this gives us a hint on how we could structure is_skel_data
> above, we now have a special calss that is "mappable maps + PERCPU_ARRAY
> whose initial representation in userspace is mappable, but the loaded
> map isn't".
Will use is_skel_data() here.
Thanks,
Leon
>
>> printf("\tskel_free_map_data(skel->%1$s, skel->maps.%1$s.initial_value, %2$zu);\n",
>> ident, bpf_map_mmap_sz(map));
>> codegen("\
>> @@ -741,7 +766,7 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
>> const void *mmap_data = NULL;
>> size_t mmap_size = 0;
>>
>> - if (!is_mmapable_map(map, ident, sizeof(ident)))
>> + if (!is_skel_data(map, ident, sizeof(ident)))
>> continue;
>>
>> codegen("\
>> @@ -849,8 +874,22 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
>> bpf_object__for_each_map(map, obj) {
>> const char *mmap_flags;
>>
>> - if (!is_mmapable_map(map, ident, sizeof(ident)))
>> + if (!is_skel_data(map, ident, sizeof(ident)))
>> + continue;
>> +
>> + if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY) {
>> + codegen("\
>> + \n\
>> + err = skel_protect_map_data(skel->%1$s, &skel->maps.%1$s.initial_value, %2$zd);\n\
>> + if (err) \n\
>> + return err; \n\
>> + #ifdef __KERNEL__ \n\
>> + skel->%1$s = NULL; \n\
>> + #endif \n\
>> + ",
>> + ident, bpf_map_mmap_sz(map));
>> continue;
>> + }
>>
>> if (bpf_map__map_flags(map) & BPF_F_RDONLY_PROG)
>> mmap_flags = "PROT_READ";
>> @@ -955,8 +994,7 @@ codegen_maps_skeleton(struct bpf_object *obj, size_t map_cnt, bool mmaped, bool
>> map->map = &obj->maps.%s; \n\
>> ",
>> i, bpf_map__name(map), ident);
>> - /* memory-mapped internal maps */
>> - if (mmaped && is_mmapable_map(map, ident, sizeof(ident))) {
>> + if (mmaped && is_skel_data(map, ident, sizeof(ident))) {
>> printf("\tmap->mmaped = (void **)&obj->%s;\n", ident);
>> }
>>
>> @@ -1740,7 +1778,9 @@ static int do_subskeleton(int argc, char **argv)
>> /* Also count all maps that have a name */
>> map_cnt++;
>>
>> - if (!is_mmapable_map(map, ident, sizeof(ident)))
>> + if (!is_skel_data(map, ident, sizeof(ident)))
>> + continue;
>> + if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>> continue;
>
> Another instance of what I was talking about.
>
>>
>> map_type_id = bpf_map__btf_value_type_id(map);
>> @@ -1863,7 +1903,9 @@ static int do_subskeleton(int argc, char **argv)
>>
>> /* walk through each symbol and emit the runtime representation */
>> bpf_object__for_each_map(map, obj) {
>> - if (!is_mmapable_map(map, ident, sizeof(ident)))
>> + if (!is_skel_data(map, ident, sizeof(ident)))
>> + continue;
>> + if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
>> continue;
>>
>> map_type_id = bpf_map__btf_value_type_id(map);
>> diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h
>> index 53fee53d36d5..1f3f332dffbe 100644
>> --- a/tools/lib/bpf/skel_internal.h
>> +++ b/tools/lib/bpf/skel_internal.h
>> @@ -131,8 +131,10 @@ static inline void skel_free_map_data(void *p, __u64 addr, size_t sz)
>> {
>> if (addr != ~0ULL)
>> kvfree(p);
>> - /* When addr == ~0ULL the 'p' points to
>> - * ((struct bpf_array *)map)->value. See skel_finalize_map_data.
>> + /*
>> + * When addr == ~0ULL the init buffer has already been released.
>> + * For skel_finalize_map_data(), 'p' points to
>> + * ((struct bpf_array *)map)->value.
>> */
>> }
>>
>> @@ -170,6 +172,15 @@ static inline void *skel_finalize_map_data(__u64 *init_val, size_t mmap_sz, int
>> return addr;
>> }
>>
>> +static inline int skel_protect_map_data(void *p, __u64 *init_val, size_t sz)
>> +{
>> + (void)sz;
>> +
>> + kvfree(p);
>> + *init_val = ~0ULL;
>> + return 0;
>> +}
>> +
>> #else
>>
>> static inline void *skel_alloc(size_t size)
>> @@ -208,6 +219,15 @@ static inline void *skel_finalize_map_data(__u64 *init_val, size_t mmap_sz, int
>> return NULL;
>> return addr;
>> }
>> +
>> +static inline int skel_protect_map_data(void *p, __u64 *init_val, size_t sz)
>> +{
>> + (void)init_val;
>> +
>> + if (mprotect(p, sz, PROT_READ))
>> + return -errno;
>> + return 0;
>> +}
>> #endif
>>
>> static inline int skel_closenz(int fd)
>
next prev parent reply other threads:[~2026-07-20 5:01 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-15 15:32 [PATCH bpf-next v10 0/9] bpf: Introduce global percpu data Leon Hwang
2026-07-15 15:32 ` [PATCH bpf-next v10 1/9] bpf: Drop duplicate blank lines in verifier Leon Hwang
2026-07-17 5:08 ` Emil Tsalapatis
2026-07-17 5:28 ` Leon Hwang
2026-07-15 15:32 ` [PATCH bpf-next v10 2/9] bpf: Introduce global percpu data Leon Hwang
2026-07-17 6:25 ` Emil Tsalapatis
2026-07-20 4:59 ` Leon Hwang
2026-07-20 22:55 ` Emil Tsalapatis
2026-07-15 15:32 ` [PATCH bpf-next v10 3/9] libbpf: Probe percpu data feature Leon Hwang
2026-07-17 21:52 ` Emil Tsalapatis
2026-07-15 15:32 ` [PATCH bpf-next v10 4/9] libbpf: Add support for global percpu data Leon Hwang
2026-07-15 16:00 ` sashiko-bot
2026-07-16 5:21 ` Leon Hwang
2026-07-17 21:39 ` Emil Tsalapatis
2026-07-20 4:59 ` Leon Hwang
2026-07-15 15:32 ` [PATCH bpf-next v10 5/9] bpftool: Generate skeleton " Leon Hwang
2026-07-17 21:52 ` Emil Tsalapatis
2026-07-20 5:00 ` Leon Hwang [this message]
2026-07-15 15:32 ` [PATCH bpf-next v10 6/9] selftests/bpf: Add tests to verify " Leon Hwang
2026-07-15 15:55 ` sashiko-bot
2026-07-16 5:25 ` Leon Hwang
2026-07-15 16:11 ` bot+bpf-ci
2026-07-16 5:30 ` Leon Hwang
2026-07-17 22:47 ` Emil Tsalapatis
2026-07-20 5:02 ` Leon Hwang
2026-07-15 15:32 ` [PATCH bpf-next v10 7/9] selftests/bpf: Test direct reading/writing read-only percpu_array map Leon Hwang
2026-07-17 22:49 ` Emil Tsalapatis
2026-07-20 5:03 ` Leon Hwang
2026-07-15 15:32 ` [PATCH bpf-next v10 8/9] selftests/bpf: Test verifier log for global percpu data Leon Hwang
2026-07-17 22:50 ` Emil Tsalapatis
2026-07-15 15:32 ` [PATCH bpf-next v10 9/9] selftests/bpf: Verify bpf_iter " Leon Hwang
2026-07-17 23:11 ` Emil Tsalapatis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=149c6e74-fe9e-4b7e-a782-015cc44bec18@linux.dev \
--to=leon.hwang@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kernel-patches-bot@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=qmo@kernel.org \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.