All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Hwang <leon.hwang@linux.dev>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Quentin Monnet <qmo@kernel.org>
Cc: bpf@vger.kernel.org, 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>,
	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 v8 5/9] bpftool: Generate skeleton for global percpu data
Date: Thu, 2 Jul 2026 14:24:28 +0800	[thread overview]
Message-ID: <ad19d283-aaab-41f1-b1a5-73de551b03a9@linux.dev> (raw)
In-Reply-To: <CAEf4BzbfA2nySpf3Oh6KVBRjDKMOOYN-NC279Sj73KwH32cGGg@mail.gmail.com>

On 2/7/26 03:32, Andrii Nakryiko wrote:
> On Wed, Jul 1, 2026 at 9:49 AM Quentin Monnet <qmo@kernel.org> wrote:
>>
>> 2026-06-29 23:24 UTC+0800 ~ Leon Hwang <leon.hwang@linux.dev>
[...]
>>> @@ -254,6 +254,20 @@ static const struct btf_type *find_type_for_map(struct btf *btf, const char *map
>>>       return NULL;
>>>  }
>>>
>>> +static bool bpf_map_is_skel_data(const struct bpf_map *map)
>>> +{
>>> +     if (!bpf_map__is_internal(map))
>>> +             return false;
>>> +
>>> +     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 bool is_mmapable_map(const struct bpf_map *map, char *buf, size_t sz)
>>>  {
>>>       size_t tmp_sz;
>>> @@ -263,7 +277,7 @@ 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_skel_data(map))
>>>               return false;
>>>
>>>       if (!get_map_ident(map, buf, sz))
>>
>>
>> Thanks! The bpftool patch looks good, with one reservation: after this
>> patch, I believe "is_mmapable_map(map, ...)" will return true if map is
>> a percpu map, although percpu maps aren't mmap-able, so we should
>> probably update the name of that function to avoid any confusion?
>>
> 
> Great observation, Quentin!
> 
> bpf_map_is_skel_data() I think was supposed to be exactly that generic
> name. But it seems like Leon went half-way through with unification.
> Unless there are some subtle situations where per-cpu array shouldn't
> be handled where is_mmapable_map() is handled, we should rename
> is_mmapable_map() and add BPF_MAP_TYPE_PERCPU_ARRAY check (assuming
> it's internal map, of course) there.
> 
> Leon, can you please check?
> 
Aha, my bad.

Try to rename is_mmapable_map() to bpf_map_is_skel_data(). See below patch.

Thanks,
Leon

---

From 654306cd9091a175883dd61c7b251996c42e7cae Mon Sep 17 00:00:00 2001
From: Leon Hwang <leon.hwang@linux.dev>
Date: Mon, 29 Jun 2026 23:24:02 +0800
Subject: [PATCH bpf-next v9 5/9] bpftool: Generate skeleton for global
percpu
 data

Enhance bpftool to generate skeletons that properly handle global percpu
variables. The generated skeleton now includes a dedicated structure for
percpu data, allowing users to initialize and access percpu variables more
efficiently.

For global percpu variables, the skeleton now includes a nested
structure, e.g.:

struct test_global_percpu_data {
	struct bpf_object_skeleton *skeleton;
	struct bpf_object *obj;
	struct {
		struct bpf_map *percpu;
	} maps;
	// ...
	struct test_global_percpu_data__percpu {
		int data;
		char run;
		struct {
			char set;
			int i;
			int nums[7];
		} struct_data;
		int nums[7];
	} *percpu;

	// ...
};

  * The "struct test_global_percpu_data__percpu *percpu" points to
    initialized data, which is actually "maps.percpu->mmaped".
  * Before loading the skeleton, updating the
    "struct test_global_percpu_data__percpu *percpu" modifies the initial
    value of the corresponding global percpu variables.
  * After loading the skeleton, "maps.percpu->mmaped" has been marked as
    read-only in libbpf. If users want to update the global percpu
    variables, they have to update the "maps.percpu" map instead.
  * For lightweight skeleton, "lskel->percpu" will be protected by
    "mprotect(p, sz, PROT_READ)".
  * For subskeleton, those variables of global percpu data will be
    skipped.

Assisted-by: Codex:gpt-5.5-xhigh
Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 tools/bpf/bpftool/gen.c       | 72 ++++++++++++++++++++++++++---------
 tools/lib/bpf/skel_internal.h | 24 +++++++++++-
 2 files changed, 76 insertions(+), 20 deletions(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 6ae7262ebe0c..798a34366e08 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -92,7 +92,7 @@ static void get_header_guard(char *guard, const char
*obj_name, const char *suff

 static bool get_map_ident(const struct bpf_map *map, char *buf, size_t
buf_sz)
 {
-	static const char *sfxs[] = { ".data", ".rodata", ".bss", ".kconfig" };
+	static const char *sfxs[] = { ".data", ".rodata", ".bss", ".percpu",
".kconfig" };
 	const char *name = bpf_map__name(map);
 	int i, n;

@@ -117,7 +117,7 @@ static bool get_map_ident(const struct bpf_map *map,
char *buf, size_t buf_sz)

 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 +254,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 bpf_map_is_skel_data(const struct bpf_map *map, char *buf,
size_t sz)
 {
 	size_t tmp_sz;

@@ -263,13 +263,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 +292,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 (!bpf_map_is_skel_data(map, map_ident, sizeof(map_ident)))
 			continue;

 		sec = find_type_for_map(btf, map_ident);
@@ -339,8 +348,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 (!bpf_map_is_skel_data(map, map_ident, sizeof(map_ident)))
+			continue;
+
+		if (bpf_map__type(map) == BPF_MAP_TYPE_PERCPU_ARRAY)
 			continue;

 		sec = find_type_for_map(btf, map_ident);
@@ -493,7 +508,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.
+ */
 static void codegen_asserts(struct bpf_object *obj, const char *obj_name)
 {
 	struct btf *btf = bpf_object__btf(obj);
@@ -517,7 +535,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 (!bpf_map_is_skel_data(map, map_ident, sizeof(map_ident)))
 			continue;

 		sec = find_type_for_map(btf, map_ident);
@@ -669,7 +687,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))
 			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 +760,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 (!bpf_map_is_skel_data(map, ident, sizeof(ident)))
 			continue;

 		codegen("\
@@ -847,9 +866,23 @@ 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 (!bpf_map_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";
 		else
@@ -953,8 +986,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 && bpf_map_is_skel_data(map, ident, sizeof(ident))) {
 			printf("\tmap->mmaped = (void **)&obj->%s;\n", ident);
 		}

@@ -1738,7 +1770,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 (!bpf_map_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);
@@ -1861,7 +1895,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 (!bpf_map_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 74503d358bc8..485b0cd17017 100644
--- a/tools/lib/bpf/skel_internal.h
+++ b/tools/lib/bpf/skel_internal.h
@@ -135,8 +135,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.
 	 */
 }

@@ -174,6 +176,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)
@@ -212,6 +223,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)
-- 
2.54.0



  reply	other threads:[~2026-07-02  6:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29 15:23 [PATCH bpf-next v8 0/9] bpf: Introduce global percpu data Leon Hwang
2026-06-29 15:23 ` [PATCH bpf-next v8 1/9] bpf: Drop duplicate blank lines in verifier Leon Hwang
2026-06-29 15:23 ` [PATCH bpf-next v8 2/9] bpf: Introduce global percpu data Leon Hwang
2026-06-29 15:50   ` sashiko-bot
2026-06-30  5:50     ` Leon Hwang
2026-07-01 19:31   ` Andrii Nakryiko
2026-07-02  6:15     ` Leon Hwang
2026-06-29 15:24 ` [PATCH bpf-next v8 3/9] libbpf: Probe percpu data feature Leon Hwang
2026-06-29 15:24 ` [PATCH bpf-next v8 4/9] libbpf: Add support for global percpu data Leon Hwang
2026-06-29 15:40   ` sashiko-bot
2026-06-30  5:54     ` Leon Hwang
2026-07-01 19:31       ` Andrii Nakryiko
2026-07-02  6:23         ` Leon Hwang
2026-07-01 19:32   ` Andrii Nakryiko
2026-07-02  6:16     ` Leon Hwang
2026-06-29 15:24 ` [PATCH bpf-next v8 5/9] bpftool: Generate skeleton " Leon Hwang
2026-07-01 16:49   ` Quentin Monnet
2026-07-01 19:32     ` Andrii Nakryiko
2026-07-02  6:24       ` Leon Hwang [this message]
2026-07-02 10:14         ` Quentin Monnet
2026-07-02 14:08           ` Leon Hwang
2026-06-29 15:24 ` [PATCH bpf-next v8 6/9] selftests/bpf: Add tests to verify " Leon Hwang
2026-06-29 15:24 ` [PATCH bpf-next v8 7/9] selftests/bpf: Test direct reading/writing read-only percpu_array map Leon Hwang
2026-06-29 15:24 ` [PATCH bpf-next v8 8/9] selftests/bpf: Test verifier log for global percpu data Leon Hwang
2026-06-29 15:24 ` [PATCH bpf-next v8 9/9] selftests/bpf: Verify bpf_iter " Leon Hwang

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=ad19d283-aaab-41f1-b1a5-73de551b03a9@linux.dev \
    --to=leon.hwang@linux.dev \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.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.