public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] bpf: introduce helper for populating bpf_cpumask
@ 2025-03-07 15:38 Emil Tsalapatis
  2025-03-07 15:38 ` [PATCH v6 1/4] bpf: add kfunc for populating cpumask bits Emil Tsalapatis
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Emil Tsalapatis @ 2025-03-07 15:38 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, tj,
	memxor, houtao, Emil Tsalapatis

Some BPF programs like scx schedulers have their own internal CPU mask types, 
mask types, which they must transform into struct bpf_cpumask instances
before passing them to scheduling-related kfuncs. There is currently no
way to efficiently populate the bitfield of a bpf_cpumask from BPF memory, 
and programs must use multiple bpf_cpumask_[set, clear] calls to do so. 
Introduce a kfunc helper to populate the bitfield of a bpf_cpumask from valid 
BPF memory with a single call.

Changelog :
-----------
v5->v6
v5:https://lore.kernel.org/bpf/20250307041738.6665-1-emil@etsalapatis.com/

Addressed feedback by Hou Tao:
	* Removed __success attributes from cpumask selftests
	* Fixed stale patch description that used old function name

v4->v5
v4: https://lore.kernel.org/bpf/20250305211235.368399-1-emil@etsalapatis.com/

Addressed feedback by Hou Tao:
	* Readded the tests in tools/selftests/bpf/prog_tests/cpumask.c,
	turns out the selftest entries were not duplicates.
	* Removed stray whitespace in selftest.
	* Add patch the missing selftest to prog_tests/cpumask.c
	* Explicitly annotate all cpumask selftests with __success

The last patch could very well be its own cleanup patch, but I rolled it into 
this series because it came up in the discussion. If the last patch in the
series has any issues I'd be fine with applying the first 3 patches and dealing 
with it separately.

v3->v4
v3: https://lore.kernel.org/bpf/20250305161327.203396-1-emil@etsalapatis.com/

	* Removed new tests from tools/selftests/bpf/prog_tests/cpumask.c because
they were being run twice.

Addressed feedback by Alexei Starovoitov:
	* Added missing return value in function kdoc
	* Added an additional patch fixing some missing kdoc fields in
	kernel/bpf/cpumask.c

Addressed feedback by Tejun Heo:
	* Renamed the kfunc to bpf_cpumask_populate to avoid confusion
	w/ bitmap_fill()

v2->v3
v2: https://lore.kernel.org/bpf/20250305021020.1004858-1-emil@etsalapatis.com/

Addressed feedback by Alexei Starovoitov:
	* Added back patch descriptions dropped from v1->v2
	* Elide the alignment check for archs with efficient
	  unaligned accesses

v1->v2
v1: https://lore.kernel.org/bpf/20250228003321.1409285-1-emil@etsalapatis.com/

Addressed feedback by Hou Tao:
	* Add check that the input buffer is aligned to sizeof(long)
	* Adjust input buffer size check to use bitmap_size()
	* Add selftest for checking the bit pattern of the bpf_cpumask
	* Moved all selftests into existing files

Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>

Emil Tsalapatis (4):
  bpf: add kfunc for populating cpumask bits
  selftests: bpf: add bpf_cpumask_populate selftests
  bpf: fix missing kdoc string fields in cpumask.c
  selftests: bpf: add cpumask test_refcount_null_tracking test to runner

 kernel/bpf/cpumask.c                          |  53 +++++++++
 .../selftests/bpf/prog_tests/cpumask.c        |   4 +
 .../selftests/bpf/progs/cpumask_common.h      |   1 +
 .../selftests/bpf/progs/cpumask_failure.c     |  38 ++++++
 .../selftests/bpf/progs/cpumask_success.c     | 110 ++++++++++++++++++
 5 files changed, 206 insertions(+)

-- 
2.47.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v6 1/4] bpf: add kfunc for populating cpumask bits
  2025-03-07 15:38 [PATCH v6 0/4] bpf: introduce helper for populating bpf_cpumask Emil Tsalapatis
@ 2025-03-07 15:38 ` Emil Tsalapatis
  2025-03-07 15:50   ` Tejun Heo
  2025-03-07 15:38 ` [PATCH v6 2/4] selftests: bpf: add bpf_cpumask_populate selftests Emil Tsalapatis
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Emil Tsalapatis @ 2025-03-07 15:38 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, tj,
	memxor, houtao, Emil Tsalapatis, Hou Tao

Add a helper kfunc that sets the bitmap of a bpf_cpumask from BPF memory.

Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
Acked-by: Hou Tao <houtao1@huawei.com>
---
 kernel/bpf/cpumask.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index cfa1c18e3a48..77900cbbbd75 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -420,6 +420,38 @@ __bpf_kfunc u32 bpf_cpumask_weight(const struct cpumask *cpumask)
 	return cpumask_weight(cpumask);
 }
 
+/**
+ * bpf_cpumask_populate() - Populate the CPU mask from the contents of
+ * a BPF memory region.
+ *
+ * @cpumask: The cpumask being populated.
+ * @src: The BPF memory holding the bit pattern.
+ * @src__sz: Length of the BPF memory region in bytes.
+ *
+ * Return:
+ * * 0 if the struct cpumask * instance was populated successfully.
+ * * -EACCES if the memory region is too small to populate the cpumask.
+ * * -EINVAL if the memory region is not aligned to the size of a long
+ *   and the architecture does not support efficient unaligned accesses.
+ */
+__bpf_kfunc int bpf_cpumask_populate(struct cpumask *cpumask, void *src, size_t src__sz)
+{
+	unsigned long source = (unsigned long)src;
+
+	/* The memory region must be large enough to populate the entire CPU mask. */
+	if (src__sz < bitmap_size(nr_cpu_ids))
+		return -EACCES;
+
+	/* If avoiding unaligned accesses, the input region must be aligned to the nearest long. */
+	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
+		!IS_ALIGNED(source, sizeof(long)))
+		return -EINVAL;
+
+	bitmap_copy(cpumask_bits(cpumask), src, nr_cpu_ids);
+
+	return 0;
+}
+
 __bpf_kfunc_end_defs();
 
 BTF_KFUNCS_START(cpumask_kfunc_btf_ids)
@@ -448,6 +480,7 @@ BTF_ID_FLAGS(func, bpf_cpumask_copy, KF_RCU)
 BTF_ID_FLAGS(func, bpf_cpumask_any_distribute, KF_RCU)
 BTF_ID_FLAGS(func, bpf_cpumask_any_and_distribute, KF_RCU)
 BTF_ID_FLAGS(func, bpf_cpumask_weight, KF_RCU)
+BTF_ID_FLAGS(func, bpf_cpumask_populate, KF_RCU)
 BTF_KFUNCS_END(cpumask_kfunc_btf_ids)
 
 static const struct btf_kfunc_id_set cpumask_kfunc_set = {
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 2/4] selftests: bpf: add bpf_cpumask_populate selftests
  2025-03-07 15:38 [PATCH v6 0/4] bpf: introduce helper for populating bpf_cpumask Emil Tsalapatis
  2025-03-07 15:38 ` [PATCH v6 1/4] bpf: add kfunc for populating cpumask bits Emil Tsalapatis
@ 2025-03-07 15:38 ` Emil Tsalapatis
  2025-03-08  1:37   ` Hou Tao
  2025-03-07 15:38 ` [PATCH v6 3/4] bpf: fix missing kdoc string fields in cpumask.c Emil Tsalapatis
  2025-03-07 15:38 ` [PATCH v6 4/4] selftests: bpf: add missing test to runner Emil Tsalapatis
  3 siblings, 1 reply; 9+ messages in thread
From: Emil Tsalapatis @ 2025-03-07 15:38 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, tj,
	memxor, houtao, Emil Tsalapatis

Add selftests for the bpf_cpumask_populate helper that sets a
bpf_cpumask to a bit pattern provided by a BPF program.

Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
---
 .../selftests/bpf/prog_tests/cpumask.c        |   3 +
 .../selftests/bpf/progs/cpumask_common.h      |   1 +
 .../selftests/bpf/progs/cpumask_failure.c     |  38 ++++++
 .../selftests/bpf/progs/cpumask_success.c     | 110 ++++++++++++++++++
 4 files changed, 152 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/cpumask.c b/tools/testing/selftests/bpf/prog_tests/cpumask.c
index e58a04654238..9b09beba988b 100644
--- a/tools/testing/selftests/bpf/prog_tests/cpumask.c
+++ b/tools/testing/selftests/bpf/prog_tests/cpumask.c
@@ -25,6 +25,9 @@ static const char * const cpumask_success_testcases[] = {
 	"test_global_mask_nested_deep_rcu",
 	"test_global_mask_nested_deep_array_rcu",
 	"test_cpumask_weight",
+	"test_populate_reject_small_mask",
+	"test_populate_reject_unaligned",
+	"test_populate",
 };
 
 static void verify_success(const char *prog_name)
diff --git a/tools/testing/selftests/bpf/progs/cpumask_common.h b/tools/testing/selftests/bpf/progs/cpumask_common.h
index 4ece7873ba60..86085b79f5ca 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_common.h
+++ b/tools/testing/selftests/bpf/progs/cpumask_common.h
@@ -61,6 +61,7 @@ u32 bpf_cpumask_any_distribute(const struct cpumask *src) __ksym __weak;
 u32 bpf_cpumask_any_and_distribute(const struct cpumask *src1,
 				   const struct cpumask *src2) __ksym __weak;
 u32 bpf_cpumask_weight(const struct cpumask *cpumask) __ksym __weak;
+int bpf_cpumask_populate(struct cpumask *cpumask, void *src, size_t src__sz) __ksym __weak;
 
 void bpf_rcu_read_lock(void) __ksym __weak;
 void bpf_rcu_read_unlock(void) __ksym __weak;
diff --git a/tools/testing/selftests/bpf/progs/cpumask_failure.c b/tools/testing/selftests/bpf/progs/cpumask_failure.c
index b40b52548ffb..8a2fd596c8a3 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_failure.c
+++ b/tools/testing/selftests/bpf/progs/cpumask_failure.c
@@ -222,3 +222,41 @@ int BPF_PROG(test_invalid_nested_array, struct task_struct *task, u64 clone_flag
 
 	return 0;
 }
+
+SEC("tp_btf/task_newtask")
+__failure __msg("type=scalar expected=fp")
+int BPF_PROG(test_populate_invalid_destination, struct task_struct *task, u64 clone_flags)
+{
+	struct bpf_cpumask *invalid = (struct bpf_cpumask *)0x123456;
+	u64 bits;
+	int ret;
+
+	ret = bpf_cpumask_populate((struct cpumask *)invalid, &bits, sizeof(bits));
+	if (!ret)
+		err = 2;
+
+	return 0;
+}
+
+SEC("tp_btf/task_newtask")
+__failure __msg("leads to invalid memory access")
+int BPF_PROG(test_populate_invalid_source, struct task_struct *task, u64 clone_flags)
+{
+	void *garbage = (void *)0x123456;
+	struct bpf_cpumask *local;
+	int ret;
+
+	local = create_cpumask();
+	if (!local) {
+		err = 1;
+		return 0;
+	}
+
+	ret = bpf_cpumask_populate((struct cpumask *)local, garbage, 8);
+	if (!ret)
+		err = 2;
+
+	bpf_cpumask_release(local);
+
+	return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
index 80ee469b0b60..23ef2737af50 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_success.c
+++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
@@ -770,3 +770,113 @@ int BPF_PROG(test_refcount_null_tracking, struct task_struct *task, u64 clone_fl
 		bpf_cpumask_release(mask2);
 	return 0;
 }
+
+SEC("tp_btf/task_newtask")
+int BPF_PROG(test_populate_reject_small_mask, struct task_struct *task, u64 clone_flags)
+{
+	struct bpf_cpumask *local;
+	u8 toofewbits;
+	int ret;
+
+	local = create_cpumask();
+	if (!local)
+		return 0;
+
+	/* The kfunc should prevent this operation */
+	ret = bpf_cpumask_populate((struct cpumask *)local, &toofewbits, sizeof(toofewbits));
+	if (ret != -EACCES)
+		err = 2;
+
+	bpf_cpumask_release(local);
+
+	return 0;
+}
+
+/* Mask is guaranteed to be large enough for bpf_cpumask_t. */
+#define CPUMASK_TEST_MASKLEN (sizeof(cpumask_t))
+
+/* Add an extra word for the test_populate_reject_unaligned test. */
+u64 bits[CPUMASK_TEST_MASKLEN / 8 + 1];
+extern bool CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS __kconfig __weak;
+
+SEC("tp_btf/task_newtask")
+int BPF_PROG(test_populate_reject_unaligned, struct task_struct *task, u64 clone_flags)
+{
+	struct bpf_cpumask *mask;
+	char *src;
+	int ret;
+
+	/* Skip if unaligned accesses are fine for this arch.  */
+	if (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+		return 0;
+
+	mask = bpf_cpumask_create();
+	if (!mask) {
+		err = 1;
+		return 0;
+	}
+
+	/* Misalign the source array by a byte. */
+	src = &((char *)bits)[1];
+
+	ret = bpf_cpumask_populate((struct cpumask *)mask, src, CPUMASK_TEST_MASKLEN);
+	if (ret != -EINVAL)
+		err = 2;
+
+	bpf_cpumask_release(mask);
+
+	return 0;
+}
+
+
+SEC("tp_btf/task_newtask")
+int BPF_PROG(test_populate, struct task_struct *task, u64 clone_flags)
+{
+	struct bpf_cpumask *mask;
+	bool bit;
+	int ret;
+	int i;
+
+	/* Set only odd bits. */
+	__builtin_memset(bits, 0xaa, CPUMASK_TEST_MASKLEN);
+
+	mask = bpf_cpumask_create();
+	if (!mask) {
+		err = 1;
+		return 0;
+	}
+
+	/* Pass the entire bits array, the kfunc will only copy the valid bits. */
+	ret = bpf_cpumask_populate((struct cpumask *)mask, bits, CPUMASK_TEST_MASKLEN);
+	if (ret) {
+		err = 2;
+		goto out;
+	}
+
+	/*
+	 * Test is there to appease the verifier. We cannot directly
+	 * access NR_CPUS, the upper bound for nr_cpus, so we infer
+	 * it from the size of cpumask_t.
+	 */
+	if (nr_cpus < 0 || nr_cpus >= CPUMASK_TEST_MASKLEN * 8) {
+		err = 3;
+		goto out;
+	}
+
+	bpf_for(i, 0, nr_cpus) {
+		/* Odd-numbered bits should be set, even ones unset. */
+		bit = bpf_cpumask_test_cpu(i, (const struct cpumask *)mask);
+		if (bit == (i % 2 != 0))
+			continue;
+
+		err = 4;
+		break;
+	}
+
+out:
+	bpf_cpumask_release(mask);
+
+	return 0;
+}
+
+#undef CPUMASK_TEST_MASKLEN
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 3/4] bpf: fix missing kdoc string fields in cpumask.c
  2025-03-07 15:38 [PATCH v6 0/4] bpf: introduce helper for populating bpf_cpumask Emil Tsalapatis
  2025-03-07 15:38 ` [PATCH v6 1/4] bpf: add kfunc for populating cpumask bits Emil Tsalapatis
  2025-03-07 15:38 ` [PATCH v6 2/4] selftests: bpf: add bpf_cpumask_populate selftests Emil Tsalapatis
@ 2025-03-07 15:38 ` Emil Tsalapatis
  2025-03-07 15:38 ` [PATCH v6 4/4] selftests: bpf: add missing test to runner Emil Tsalapatis
  3 siblings, 0 replies; 9+ messages in thread
From: Emil Tsalapatis @ 2025-03-07 15:38 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, tj,
	memxor, houtao, Emil Tsalapatis

Some bpf_cpumask-related kfuncs have kdoc strings that are missing
return values. Add a the missing descriptions for the return values.

Reported-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
---
 kernel/bpf/cpumask.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index 77900cbbbd75..9876c5fe6c2a 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -45,6 +45,10 @@ __bpf_kfunc_start_defs();
  *
  * bpf_cpumask_create() allocates memory using the BPF memory allocator, and
  * will not block. It may return NULL if no memory is available.
+ *
+ * Return:
+ * * A pointer to a new struct bpf_cpumask instance on success.
+ * * NULL if the BPF memory allocator is out of memory.
  */
 __bpf_kfunc struct bpf_cpumask *bpf_cpumask_create(void)
 {
@@ -71,6 +75,10 @@ __bpf_kfunc struct bpf_cpumask *bpf_cpumask_create(void)
  * Acquires a reference to a BPF cpumask. The cpumask returned by this function
  * must either be embedded in a map as a kptr, or freed with
  * bpf_cpumask_release().
+ *
+ * Return:
+ * * The struct bpf_cpumask pointer passed to the function.
+ *
  */
 __bpf_kfunc struct bpf_cpumask *bpf_cpumask_acquire(struct bpf_cpumask *cpumask)
 {
@@ -106,6 +114,9 @@ CFI_NOSEAL(bpf_cpumask_release_dtor);
  *
  * Find the index of the first nonzero bit of the cpumask. A struct bpf_cpumask
  * pointer may be safely passed to this function.
+ *
+ * Return:
+ * * The index of the first nonzero bit in the struct cpumask.
  */
 __bpf_kfunc u32 bpf_cpumask_first(const struct cpumask *cpumask)
 {
@@ -119,6 +130,9 @@ __bpf_kfunc u32 bpf_cpumask_first(const struct cpumask *cpumask)
  *
  * Find the index of the first unset bit of the cpumask. A struct bpf_cpumask
  * pointer may be safely passed to this function.
+ *
+ * Return:
+ * * The index of the first zero bit in the struct cpumask.
  */
 __bpf_kfunc u32 bpf_cpumask_first_zero(const struct cpumask *cpumask)
 {
@@ -133,6 +147,9 @@ __bpf_kfunc u32 bpf_cpumask_first_zero(const struct cpumask *cpumask)
  *
  * Find the index of the first nonzero bit of the AND of two cpumasks.
  * struct bpf_cpumask pointers may be safely passed to @src1 and @src2.
+ *
+ * Return:
+ * * The index of the first bit that is nonzero in both cpumask instances.
  */
 __bpf_kfunc u32 bpf_cpumask_first_and(const struct cpumask *src1,
 				      const struct cpumask *src2)
@@ -414,6 +431,9 @@ __bpf_kfunc u32 bpf_cpumask_any_and_distribute(const struct cpumask *src1,
  * @cpumask: The cpumask being queried.
  *
  * Count the number of set bits in the given cpumask.
+ *
+ * Return:
+ * * The number of bits set in the mask.
  */
 __bpf_kfunc u32 bpf_cpumask_weight(const struct cpumask *cpumask)
 {
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 4/4] selftests: bpf: add missing test to runner
  2025-03-07 15:38 [PATCH v6 0/4] bpf: introduce helper for populating bpf_cpumask Emil Tsalapatis
                   ` (2 preceding siblings ...)
  2025-03-07 15:38 ` [PATCH v6 3/4] bpf: fix missing kdoc string fields in cpumask.c Emil Tsalapatis
@ 2025-03-07 15:38 ` Emil Tsalapatis
  2025-03-08  1:34   ` Hou Tao
  3 siblings, 1 reply; 9+ messages in thread
From: Emil Tsalapatis @ 2025-03-07 15:38 UTC (permalink / raw)
  To: bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, tj,
	memxor, houtao, Emil Tsalapatis

BPF cpumask selftests need to be added to bpf/prog_tests/cpumask.c to be
run. However, the test_refcount_null_tracking is missing from the main
test file. Add the missing test name to properly trigger the selftest.

Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
---
 tools/testing/selftests/bpf/prog_tests/cpumask.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/cpumask.c b/tools/testing/selftests/bpf/prog_tests/cpumask.c
index 9b09beba988b..447a6e362fcd 100644
--- a/tools/testing/selftests/bpf/prog_tests/cpumask.c
+++ b/tools/testing/selftests/bpf/prog_tests/cpumask.c
@@ -25,6 +25,7 @@ static const char * const cpumask_success_testcases[] = {
 	"test_global_mask_nested_deep_rcu",
 	"test_global_mask_nested_deep_array_rcu",
 	"test_cpumask_weight",
+	"test_refcount_null_tracking",
 	"test_populate_reject_small_mask",
 	"test_populate_reject_unaligned",
 	"test_populate",
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 1/4] bpf: add kfunc for populating cpumask bits
  2025-03-07 15:38 ` [PATCH v6 1/4] bpf: add kfunc for populating cpumask bits Emil Tsalapatis
@ 2025-03-07 15:50   ` Tejun Heo
       [not found]     ` <CABFh=a63-=TooZ1s56=HqbNRUO5fWT3-+FSbK9U39HRVzY0i=A@mail.gmail.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Tejun Heo @ 2025-03-07 15:50 UTC (permalink / raw)
  To: Emil Tsalapatis
  Cc: bpf, ast, daniel, andrii, martin.lau, eddyz87, yonghong.song,
	memxor, houtao, Hou Tao

On Fri, Mar 07, 2025 at 10:38:44AM -0500, Emil Tsalapatis wrote:
> Add a helper kfunc that sets the bitmap of a bpf_cpumask from BPF memory.
> 
> Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
> Acked-by: Hou Tao <houtao1@huawei.com>

Would a kfunc to transfer it in the other direction be useful too? If so,
how would that function be named?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 1/4] bpf: add kfunc for populating cpumask bits
       [not found]         ` <Z8seyEHwlT5mhi4n@slm.duckdns.org>
@ 2025-03-07 16:53           ` Emil Tsalapatis
  0 siblings, 0 replies; 9+ messages in thread
From: Emil Tsalapatis @ 2025-03-07 16:53 UTC (permalink / raw)
  To: Tejun Heo, bpf
  Cc: ast, Eduard Zingerman, Kumar Kartikeya Dwivedi, yonghong.song,
	andrii, Hou Tao, martin.lau

On Fri, Mar 7, 2025 at 11:28 AM Tejun Heo <tj@kernel.org> wrote:
>
> On Fri, Mar 07, 2025 at 06:28:21AM -1000, Tejun Heo wrote:
> > Hello,
> >
> > On Fri, Mar 07, 2025 at 11:18:46AM -0500, Emil Tsalapatis wrote:
> > > On Fri, Mar 7, 2025 at 10:50 AM Tejun Heo <tj@kernel.org> wrote:
> > > >
> > > > On Fri, Mar 07, 2025 at 10:38:44AM -0500, Emil Tsalapatis wrote:
> > > > > Add a helper kfunc that sets the bitmap of a bpf_cpumask from BPF memory.
> > > > >
> > > > > Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
> > > > > Acked-by: Hou Tao <houtao1@huawei.com>
> > > >
> > > > Would a kfunc to transfer it in the other direction be useful too? If so,
> > > > how would that function be named?
> > > >
> > >
> > > We could add one, but it is not necessary because the BPF program can do the
> > > copy itself by reading the struct cpumask e.g. like this:
> > >
> > > https://github.com/sched-ext/scx/blob/ecdba1f4d9d518bd6a58343cd303187155a39bf3/scheds/rust/scx_wd40/src/bpf/cpumask.bpf.c#L184
> >
> > Ah, right.
> >
> > > If we added a function going bpf_cpumask -> BPF would
> > > bpf_cpumask_into() work as a name?
> >
> > Yeah, was mostly thinking whether the _populate() name would look weird if
> > we need something in the other direction. If we don't, the name is fine:
> >
> >  Acked-by: Tejun Heo <tj@kernel.org>
>
> Oops, you dropped the cc list. Can you restore the cc list and quote the
> whole exchange?
>

Ah sorry about that, I fat fingered the reply. Hopefully this properly
restores the thread in lore.kernel.org.

> Thanks.
>
> --
> tejun

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 4/4] selftests: bpf: add missing test to runner
  2025-03-07 15:38 ` [PATCH v6 4/4] selftests: bpf: add missing test to runner Emil Tsalapatis
@ 2025-03-08  1:34   ` Hou Tao
  0 siblings, 0 replies; 9+ messages in thread
From: Hou Tao @ 2025-03-08  1:34 UTC (permalink / raw)
  To: Emil Tsalapatis, bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, tj,
	memxor

Hi,

On 3/7/2025 11:38 PM, Emil Tsalapatis wrote:
> BPF cpumask selftests need to be added to bpf/prog_tests/cpumask.c to be
> run. However, the test_refcount_null_tracking is missing from the main
> test file. Add the missing test name to properly trigger the selftest.
>
> Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/cpumask.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/cpumask.c b/tools/testing/selftests/bpf/prog_tests/cpumask.c
> index 9b09beba988b..447a6e362fcd 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cpumask.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cpumask.c
> @@ -25,6 +25,7 @@ static const char * const cpumask_success_testcases[] = {
>  	"test_global_mask_nested_deep_rcu",
>  	"test_global_mask_nested_deep_array_rcu",
>  	"test_cpumask_weight",
> +	"test_refcount_null_tracking",
>  	"test_populate_reject_small_mask",
>  	"test_populate_reject_unaligned",
>  	"test_populate",

Just find out that the invocation of RUN_TESTS(cpumask_success) will
lead to the double test result output for every program in the
cpumask_success.c. Considering we have test the loading of
test_refcount_null_tracking program through cpumask_success_testcases.
It would be better to remove the __success annotation for
test_refcount_null_tracking and the invocation of
RUN_TESTS(cpumask_success) as well.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 2/4] selftests: bpf: add bpf_cpumask_populate selftests
  2025-03-07 15:38 ` [PATCH v6 2/4] selftests: bpf: add bpf_cpumask_populate selftests Emil Tsalapatis
@ 2025-03-08  1:37   ` Hou Tao
  0 siblings, 0 replies; 9+ messages in thread
From: Hou Tao @ 2025-03-08  1:37 UTC (permalink / raw)
  To: Emil Tsalapatis, bpf
  Cc: ast, daniel, andrii, martin.lau, eddyz87, yonghong.song, tj,
	memxor

Hi,

On 3/7/2025 11:38 PM, Emil Tsalapatis wrote:
> Add selftests for the bpf_cpumask_populate helper that sets a
> bpf_cpumask to a bit pattern provided by a BPF program.
>
> Signed-off-by: Emil Tsalapatis (Meta) <emil@etsalapatis.com>
> ---
>  .../selftests/bpf/prog_tests/cpumask.c        |   3 +
>  .../selftests/bpf/progs/cpumask_common.h      |   1 +
>  .../selftests/bpf/progs/cpumask_failure.c     |  38 ++++++
>  .../selftests/bpf/progs/cpumask_success.c     | 110 ++++++++++++++++++
>  4 files changed, 152 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/cpumask.c b/tools/testing/selftests/bpf/prog_tests/cpumask.c
> index e58a04654238..9b09beba988b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cpumask.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cpumask.c
> @@ -25,6 +25,9 @@ static const char * const cpumask_success_testcases[] = {
>  	"test_global_mask_nested_deep_rcu",
>  	"test_global_mask_nested_deep_array_rcu",
>  	"test_cpumask_weight",
> +	"test_populate_reject_small_mask",
> +	"test_populate_reject_unaligned",
> +	"test_populate",
>  };
>  
>  static void verify_success(const char *prog_name)
> diff --git a/tools/testing/selftests/bpf/progs/cpumask_common.h b/tools/testing/selftests/bpf/progs/cpumask_common.h
> index 4ece7873ba60..86085b79f5ca 100644
> --- a/tools/testing/selftests/bpf/progs/cpumask_common.h
> +++ b/tools/testing/selftests/bpf/progs/cpumask_common.h
> @@ -61,6 +61,7 @@ u32 bpf_cpumask_any_distribute(const struct cpumask *src) __ksym __weak;
>  u32 bpf_cpumask_any_and_distribute(const struct cpumask *src1,
>  				   const struct cpumask *src2) __ksym __weak;
>  u32 bpf_cpumask_weight(const struct cpumask *cpumask) __ksym __weak;
> +int bpf_cpumask_populate(struct cpumask *cpumask, void *src, size_t src__sz) __ksym __weak;
>  
>  void bpf_rcu_read_lock(void) __ksym __weak;
>  void bpf_rcu_read_unlock(void) __ksym __weak;
> diff --git a/tools/testing/selftests/bpf/progs/cpumask_failure.c b/tools/testing/selftests/bpf/progs/cpumask_failure.c
> index b40b52548ffb..8a2fd596c8a3 100644
> --- a/tools/testing/selftests/bpf/progs/cpumask_failure.c
> +++ b/tools/testing/selftests/bpf/progs/cpumask_failure.c
> @@ -222,3 +222,41 @@ int BPF_PROG(test_invalid_nested_array, struct task_struct *task, u64 clone_flag
>  
>  	return 0;
>  }
> +
> +SEC("tp_btf/task_newtask")
> +__failure __msg("type=scalar expected=fp")
> +int BPF_PROG(test_populate_invalid_destination, struct task_struct *task, u64 clone_flags)
> +{
> +	struct bpf_cpumask *invalid = (struct bpf_cpumask *)0x123456;
> +	u64 bits;
> +	int ret;
> +
> +	ret = bpf_cpumask_populate((struct cpumask *)invalid, &bits, sizeof(bits));
> +	if (!ret)
> +		err = 2;
> +
> +	return 0;
> +}
> +
> +SEC("tp_btf/task_newtask")
> +__failure __msg("leads to invalid memory access")
> +int BPF_PROG(test_populate_invalid_source, struct task_struct *task, u64 clone_flags)
> +{
> +	void *garbage = (void *)0x123456;
> +	struct bpf_cpumask *local;
> +	int ret;
> +
> +	local = create_cpumask();
> +	if (!local) {
> +		err = 1;
> +		return 0;
> +	}
> +
> +	ret = bpf_cpumask_populate((struct cpumask *)local, garbage, 8);
> +	if (!ret)
> +		err = 2;
> +
> +	bpf_cpumask_release(local);
> +
> +	return 0;
> +}
> diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
> index 80ee469b0b60..23ef2737af50 100644
> --- a/tools/testing/selftests/bpf/progs/cpumask_success.c
> +++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
> @@ -770,3 +770,113 @@ int BPF_PROG(test_refcount_null_tracking, struct task_struct *task, u64 clone_fl
>  		bpf_cpumask_release(mask2);
>  	return 0;
>  }
> +
> +SEC("tp_btf/task_newtask")
> +int BPF_PROG(test_populate_reject_small_mask, struct task_struct *task, u64 clone_flags)
> +{
> +	struct bpf_cpumask *local;
> +	u8 toofewbits;
> +	int ret;
> +

Sorry for bringing up it so later. It seems it is better to add an
is_test_task() check for these success tests.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-03-08  1:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-07 15:38 [PATCH v6 0/4] bpf: introduce helper for populating bpf_cpumask Emil Tsalapatis
2025-03-07 15:38 ` [PATCH v6 1/4] bpf: add kfunc for populating cpumask bits Emil Tsalapatis
2025-03-07 15:50   ` Tejun Heo
     [not found]     ` <CABFh=a63-=TooZ1s56=HqbNRUO5fWT3-+FSbK9U39HRVzY0i=A@mail.gmail.com>
     [not found]       ` <Z8sepWRKPbXvnMzf@slm.duckdns.org>
     [not found]         ` <Z8seyEHwlT5mhi4n@slm.duckdns.org>
2025-03-07 16:53           ` Emil Tsalapatis
2025-03-07 15:38 ` [PATCH v6 2/4] selftests: bpf: add bpf_cpumask_populate selftests Emil Tsalapatis
2025-03-08  1:37   ` Hou Tao
2025-03-07 15:38 ` [PATCH v6 3/4] bpf: fix missing kdoc string fields in cpumask.c Emil Tsalapatis
2025-03-07 15:38 ` [PATCH v6 4/4] selftests: bpf: add missing test to runner Emil Tsalapatis
2025-03-08  1:34   ` Hou Tao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox