The Linux Kernel Mailing List
 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: 19+ 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-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-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox