[PATCH bpf 0/4] Fix the unmatched unit_size of bpf_mem_cache

[PATCH bpf 0/4] Fix the unmatched unit_size of bpf_mem_cache

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

Hi,

The patchset aims to fix the reported warning [0] when the unit_size of
bpf_mem_cache is mismatched with the object size of underly slab-cache.

Patch #1 fixes the warning by adjusting size_index according to the
value of KMALLOC_MIN_SIZE, so bpf_mem_cache with unit_size which is
smaller than KMALLOC_MIN_SIZE or is not aligned with KMALLOC_MIN_SIZE
will be redirected to bpf_mem_cache with bigger unit_size. Patch #2
doesn't do prefill for these redirected bpf_mem_cache to save memory.
Patch #3 adds further error check in bpf_mem_alloc_init() to ensure the
unit_size and object_size are always matched and to prevent potential
issues due to the mismatch.

Please see individual patches for more details. And comments are always
welcome.

[0]: https://lore.kernel.org/bpf/87jztjmmy4.fsf@all.your.base.are.belong.to.us

Hou Tao (4):
  bpf: Adjust size_index according to the value of KMALLOC_MIN_SIZE
  bpf: Don't prefill for unused bpf_mem_cache
  bpf: Ensure unit_size is matched with slab cache object size
  selftests/bpf: Test all valid alloc sizes for bpf mem allocator

 kernel/bpf/memalloc.c                         |  87 ++++++++++++-
 .../selftests/bpf/prog_tests/test_bpf_ma.c    |  50 +++++++
 .../testing/selftests/bpf/progs/test_bpf_ma.c | 123 ++++++++++++++++++
 3 files changed, 256 insertions(+), 4 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_bpf_ma.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_bpf_ma.c

-- 
2.29.2

[PATCH bpf 1/4] bpf: Adjust size_index according to the value of KMALLOC_MIN_SIZE

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

The following warning was reported when running "./test_progs -a
link_api -a linked_list" on a RISC-V QEMU VM:

  ------------[ cut here ]------------
  WARNING: CPU: 3 PID: 261 at kernel/bpf/memalloc.c:342 bpf_mem_refill
  Modules linked in: bpf_testmod(OE)
  CPU: 3 PID: 261 Comm: test_progs- ... 6.5.0-rc5-01743-gdcb152bb8328 #2
  Hardware name: riscv-virtio,qemu (DT)
  epc : bpf_mem_refill+0x1fc/0x206
   ra : irq_work_single+0x68/0x70
  epc : ffffffff801b1bc4 ra : ffffffff8015fe84 sp : ff2000000001be20
   gp : ffffffff82d26138 tp : ff6000008477a800 t0 : 0000000000046600
   t1 : ffffffff812b6ddc t2 : 0000000000000000 s0 : ff2000000001be70
   s1 : ff5ffffffffe8998 a0 : ff5ffffffffe8998 a1 : ff600003fef4b000
   a2 : 000000000000003f a3 : ffffffff80008250 a4 : 0000000000000060
   a5 : 0000000000000080 a6 : 0000000000000000 a7 : 0000000000735049
   s2 : ff5ffffffffe8998 s3 : 0000000000000022 s4 : 0000000000001000
   s5 : 0000000000000007 s6 : ff5ffffffffe8570 s7 : ffffffff82d6bd30
   s8 : 000000000000003f s9 : ffffffff82d2c5e8 s10: 000000000000ffff
   s11: ffffffff82d2c5d8 t3 : ffffffff81ea8f28 t4 : 0000000000000000
   t5 : ff6000008fd28278 t6 : 0000000000040000
  [<ffffffff801b1bc4>] bpf_mem_refill+0x1fc/0x206
  [<ffffffff8015fe84>] irq_work_single+0x68/0x70
  [<ffffffff8015feb4>] irq_work_run_list+0x28/0x36
  [<ffffffff8015fefa>] irq_work_run+0x38/0x66
  [<ffffffff8000828a>] handle_IPI+0x3a/0xb4
  [<ffffffff800a5c3a>] handle_percpu_devid_irq+0xa4/0x1f8
  [<ffffffff8009fafa>] generic_handle_domain_irq+0x28/0x36
  [<ffffffff800ae570>] ipi_mux_process+0xac/0xfa
  [<ffffffff8000a8ea>] sbi_ipi_handle+0x2e/0x88
  [<ffffffff8009fafa>] generic_handle_domain_irq+0x28/0x36
  [<ffffffff807ee70e>] riscv_intc_irq+0x36/0x4e
  [<ffffffff812b5d3a>] handle_riscv_irq+0x54/0x86
  [<ffffffff812b6904>] do_irq+0x66/0x98
  ---[ end trace 0000000000000000 ]---

The warning is due to WARN_ON_ONCE(tgt->unit_size != c->unit_size) in
free_bulk(). The direct reason is that a object is allocated and
freed by bpf_mem_caches with different unit_size.

The root cause is that KMALLOC_MIN_SIZE is 64 and there is no 96-bytes
slab cache in the specific VM. When linked_list test allocates a
72-bytes object through bpf_obj_new(), bpf_global_ma will allocate it
from a bpf_mem_cache with 96-bytes unit_size, but this bpf_mem_cache is
backed by 128-bytes slab cache. When the object is freed, bpf_mem_free()
uses ksize() to choose the corresponding bpf_mem_cache. Because the
object is allocated from 128-bytes slab cache, ksize() returns 128,
bpf_mem_free() chooses a 128-bytes bpf_mem_cache to free the object and
triggers the warning.

A similar warning will also be reported when using CONFIG_SLAB instead
of CONFIG_SLUB in a x86-64 kernel. Because CONFIG_SLUB defines
KMALLOC_MIN_SIZE as 8 but CONFIG_SLAB defines KMALLOC_MIN_SIZE as 32.

An alternative fix is to use kmalloc_size_round() in bpf_mem_alloc() to
choose a bpf_mem_cache which has the same unit_size with the backing
slab cache, but it may introduce performance degradation, so fix the
warning by adjusting the indexes in size_index according to the value of
KMALLOC_MIN_SIZE just like setup_kmalloc_cache_index_table() does.

Fixes: 822fb26bdb55 ("bpf: Add a hint to allocated objects.")
Reported-by: Björn Töpel <bjorn@kernel.org>
Closes: https://lore.kernel.org/bpf/87jztjmmy4.fsf@all.your.base.are.belong.to.us
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 kernel/bpf/memalloc.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
index 9c49ae53deaf..98d9e96fba3c 100644
--- a/kernel/bpf/memalloc.c
+++ b/kernel/bpf/memalloc.c
@@ -916,3 +916,41 @@ void notrace *bpf_mem_cache_alloc_flags(struct bpf_mem_alloc *ma, gfp_t flags)
 
 	return !ret ? NULL : ret + LLIST_NODE_SZ;
 }
+
+/* Most of the logic is taken from setup_kmalloc_cache_index_table() */
+static __init int bpf_mem_cache_adjust_size(void)
+{
+	unsigned int size, index;
+
+	/* Normally KMALLOC_MIN_SIZE is 8-bytes, but it can be
+	 * up-to 256-bytes.
+	 */
+	size = KMALLOC_MIN_SIZE;
+	if (size <= 192)
+		index = size_index[(size - 1) / 8];
+	else
+		index = fls(size - 1) - 1;
+	for (size = 8; size < KMALLOC_MIN_SIZE && size <= 192; size += 8)
+		size_index[(size - 1) / 8] = index;
+
+	/* The minimal alignment is 64-bytes, so disable 96-bytes cache and
+	 * use 128-bytes cache instead.
+	 */
+	if (KMALLOC_MIN_SIZE >= 64) {
+		index = size_index[(128 - 1) / 8];
+		for (size = 64 + 8; size <= 96; size += 8)
+			size_index[(size - 1) / 8] = index;
+	}
+
+	/* The minimal alignment is 128-bytes, so disable 192-bytes cache and
+	 * use 256-bytes cache instead.
+	 */
+	if (KMALLOC_MIN_SIZE >= 128) {
+		index = fls(256 - 1) - 1;
+		for (size = 128 + 8; size <= 192; size += 8)
+			size_index[(size - 1) / 8] = index;
+	}
+
+	return 0;
+}
+subsys_initcall(bpf_mem_cache_adjust_size);
-- 
2.29.2

[PATCH bpf 2/4] bpf: Don't prefill for unused bpf_mem_cache

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

When the unit_size of a bpf_mem_cache is unmatched with the object_size
of the underlying slab cache, the bpf_mem_cache will not be used, and
the allocation will be redirected to a bpf_mem_cache with a bigger
unit_size instead, so there is no need to prefill for these
unused bpf_mem_caches.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 kernel/bpf/memalloc.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
index 98d9e96fba3c..90c1ed8210a2 100644
--- a/kernel/bpf/memalloc.c
+++ b/kernel/bpf/memalloc.c
@@ -459,8 +459,7 @@ static void notrace irq_work_raise(struct bpf_mem_cache *c)
  * Typical case will be between 11K and 116K closer to 11K.
  * bpf progs can and should share bpf_mem_cache when possible.
  */
-
-static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
+static void init_refill_work(struct bpf_mem_cache *c)
 {
 	init_irq_work(&c->refill_work, bpf_mem_refill);
 	if (c->unit_size <= 256) {
@@ -476,7 +475,10 @@ static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
 		c->high_watermark = max(96 * 256 / c->unit_size, 3);
 	}
 	c->batch = max((c->high_watermark - c->low_watermark) / 4 * 3, 1);
+}
 
+static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
+{
 	/* To avoid consuming memory assume that 1st run of bpf
 	 * prog won't be doing more than 4 map_update_elem from
 	 * irq disabled region
@@ -521,6 +523,7 @@ int bpf_mem_alloc_init(struct bpf_mem_alloc *ma, int size, bool percpu)
 			c->objcg = objcg;
 			c->percpu_size = percpu_size;
 			c->tgt = c;
+			init_refill_work(c);
 			prefill_mem_cache(c, cpu);
 		}
 		ma->cache = pc;
@@ -544,6 +547,15 @@ int bpf_mem_alloc_init(struct bpf_mem_alloc *ma, int size, bool percpu)
 			c->unit_size = sizes[i];
 			c->objcg = objcg;
 			c->tgt = c;
+
+			init_refill_work(c);
+			/* Another bpf_mem_cache will be used when allocating
+			 * c->unit_size in bpf_mem_alloc(), so doesn't prefill
+			 * for the bpf_mem_cache because these free objects will
+			 * never be used.
+			 */
+			if (i != bpf_mem_cache_idx(c->unit_size))
+				continue;
 			prefill_mem_cache(c, cpu);
 		}
 	}
-- 
2.29.2

[PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
bpf_mem_cache is matched with the object_size of underlying slab cache.
If these two sizes are unmatched, print a warning once and return
-EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
the potential issue can be prevented.

Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 kernel/bpf/memalloc.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
index 90c1ed8210a2..1c22b90e754a 100644
--- a/kernel/bpf/memalloc.c
+++ b/kernel/bpf/memalloc.c
@@ -486,6 +486,24 @@ static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
 	alloc_bulk(c, c->unit_size <= 256 ? 4 : 1, cpu_to_node(cpu), false);
 }
 
+static int check_obj_size(struct bpf_mem_cache *c, unsigned int idx)
+{
+	struct llist_node *first;
+	unsigned int obj_size;
+
+	first = c->free_llist.first;
+	if (!first)
+		return 0;
+
+	obj_size = ksize(first);
+	if (obj_size != c->unit_size) {
+		WARN_ONCE(1, "bpf_mem_cache[%u]: unexpected object size %u, expect %u\n",
+			  idx, obj_size, c->unit_size);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 /* When size != 0 bpf_mem_cache for each cpu.
  * This is typical bpf hash map use case when all elements have equal size.
  *
@@ -496,10 +514,10 @@ static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
 int bpf_mem_alloc_init(struct bpf_mem_alloc *ma, int size, bool percpu)
 {
 	static u16 sizes[NUM_CACHES] = {96, 192, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096};
+	int cpu, i, err, unit_size, percpu_size = 0;
 	struct bpf_mem_caches *cc, __percpu *pcc;
 	struct bpf_mem_cache *c, __percpu *pc;
 	struct obj_cgroup *objcg = NULL;
-	int cpu, i, unit_size, percpu_size = 0;
 
 	if (size) {
 		pc = __alloc_percpu_gfp(sizeof(*pc), 8, GFP_KERNEL);
@@ -537,6 +555,7 @@ int bpf_mem_alloc_init(struct bpf_mem_alloc *ma, int size, bool percpu)
 	pcc = __alloc_percpu_gfp(sizeof(*cc), 8, GFP_KERNEL);
 	if (!pcc)
 		return -ENOMEM;
+	err = 0;
 #ifdef CONFIG_MEMCG_KMEM
 	objcg = get_obj_cgroup_from_current();
 #endif
@@ -557,10 +576,20 @@ int bpf_mem_alloc_init(struct bpf_mem_alloc *ma, int size, bool percpu)
 			if (i != bpf_mem_cache_idx(c->unit_size))
 				continue;
 			prefill_mem_cache(c, cpu);
+			err = check_obj_size(c, i);
+			if (err)
+				goto out;
 		}
 	}
+
+out:
 	ma->caches = pcc;
-	return 0;
+	/* refill_work is either zeroed or initialized, so it is safe to
+	 * call irq_work_sync().
+	 */
+	if (err)
+		bpf_mem_alloc_destroy(ma);
+	return err;
 }
 
 static void drain_mem_cache(struct bpf_mem_cache *c)
-- 
2.29.2

Re: [PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Nathan Chancellor]

Hi Hou,

On Fri, Sep 08, 2023 at 09:39:22PM +0800, Hou Tao wrote:
> From: Hou Tao <houtao1@huawei.com>
> 
> Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
> bpf_mem_cache is matched with the object_size of underlying slab cache.
> If these two sizes are unmatched, print a warning once and return
> -EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
> the potential issue can be prevented.
> 
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  kernel/bpf/memalloc.c | 33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> index 90c1ed8210a2..1c22b90e754a 100644
> --- a/kernel/bpf/memalloc.c
> +++ b/kernel/bpf/memalloc.c
> @@ -486,6 +486,24 @@ static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
>  	alloc_bulk(c, c->unit_size <= 256 ? 4 : 1, cpu_to_node(cpu), false);
>  }
>  
> +static int check_obj_size(struct bpf_mem_cache *c, unsigned int idx)
> +{
> +	struct llist_node *first;
> +	unsigned int obj_size;
> +
> +	first = c->free_llist.first;
> +	if (!first)
> +		return 0;
> +
> +	obj_size = ksize(first);
> +	if (obj_size != c->unit_size) {
> +		WARN_ONCE(1, "bpf_mem_cache[%u]: unexpected object size %u, expect %u\n",
> +			  idx, obj_size, c->unit_size);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}

I am seeing the warning added by this change as commit c93047255202
("bpf: Ensure unit_size is matched with slab cache object size") when
booting ARCH=riscv defconfig in QEMU. I have seen some discussion on the
mailing list around this, so I apologize if this is a duplicate report
but it sounded like the previously reported instance of this warning was
already resolved by some other changeor supposed to be resolved by [1].
Unfortunately, I tested both current bpf master (currently at
6bd5bcb18f94) with and without that change and I still see the warning
in both cases. The rootfs is available at [2], if it is relevant.

$ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- mrproper defconfig Image

$ qemu-system-riscv64 \
    -display none \
    -nodefaults \
    -bios default \
    -M virt \
    -append earlycon \
    -kernel arch/riscv/boot/Image \
    -initrd riscv-rootfs.cpio \
    -m 512m \
    -serial mon:stdio
...
[    0.000000] Linux version 6.5.0-12679-gc93047255202 (nathan@dev-arch.thelio-3990X) (riscv64-linux-gcc (GCC) 13.2.0, GNU ld (GNU Binutils) 2.41) #1 SMP Thu Sep 14 10:44:41 MST 2023
...
[    0.433002] ------------[ cut here ]------------
[    0.433128] bpf_mem_cache[0]: unexpected object size 128, expect 96
[    0.433585] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:500 bpf_mem_alloc_init+0x348/0x354
[    0.433810] Modules linked in:
[    0.433928] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-12679-gc93047255202 #1
[    0.434025] Hardware name: riscv-virtio,qemu (DT)
[    0.434105] epc : bpf_mem_alloc_init+0x348/0x354
[    0.434140]  ra : bpf_mem_alloc_init+0x348/0x354
[    0.434163] epc : ffffffff80112572 ra : ffffffff80112572 sp : ff2000000000bd30
[    0.434177]  gp : ffffffff81501588 tp : ff600000018d0000 t0 : ffffffff808cd1a0
[    0.434190]  t1 : 0720072007200720 t2 : 635f6d656d5f6670 s0 : ff2000000000bdd0
[    0.434202]  s1 : ffffffff80e17620 a0 : 0000000000000037 a1 : ffffffff814866b8
[    0.434215]  a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000
[    0.434227]  a5 : 0000000000000000 a6 : 0000000000000047 a7 : 0000000000000046
[    0.434239]  s2 : 000000000000000b s3 : 0000000000000000 s4 : 0000000000000000
[    0.434251]  s5 : 0000000000000100 s6 : ffffffff815031f8 s7 : ffffffff8153a610
[    0.434264]  s8 : 0000000000000060 s9 : 0000000000000060 s10: 0000000000000000
[    0.434276]  s11: ff6000001ffe5410 t3 : ff60000001858f00 t4 : ff60000001858f00
[    0.434288]  t5 : ff60000001858000 t6 : ff2000000000bb48
[    0.434299] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
[    0.434394] [<ffffffff80112572>] bpf_mem_alloc_init+0x348/0x354
[    0.434492] [<ffffffff80a0f302>] bpf_global_ma_init+0x1c/0x30
[    0.434516] [<ffffffff8000212c>] do_one_initcall+0x58/0x19c
[    0.434526] [<ffffffff80a0105e>] kernel_init_freeable+0x214/0x27e
[    0.434537] [<ffffffff808db4dc>] kernel_init+0x1e/0x10a
[    0.434548] [<ffffffff80003386>] ret_from_fork+0xa/0x1c
[    0.434618] ---[ end trace 0000000000000000 ]---

[1]: https://lore.kernel.org/20230913135943.3137292-1-houtao@huaweicloud.com/
[2]: https://github.com/ClangBuiltLinux/boot-utils/releases

Cheers,
Nathan

# bad: [98897dc735cf6635f0966f76eb0108354168fb15] Add linux-next specific files for 20230914
# good: [aed8aee11130a954356200afa3f1b8753e8a9482] Merge tag 'pmdomain-v6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm
git bisect start '98897dc735cf6635f0966f76eb0108354168fb15' 'aed8aee11130a954356200afa3f1b8753e8a9482'
# good: [ea1bbd78a48c8b325583e8c0bc2690850cb51807] bcachefs: Fix assorted checkpatch nits
git bisect good ea1bbd78a48c8b325583e8c0bc2690850cb51807
# bad: [9c4e2139cfa15d769eafd51bf3e051293b106986] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
git bisect bad 9c4e2139cfa15d769eafd51bf3e051293b106986
# bad: [4f07b13481ab390108b015da2bc8f560416e48d2] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
git bisect bad 4f07b13481ab390108b015da2bc8f560416e48d2
# bad: [bcfe98207530e1ea0004f4e5dbd6e7e4d9eb2471] Merge branch 'for-linux-next-fixes' of git://anongit.freedesktop.org/drm/drm-misc
git bisect bad bcfe98207530e1ea0004f4e5dbd6e7e4d9eb2471
# bad: [95d3e99b1ca8ad3da86c525cc1c00e4ba27592ac] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git
git bisect bad 95d3e99b1ca8ad3da86c525cc1c00e4ba27592ac
# good: [6836d373943afeeeb8e2989c22aaaa51218a83c6] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git
git bisect good 6836d373943afeeeb8e2989c22aaaa51218a83c6
# good: [3d3e2fb5e45a08a45ae01f0dfaf9621ae0e439f9] Merge branch 'main' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
git bisect good 3d3e2fb5e45a08a45ae01f0dfaf9621ae0e439f9
# bad: [51d56d51d3881addaea2c7242ae859155ae75607] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git
git bisect bad 51d56d51d3881addaea2c7242ae859155ae75607
# bad: [1a49f4195d3498fe458a7f5ff7ec5385da70d92e] bpf: Avoid dummy bpf_offload_netdev in __bpf_prog_dev_bound_init
git bisect bad 1a49f4195d3498fe458a7f5ff7ec5385da70d92e
# bad: [c930472552022bd09aab3cd946ba3f243070d5c7] bpf: Ensure unit_size is matched with slab cache object size
git bisect bad c930472552022bd09aab3cd946ba3f243070d5c7
# good: [7182e56411b9a8b76797ed7b6095fc84be76dfb0] selftests/bpf: Add kprobe_multi override test
git bisect good 7182e56411b9a8b76797ed7b6095fc84be76dfb0
# good: [b1d53958b69312e43c118d4093d8f93d3f6f80af] bpf: Don't prefill for unused bpf_mem_cache
git bisect good b1d53958b69312e43c118d4093d8f93d3f6f80af
# first bad commit: [c930472552022bd09aab3cd946ba3f243070d5c7] bpf: Ensure unit_size is matched with slab cache object size

Re: [PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Hou Tao]

[-- Attachment #1: Type: text/plain, Size: 7984 bytes --]

Hi,

On 9/15/2023 2:14 AM, Nathan Chancellor wrote:
> Hi Hou,
>
> On Fri, Sep 08, 2023 at 09:39:22PM +0800, Hou Tao wrote:
>> From: Hou Tao <houtao1@huawei.com>
>>
>> Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
>> bpf_mem_cache is matched with the object_size of underlying slab cache.
>> If these two sizes are unmatched, print a warning once and return
>> -EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
>> the potential issue can be prevented.
>>
>> Suggested-by: Alexei Starovoitov <ast@kernel.org>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>>  kernel/bpf/memalloc.c | 33 +++++++++++++++++++++++++++++++--
>>  1 file changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
>> index 90c1ed8210a2..1c22b90e754a 100644
>> --- a/kernel/bpf/memalloc.c
>> +++ b/kernel/bpf/memalloc.c
>> @@ -486,6 +486,24 @@ static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
>>  	alloc_bulk(c, c->unit_size <= 256 ? 4 : 1, cpu_to_node(cpu), false);
>>  }
>>  
>> +static int check_obj_size(struct bpf_mem_cache *c, unsigned int idx)
>> +{
>> +	struct llist_node *first;
>> +	unsigned int obj_size;
>> +
>> +	first = c->free_llist.first;
>> +	if (!first)
>> +		return 0;
>> +
>> +	obj_size = ksize(first);
>> +	if (obj_size != c->unit_size) {
>> +		WARN_ONCE(1, "bpf_mem_cache[%u]: unexpected object size %u, expect %u\n",
>> +			  idx, obj_size, c->unit_size);
>> +		return -EINVAL;
>> +	}
>> +	return 0;
>> +}
> I am seeing the warning added by this change as commit c93047255202
> ("bpf: Ensure unit_size is matched with slab cache object size") when
> booting ARCH=riscv defconfig in QEMU. I have seen some discussion on the
> mailing list around this, so I apologize if this is a duplicate report
> but it sounded like the previously reported instance of this warning was
> already resolved by some other changeor supposed to be resolved by [1].
> Unfortunately, I tested both current bpf master (currently at
> 6bd5bcb18f94) with and without that change and I still see the warning
> in both cases. The rootfs is available at [2], if it is relevant.

Thanks for the report. It seems it is a new problem which should be
fixed by commit d52b59315bf5 ("bpf: Adjust size_index according to the
value of KMALLOC_MIN_SIZE"), but failed to. However I could not
reproduce the problem by using the provided steps. Do you configure any
boot cmd line in your local setup ? I suspect dma_get_cache_alignment()
returns 64 of 1 in your setup, so slab-96 is rounded up to slab-128.
Could you please run the attached debug patch and post its output ?

> $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- mrproper defconfig Image
>
> $ qemu-system-riscv64 \
>     -display none \
>     -nodefaults \
>     -bios default \
>     -M virt \
>     -append earlycon \
>     -kernel arch/riscv/boot/Image \
>     -initrd riscv-rootfs.cpio \
>     -m 512m \
>     -serial mon:stdio
> ...
> [    0.000000] Linux version 6.5.0-12679-gc93047255202 (nathan@dev-arch.thelio-3990X) (riscv64-linux-gcc (GCC) 13.2.0, GNU ld (GNU Binutils) 2.41) #1 SMP Thu Sep 14 10:44:41 MST 2023
> ...
> [    0.433002] ------------[ cut here ]------------
> [    0.433128] bpf_mem_cache[0]: unexpected object size 128, expect 96
> [    0.433585] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:500 bpf_mem_alloc_init+0x348/0x354
> [    0.433810] Modules linked in:
> [    0.433928] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-12679-gc93047255202 #1
> [    0.434025] Hardware name: riscv-virtio,qemu (DT)
> [    0.434105] epc : bpf_mem_alloc_init+0x348/0x354
> [    0.434140]  ra : bpf_mem_alloc_init+0x348/0x354
> [    0.434163] epc : ffffffff80112572 ra : ffffffff80112572 sp : ff2000000000bd30
> [    0.434177]  gp : ffffffff81501588 tp : ff600000018d0000 t0 : ffffffff808cd1a0
> [    0.434190]  t1 : 0720072007200720 t2 : 635f6d656d5f6670 s0 : ff2000000000bdd0
> [    0.434202]  s1 : ffffffff80e17620 a0 : 0000000000000037 a1 : ffffffff814866b8
> [    0.434215]  a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000
> [    0.434227]  a5 : 0000000000000000 a6 : 0000000000000047 a7 : 0000000000000046
> [    0.434239]  s2 : 000000000000000b s3 : 0000000000000000 s4 : 0000000000000000
> [    0.434251]  s5 : 0000000000000100 s6 : ffffffff815031f8 s7 : ffffffff8153a610
> [    0.434264]  s8 : 0000000000000060 s9 : 0000000000000060 s10: 0000000000000000
> [    0.434276]  s11: ff6000001ffe5410 t3 : ff60000001858f00 t4 : ff60000001858f00
> [    0.434288]  t5 : ff60000001858000 t6 : ff2000000000bb48
> [    0.434299] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> [    0.434394] [<ffffffff80112572>] bpf_mem_alloc_init+0x348/0x354
> [    0.434492] [<ffffffff80a0f302>] bpf_global_ma_init+0x1c/0x30
> [    0.434516] [<ffffffff8000212c>] do_one_initcall+0x58/0x19c
> [    0.434526] [<ffffffff80a0105e>] kernel_init_freeable+0x214/0x27e
> [    0.434537] [<ffffffff808db4dc>] kernel_init+0x1e/0x10a
> [    0.434548] [<ffffffff80003386>] ret_from_fork+0xa/0x1c
> [    0.434618] ---[ end trace 0000000000000000 ]---
>
> [1]: https://lore.kernel.org/20230913135943.3137292-1-houtao@huaweicloud.com/
> [2]: https://github.com/ClangBuiltLinux/boot-utils/releases
>
> Cheers,
> Nathan
>
> # bad: [98897dc735cf6635f0966f76eb0108354168fb15] Add linux-next specific files for 20230914
> # good: [aed8aee11130a954356200afa3f1b8753e8a9482] Merge tag 'pmdomain-v6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm
> git bisect start '98897dc735cf6635f0966f76eb0108354168fb15' 'aed8aee11130a954356200afa3f1b8753e8a9482'
> # good: [ea1bbd78a48c8b325583e8c0bc2690850cb51807] bcachefs: Fix assorted checkpatch nits
> git bisect good ea1bbd78a48c8b325583e8c0bc2690850cb51807
> # bad: [9c4e2139cfa15d769eafd51bf3e051293b106986] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
> git bisect bad 9c4e2139cfa15d769eafd51bf3e051293b106986
> # bad: [4f07b13481ab390108b015da2bc8f560416e48d2] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
> git bisect bad 4f07b13481ab390108b015da2bc8f560416e48d2
> # bad: [bcfe98207530e1ea0004f4e5dbd6e7e4d9eb2471] Merge branch 'for-linux-next-fixes' of git://anongit.freedesktop.org/drm/drm-misc
> git bisect bad bcfe98207530e1ea0004f4e5dbd6e7e4d9eb2471
> # bad: [95d3e99b1ca8ad3da86c525cc1c00e4ba27592ac] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git
> git bisect bad 95d3e99b1ca8ad3da86c525cc1c00e4ba27592ac
> # good: [6836d373943afeeeb8e2989c22aaaa51218a83c6] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git
> git bisect good 6836d373943afeeeb8e2989c22aaaa51218a83c6
> # good: [3d3e2fb5e45a08a45ae01f0dfaf9621ae0e439f9] Merge branch 'main' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
> git bisect good 3d3e2fb5e45a08a45ae01f0dfaf9621ae0e439f9
> # bad: [51d56d51d3881addaea2c7242ae859155ae75607] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git
> git bisect bad 51d56d51d3881addaea2c7242ae859155ae75607
> # bad: [1a49f4195d3498fe458a7f5ff7ec5385da70d92e] bpf: Avoid dummy bpf_offload_netdev in __bpf_prog_dev_bound_init
> git bisect bad 1a49f4195d3498fe458a7f5ff7ec5385da70d92e
> # bad: [c930472552022bd09aab3cd946ba3f243070d5c7] bpf: Ensure unit_size is matched with slab cache object size
> git bisect bad c930472552022bd09aab3cd946ba3f243070d5c7
> # good: [7182e56411b9a8b76797ed7b6095fc84be76dfb0] selftests/bpf: Add kprobe_multi override test
> git bisect good 7182e56411b9a8b76797ed7b6095fc84be76dfb0
> # good: [b1d53958b69312e43c118d4093d8f93d3f6f80af] bpf: Don't prefill for unused bpf_mem_cache
> git bisect good b1d53958b69312e43c118d4093d8f93d3f6f80af
> # first bad commit: [c930472552022bd09aab3cd946ba3f243070d5c7] bpf: Ensure unit_size is matched with slab cache object size


[-- Attachment #2: 0001-bpf-Check-the-return-value-of-dma_get_cache_alignmen.patch --]
[-- Type: text/plain, Size: 1595 bytes --]

From 369f2e59cc6c63c3924f654ba3f8491dba58b87b Mon Sep 17 00:00:00 2001
From: Hou Tao <houtao1@huawei.com>
Date: Sat, 16 Sep 2023 10:35:52 +0800
Subject: [PATCH] bpf: Check the return value of dma_get_cache_alignment()

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 kernel/bpf/memalloc.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
index 1c22b90e754a..c8d2c3097c43 100644
--- a/kernel/bpf/memalloc.c
+++ b/kernel/bpf/memalloc.c
@@ -6,6 +6,8 @@
 #include <linux/irq_work.h>
 #include <linux/bpf_mem_alloc.h>
 #include <linux/memcontrol.h>
+#include <linux/dma-mapping.h>
+#include <linux/swiotlb.h>
 #include <asm/local.h>
 
 /* Any context (including NMI) BPF specific memory allocator.
@@ -958,6 +960,14 @@ void notrace *bpf_mem_cache_alloc_flags(struct bpf_mem_alloc *ma, gfp_t flags)
 	return !ret ? NULL : ret + LLIST_NODE_SZ;
 }
 
+static unsigned int __kmalloc_minalign(void)
+{
+	if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
+	    is_swiotlb_allocated())
+		return ARCH_KMALLOC_MINALIGN;
+	return dma_get_cache_alignment();
+}
+
 /* Most of the logic is taken from setup_kmalloc_cache_index_table() */
 static __init int bpf_mem_cache_adjust_size(void)
 {
@@ -992,6 +1002,9 @@ static __init int bpf_mem_cache_adjust_size(void)
 			size_index[(size - 1) / 8] = index;
 	}
 
+	pr_info("==== bpf_mem_cache: KMALLOC_MIN_SIZE %u ARCH_KMALLOC_MINALIGN %u min_align %u\n",
+		KMALLOC_MIN_SIZE, ARCH_KMALLOC_MINALIGN, __kmalloc_minalign());
+
 	return 0;
 }
 subsys_initcall(bpf_mem_cache_adjust_size);
-- 
2.29.2

Re: [PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Nathan Chancellor]

On Sat, Sep 16, 2023 at 10:38:09AM +0800, Hou Tao wrote:
> Hi,
> 
> On 9/15/2023 2:14 AM, Nathan Chancellor wrote:
> > Hi Hou,
> >
> > On Fri, Sep 08, 2023 at 09:39:22PM +0800, Hou Tao wrote:
> >> From: Hou Tao <houtao1@huawei.com>
> >>
> >> Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
> >> bpf_mem_cache is matched with the object_size of underlying slab cache.
> >> If these two sizes are unmatched, print a warning once and return
> >> -EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
> >> the potential issue can be prevented.
> >>
> >> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> >> Signed-off-by: Hou Tao <houtao1@huawei.com>
> >> ---
> >>  kernel/bpf/memalloc.c | 33 +++++++++++++++++++++++++++++++--
> >>  1 file changed, 31 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> >> index 90c1ed8210a2..1c22b90e754a 100644
> >> --- a/kernel/bpf/memalloc.c
> >> +++ b/kernel/bpf/memalloc.c
> >> @@ -486,6 +486,24 @@ static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
> >>  	alloc_bulk(c, c->unit_size <= 256 ? 4 : 1, cpu_to_node(cpu), false);
> >>  }
> >>  
> >> +static int check_obj_size(struct bpf_mem_cache *c, unsigned int idx)
> >> +{
> >> +	struct llist_node *first;
> >> +	unsigned int obj_size;
> >> +
> >> +	first = c->free_llist.first;
> >> +	if (!first)
> >> +		return 0;
> >> +
> >> +	obj_size = ksize(first);
> >> +	if (obj_size != c->unit_size) {
> >> +		WARN_ONCE(1, "bpf_mem_cache[%u]: unexpected object size %u, expect %u\n",
> >> +			  idx, obj_size, c->unit_size);
> >> +		return -EINVAL;
> >> +	}
> >> +	return 0;
> >> +}
> > I am seeing the warning added by this change as commit c93047255202
> > ("bpf: Ensure unit_size is matched with slab cache object size") when
> > booting ARCH=riscv defconfig in QEMU. I have seen some discussion on the
> > mailing list around this, so I apologize if this is a duplicate report
> > but it sounded like the previously reported instance of this warning was
> > already resolved by some other changeor supposed to be resolved by [1].
> > Unfortunately, I tested both current bpf master (currently at
> > 6bd5bcb18f94) with and without that change and I still see the warning
> > in both cases. The rootfs is available at [2], if it is relevant.
> 
> Thanks for the report. It seems it is a new problem which should be
> fixed by commit d52b59315bf5 ("bpf: Adjust size_index according to the
> value of KMALLOC_MIN_SIZE"), but failed to. However I could not
> reproduce the problem by using the provided steps. Do you configure any
> boot cmd line in your local setup ? I suspect dma_get_cache_alignment()
> returns 64 of 1 in your setup, so slab-96 is rounded up to slab-128.
> Could you please run the attached debug patch and post its output ?

Apologies for the delay, I have been traveling (and will be again
shortly). It seems like your theory could be accurate, as I see:

[    0.120969] ==== bpf_mem_cache: KMALLOC_MIN_SIZE 8 ARCH_KMALLOC_MINALIGN 8 min_align 64

There are no cmdline arguments aside from 'earlycon'. Could the QEMU
version matter here for reproduction purposes, as I am still able to
reproduce with my commands on next-20230919? I am using 8.1.0.

Cheers,
Nathan

> > $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- mrproper defconfig Image
> >
> > $ qemu-system-riscv64 \
> >     -display none \
> >     -nodefaults \
> >     -bios default \
> >     -M virt \
> >     -append earlycon \
> >     -kernel arch/riscv/boot/Image \
> >     -initrd riscv-rootfs.cpio \
> >     -m 512m \
> >     -serial mon:stdio
> > ...
> > [    0.000000] Linux version 6.5.0-12679-gc93047255202 (nathan@dev-arch.thelio-3990X) (riscv64-linux-gcc (GCC) 13.2.0, GNU ld (GNU Binutils) 2.41) #1 SMP Thu Sep 14 10:44:41 MST 2023
> > ...
> > [    0.433002] ------------[ cut here ]------------
> > [    0.433128] bpf_mem_cache[0]: unexpected object size 128, expect 96
> > [    0.433585] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:500 bpf_mem_alloc_init+0x348/0x354
> > [    0.433810] Modules linked in:
> > [    0.433928] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-12679-gc93047255202 #1
> > [    0.434025] Hardware name: riscv-virtio,qemu (DT)
> > [    0.434105] epc : bpf_mem_alloc_init+0x348/0x354
> > [    0.434140]  ra : bpf_mem_alloc_init+0x348/0x354
> > [    0.434163] epc : ffffffff80112572 ra : ffffffff80112572 sp : ff2000000000bd30
> > [    0.434177]  gp : ffffffff81501588 tp : ff600000018d0000 t0 : ffffffff808cd1a0
> > [    0.434190]  t1 : 0720072007200720 t2 : 635f6d656d5f6670 s0 : ff2000000000bdd0
> > [    0.434202]  s1 : ffffffff80e17620 a0 : 0000000000000037 a1 : ffffffff814866b8
> > [    0.434215]  a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000
> > [    0.434227]  a5 : 0000000000000000 a6 : 0000000000000047 a7 : 0000000000000046
> > [    0.434239]  s2 : 000000000000000b s3 : 0000000000000000 s4 : 0000000000000000
> > [    0.434251]  s5 : 0000000000000100 s6 : ffffffff815031f8 s7 : ffffffff8153a610
> > [    0.434264]  s8 : 0000000000000060 s9 : 0000000000000060 s10: 0000000000000000
> > [    0.434276]  s11: ff6000001ffe5410 t3 : ff60000001858f00 t4 : ff60000001858f00
> > [    0.434288]  t5 : ff60000001858000 t6 : ff2000000000bb48
> > [    0.434299] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [    0.434394] [<ffffffff80112572>] bpf_mem_alloc_init+0x348/0x354
> > [    0.434492] [<ffffffff80a0f302>] bpf_global_ma_init+0x1c/0x30
> > [    0.434516] [<ffffffff8000212c>] do_one_initcall+0x58/0x19c
> > [    0.434526] [<ffffffff80a0105e>] kernel_init_freeable+0x214/0x27e
> > [    0.434537] [<ffffffff808db4dc>] kernel_init+0x1e/0x10a
> > [    0.434548] [<ffffffff80003386>] ret_from_fork+0xa/0x1c
> > [    0.434618] ---[ end trace 0000000000000000 ]---
> >
> > [1]: https://lore.kernel.org/20230913135943.3137292-1-houtao@huaweicloud.com/
> > [2]: https://github.com/ClangBuiltLinux/boot-utils/releases
> >
> > Cheers,
> > Nathan
> >
> > # bad: [98897dc735cf6635f0966f76eb0108354168fb15] Add linux-next specific files for 20230914
> > # good: [aed8aee11130a954356200afa3f1b8753e8a9482] Merge tag 'pmdomain-v6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm
> > git bisect start '98897dc735cf6635f0966f76eb0108354168fb15' 'aed8aee11130a954356200afa3f1b8753e8a9482'
> > # good: [ea1bbd78a48c8b325583e8c0bc2690850cb51807] bcachefs: Fix assorted checkpatch nits
> > git bisect good ea1bbd78a48c8b325583e8c0bc2690850cb51807
> > # bad: [9c4e2139cfa15d769eafd51bf3e051293b106986] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
> > git bisect bad 9c4e2139cfa15d769eafd51bf3e051293b106986
> > # bad: [4f07b13481ab390108b015da2bc8f560416e48d2] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
> > git bisect bad 4f07b13481ab390108b015da2bc8f560416e48d2
> > # bad: [bcfe98207530e1ea0004f4e5dbd6e7e4d9eb2471] Merge branch 'for-linux-next-fixes' of git://anongit.freedesktop.org/drm/drm-misc
> > git bisect bad bcfe98207530e1ea0004f4e5dbd6e7e4d9eb2471
> > # bad: [95d3e99b1ca8ad3da86c525cc1c00e4ba27592ac] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git
> > git bisect bad 95d3e99b1ca8ad3da86c525cc1c00e4ba27592ac
> > # good: [6836d373943afeeeb8e2989c22aaaa51218a83c6] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git
> > git bisect good 6836d373943afeeeb8e2989c22aaaa51218a83c6
> > # good: [3d3e2fb5e45a08a45ae01f0dfaf9621ae0e439f9] Merge branch 'main' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
> > git bisect good 3d3e2fb5e45a08a45ae01f0dfaf9621ae0e439f9
> > # bad: [51d56d51d3881addaea2c7242ae859155ae75607] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git
> > git bisect bad 51d56d51d3881addaea2c7242ae859155ae75607
> > # bad: [1a49f4195d3498fe458a7f5ff7ec5385da70d92e] bpf: Avoid dummy bpf_offload_netdev in __bpf_prog_dev_bound_init
> > git bisect bad 1a49f4195d3498fe458a7f5ff7ec5385da70d92e
> > # bad: [c930472552022bd09aab3cd946ba3f243070d5c7] bpf: Ensure unit_size is matched with slab cache object size
> > git bisect bad c930472552022bd09aab3cd946ba3f243070d5c7
> > # good: [7182e56411b9a8b76797ed7b6095fc84be76dfb0] selftests/bpf: Add kprobe_multi override test
> > git bisect good 7182e56411b9a8b76797ed7b6095fc84be76dfb0
> > # good: [b1d53958b69312e43c118d4093d8f93d3f6f80af] bpf: Don't prefill for unused bpf_mem_cache
> > git bisect good b1d53958b69312e43c118d4093d8f93d3f6f80af
> > # first bad commit: [c930472552022bd09aab3cd946ba3f243070d5c7] bpf: Ensure unit_size is matched with slab cache object size
> 

> From 369f2e59cc6c63c3924f654ba3f8491dba58b87b Mon Sep 17 00:00:00 2001
> From: Hou Tao <houtao1@huawei.com>
> Date: Sat, 16 Sep 2023 10:35:52 +0800
> Subject: [PATCH] bpf: Check the return value of dma_get_cache_alignment()
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  kernel/bpf/memalloc.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
> index 1c22b90e754a..c8d2c3097c43 100644
> --- a/kernel/bpf/memalloc.c
> +++ b/kernel/bpf/memalloc.c
> @@ -6,6 +6,8 @@
>  #include <linux/irq_work.h>
>  #include <linux/bpf_mem_alloc.h>
>  #include <linux/memcontrol.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/swiotlb.h>
>  #include <asm/local.h>
>  
>  /* Any context (including NMI) BPF specific memory allocator.
> @@ -958,6 +960,14 @@ void notrace *bpf_mem_cache_alloc_flags(struct bpf_mem_alloc *ma, gfp_t flags)
>  	return !ret ? NULL : ret + LLIST_NODE_SZ;
>  }
>  
> +static unsigned int __kmalloc_minalign(void)
> +{
> +	if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
> +	    is_swiotlb_allocated())
> +		return ARCH_KMALLOC_MINALIGN;
> +	return dma_get_cache_alignment();
> +}
> +
>  /* Most of the logic is taken from setup_kmalloc_cache_index_table() */
>  static __init int bpf_mem_cache_adjust_size(void)
>  {
> @@ -992,6 +1002,9 @@ static __init int bpf_mem_cache_adjust_size(void)
>  			size_index[(size - 1) / 8] = index;
>  	}
>  
> +	pr_info("==== bpf_mem_cache: KMALLOC_MIN_SIZE %u ARCH_KMALLOC_MINALIGN %u min_align %u\n",
> +		KMALLOC_MIN_SIZE, ARCH_KMALLOC_MINALIGN, __kmalloc_minalign());
> +
>  	return 0;
>  }
>  subsys_initcall(bpf_mem_cache_adjust_size);
> -- 
> 2.29.2
> 

Re: [PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Hou Tao]

Hi,

On 9/20/2023 7:46 AM, Nathan Chancellor wrote:
> On Sat, Sep 16, 2023 at 10:38:09AM +0800, Hou Tao wrote:
>> Hi,
>>
>> On 9/15/2023 2:14 AM, Nathan Chancellor wrote:
>>> Hi Hou,
>>>
>>> On Fri, Sep 08, 2023 at 09:39:22PM +0800, Hou Tao wrote:
>>>> From: Hou Tao <houtao1@huawei.com>
>>>>
>>>> Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
>>>> bpf_mem_cache is matched with the object_size of underlying slab cache.
>>>> If these two sizes are unmatched, print a warning once and return
>>>> -EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
>>>> the potential issue can be prevented.
>>>>
>>>> Suggested-by: Alexei Starovoitov <ast@kernel.org>
>>>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>>>> ---
>>>>  kernel/bpf/memalloc.c | 33 +++++++++++++++++++++++++++++++--
>>>>  1 file changed, 31 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
>>>> index 90c1ed8210a2..1c22b90e754a 100644
>>>> --- a/kernel/bpf/memalloc.c
>>>> +++ b/kernel/bpf/memalloc.c
>>>> @@ -486,6 +486,24 @@ static void prefill_mem_cache(struct bpf_mem_cache *c, int cpu)
>>>>  	alloc_bulk(c, c->unit_size <= 256 ? 4 : 1, cpu_to_node(cpu), false);
>>>>  }
>>>>  
>>>> +static int check_obj_size(struct bpf_mem_cache *c, unsigned int idx)
>>>> +{
>>>> +	struct llist_node *first;
>>>> +	unsigned int obj_size;
>>>> +
>>>> +	first = c->free_llist.first;
>>>> +	if (!first)
>>>> +		return 0;
>>>> +
>>>> +	obj_size = ksize(first);
>>>> +	if (obj_size != c->unit_size) {
>>>> +		WARN_ONCE(1, "bpf_mem_cache[%u]: unexpected object size %u, expect %u\n",
>>>> +			  idx, obj_size, c->unit_size);
>>>> +		return -EINVAL;
>>>> +	}
>>>> +	return 0;
>>>> +}
>>> I am seeing the warning added by this change as commit c93047255202
>>> ("bpf: Ensure unit_size is matched with slab cache object size") when
>>> booting ARCH=riscv defconfig in QEMU. I have seen some discussion on the
>>> mailing list around this, so I apologize if this is a duplicate report
>>> but it sounded like the previously reported instance of this warning was
>>> already resolved by some other changeor supposed to be resolved by [1].
>>> Unfortunately, I tested both current bpf master (currently at
>>> 6bd5bcb18f94) with and without that change and I still see the warning
>>> in both cases. The rootfs is available at [2], if it is relevant.
>> Thanks for the report. It seems it is a new problem which should be
>> fixed by commit d52b59315bf5 ("bpf: Adjust size_index according to the
>> value of KMALLOC_MIN_SIZE"), but failed to. However I could not
>> reproduce the problem by using the provided steps. Do you configure any
>> boot cmd line in your local setup ? I suspect dma_get_cache_alignment()
>> returns 64 of 1 in your setup, so slab-96 is rounded up to slab-128.
>> Could you please run the attached debug patch and post its output ?

Sorry for the delay. I was in off-site training last week.

> Apologies for the delay, I have been traveling (and will be again
> shortly). It seems like your theory could be accurate, as I see:
>
> [    0.120969] ==== bpf_mem_cache: KMALLOC_MIN_SIZE 8 ARCH_KMALLOC_MINALIGN 8 min_align 64
>
> There are no cmdline arguments aside from 'earlycon'. Could the QEMU
> version matter here for reproduction purposes, as I am still able to
> reproduce with my commands on next-20230919? I am using 8.1.0.

The QEMU version matters. I can reproduce the problem by using v8.1.0
but can not reproduce by using v4.2.1. Will post a patch to fix it soon.
>
> Cheers,
> Nathan
>
>>> $ make -skj"$(nproc)" ARCH=riscv CROSS_COMPILE=riscv64-linux- mrproper defconfig Image
>>>
>>> $ qemu-system-riscv64 \
>>>     -display none \
>>>     -nodefaults \
>>>     -bios default \
>>>     -M virt \
>>>     -append earlycon \
>>>     -kernel arch/riscv/boot/Image \
>>>     -initrd riscv-rootfs.cpio \
>>>     -m 512m \
>>>     -serial mon:stdio
>>> ...
>>> [    0.000000] Linux version 6.5.0-12679-gc93047255202 (nathan@dev-arch.thelio-3990X) (riscv64-linux-gcc (GCC) 13.2.0, GNU ld (GNU Binutils) 2.41) #1 SMP Thu Sep 14 10:44:41 MST 2023
>>> ...
>>> [    0.433002] ------------[ cut here ]------------
>>> [    0.433128] bpf_mem_cache[0]: unexpected object size 128, expect 96
>>> [    0.433585] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:500 bpf_mem_alloc_init+0x348/0x354
>>> [    0.433810] Modules linked in:
>>> [    0.433928] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.5.0-12679-gc93047255202 #1
>>> [    0.434025] Hardware name: riscv-virtio,qemu (DT)
>>> [    0.434105] epc : bpf_mem_alloc_init+0x348/0x354
>>> [    0.434140]  ra : bpf_mem_alloc_init+0x348/0x354
>>> [    0.434163] epc : ffffffff80112572 ra : ffffffff80112572 sp : ff2000000000bd30
>>> [    0.434177]  gp : ffffffff81501588 tp : ff600000018d0000 t0 : ffffffff808cd1a0
>>> [    0.434190]  t1 : 0720072007200720 t2 : 635f6d656d5f6670 s0 : ff2000000000bdd0
>>> [    0.434202]  s1 : ffffffff80e17620 a0 : 0000000000000037 a1 : ffffffff814866b8
>>> [    0.434215]  a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000
>>> [    0.434227]  a5 : 0000000000000000 a6 : 0000000000000047 a7 : 0000000000000046
>>> [    0.434239]  s2 : 000000000000000b s3 : 0000000000000000 s4 : 0000000000000000
>>> [    0.434251]  s5 : 0000000000000100 s6 : ffffffff815031f8 s7 : ffffffff8153a610
>>> [    0.434264]  s8 : 0000000000000060 s9 : 0000000000000060 s10: 0000000000000000
>>> [    0.434276]  s11: ff6000001ffe5410 t3 : ff60000001858f00 t4 : ff60000001858f00
>>> [    0.434288]  t5 : ff60000001858000 t6 : ff2000000000bb48
>>> [    0.434299] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
>>> [    0.434394] [<ffffffff80112572>] bpf_mem_alloc_init+0x348/0x354
>>> [    0.434492] [<ffffffff80a0f302>] bpf_global_ma_init+0x1c/0x30
>>> [    0.434516] [<ffffffff8000212c>] do_one_initcall+0x58/0x19c
>>> [    0.434526] [<ffffffff80a0105e>] kernel_init_freeable+0x214/0x27e
>>> [    0.434537] [<ffffffff808db4dc>] kernel_init+0x1e/0x10a
>>> [    0.434548] [<ffffffff80003386>] ret_from_fork+0xa/0x1c
>>> [    0.434618] ---[ end trace 0000000000000000 ]---
>>>
>>> [1]: https://lore.kernel.org/20230913135943.3137292-1-houtao@huaweicloud.com/
>>> [2]: https://github.com/ClangBuiltLinux/boot-utils/releases
>>>
>>> Cheers,
>>> Nathan
>>>
>>> # bad: [98897dc735cf6635f0966f76eb0108354168fb15] Add linux-next specific files for 20230914
>>> # good: [aed8aee11130a954356200afa3f1b8753e8a9482] Merge tag 'pmdomain-v6.6-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/linux-pm
>>> git bisect start '98897dc735cf6635f0966f76eb0108354168fb15' 'aed8aee11130a954356200afa3f1b8753e8a9482'
>>> # good: [ea1bbd78a48c8b325583e8c0bc2690850cb51807] bcachefs: Fix assorted checkpatch nits
>>> git bisect good ea1bbd78a48c8b325583e8c0bc2690850cb51807
>>> # bad: [9c4e2139cfa15d769eafd51bf3e051293b106986] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
>>> git bisect bad 9c4e2139cfa15d769eafd51bf3e051293b106986
>>> # bad: [4f07b13481ab390108b015da2bc8f560416e48d2] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
>>> git bisect bad 4f07b13481ab390108b015da2bc8f560416e48d2
>>> # bad: [bcfe98207530e1ea0004f4e5dbd6e7e4d9eb2471] Merge branch 'for-linux-next-fixes' of git://anongit.freedesktop.org/drm/drm-misc
>>> git bisect bad bcfe98207530e1ea0004f4e5dbd6e7e4d9eb2471
>>> # bad: [95d3e99b1ca8ad3da86c525cc1c00e4ba27592ac] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless.git
>>> git bisect bad 95d3e99b1ca8ad3da86c525cc1c00e4ba27592ac
>>> # good: [6836d373943afeeeb8e2989c22aaaa51218a83c6] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc.git
>>> git bisect good 6836d373943afeeeb8e2989c22aaaa51218a83c6
>>> # good: [3d3e2fb5e45a08a45ae01f0dfaf9621ae0e439f9] Merge branch 'main' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
>>> git bisect good 3d3e2fb5e45a08a45ae01f0dfaf9621ae0e439f9
>>> # bad: [51d56d51d3881addaea2c7242ae859155ae75607] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git
>>> git bisect bad 51d56d51d3881addaea2c7242ae859155ae75607
>>> # bad: [1a49f4195d3498fe458a7f5ff7ec5385da70d92e] bpf: Avoid dummy bpf_offload_netdev in __bpf_prog_dev_bound_init
>>> git bisect bad 1a49f4195d3498fe458a7f5ff7ec5385da70d92e
>>> # bad: [c930472552022bd09aab3cd946ba3f243070d5c7] bpf: Ensure unit_size is matched with slab cache object size
>>> git bisect bad c930472552022bd09aab3cd946ba3f243070d5c7
>>> # good: [7182e56411b9a8b76797ed7b6095fc84be76dfb0] selftests/bpf: Add kprobe_multi override test
>>> git bisect good 7182e56411b9a8b76797ed7b6095fc84be76dfb0
>>> # good: [b1d53958b69312e43c118d4093d8f93d3f6f80af] bpf: Don't prefill for unused bpf_mem_cache
>>> git bisect good b1d53958b69312e43c118d4093d8f93d3f6f80af
>>> # first bad commit: [c930472552022bd09aab3cd946ba3f243070d5c7] bpf: Ensure unit_size is matched with slab cache object size
>> From 369f2e59cc6c63c3924f654ba3f8491dba58b87b Mon Sep 17 00:00:00 2001
>> From: Hou Tao <houtao1@huawei.com>
>> Date: Sat, 16 Sep 2023 10:35:52 +0800
>> Subject: [PATCH] bpf: Check the return value of dma_get_cache_alignment()
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>>  kernel/bpf/memalloc.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/kernel/bpf/memalloc.c b/kernel/bpf/memalloc.c
>> index 1c22b90e754a..c8d2c3097c43 100644
>> --- a/kernel/bpf/memalloc.c
>> +++ b/kernel/bpf/memalloc.c
>> @@ -6,6 +6,8 @@
>>  #include <linux/irq_work.h>
>>  #include <linux/bpf_mem_alloc.h>
>>  #include <linux/memcontrol.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/swiotlb.h>
>>  #include <asm/local.h>
>>  
>>  /* Any context (including NMI) BPF specific memory allocator.
>> @@ -958,6 +960,14 @@ void notrace *bpf_mem_cache_alloc_flags(struct bpf_mem_alloc *ma, gfp_t flags)
>>  	return !ret ? NULL : ret + LLIST_NODE_SZ;
>>  }
>>  
>> +static unsigned int __kmalloc_minalign(void)
>> +{
>> +	if (IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
>> +	    is_swiotlb_allocated())
>> +		return ARCH_KMALLOC_MINALIGN;
>> +	return dma_get_cache_alignment();
>> +}
>> +
>>  /* Most of the logic is taken from setup_kmalloc_cache_index_table() */
>>  static __init int bpf_mem_cache_adjust_size(void)
>>  {
>> @@ -992,6 +1002,9 @@ static __init int bpf_mem_cache_adjust_size(void)
>>  			size_index[(size - 1) / 8] = index;
>>  	}
>>  
>> +	pr_info("==== bpf_mem_cache: KMALLOC_MIN_SIZE %u ARCH_KMALLOC_MINALIGN %u min_align %u\n",
>> +		KMALLOC_MIN_SIZE, ARCH_KMALLOC_MINALIGN, __kmalloc_minalign());
>> +
>>  	return 0;
>>  }
>>  subsys_initcall(bpf_mem_cache_adjust_size);
>> -- 
>> 2.29.2
>>
> .

Re: [PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Guenter Roeck]

Hi,

On Fri, Sep 08, 2023 at 09:39:22PM +0800, Hou Tao wrote:
> From: Hou Tao <houtao1@huawei.com>
> 
> Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
> bpf_mem_cache is matched with the object_size of underlying slab cache.
> If these two sizes are unmatched, print a warning once and return
> -EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
> the potential issue can be prevented.
> 
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Hou Tao <houtao1@huawei.com>

With this patch in place, I see the following backtrace on riscv systems.

[    2.953088] bpf_mem_cache[0]: unexpected object size 128, expect 96
[    2.953481] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:507 bpf_mem_alloc_init+0x326/0x32e
[    2.953645] Modules linked in:
[    2.953736] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.6.0-rc2-00244-g27bbf45eae9c #1
[    2.953790] Hardware name: riscv-virtio,qemu (DT)
[    2.953855] epc : bpf_mem_alloc_init+0x326/0x32e
[    2.953891]  ra : bpf_mem_alloc_init+0x326/0x32e
[    2.953909] epc : ffffffff8016cbd2 ra : ffffffff8016cbd2 sp : ff2000000000bd20
[    2.953920]  gp : ffffffff81c39298 tp : ff60000002e80040 t0 : 0000000000000000
[    2.953930]  t1 : ffffffffbbbabbc3 t2 : 635f6d656d5f6670 s0 : ff2000000000bdc0
[    2.953940]  s1 : ffffffff8121c7da a0 : 0000000000000037 a1 : ffffffff81a93048
[    2.953949]  a2 : 0000000000000010 a3 : 0000000000000001 a4 : 0000000000000000
[    2.953959]  a5 : 0000000000000000 a6 : ffffffff81c4fe08 a7 : 0000000000000000
[    2.953968]  s2 : 000000000000000b s3 : 0000000000000000 s4 : 0000000000000000
[    2.953977]  s5 : 0000000000000000 s6 : 0000000000000100 s7 : ff5ffffffffd3128
[    2.953986]  s8 : ffffffff81c3d1f8 s9 : 0000000000000060 s10: 0000000000000000
[    2.953996]  s11: 0000000000000060 t3 : 0000000065a61b33 t4 : 0000000000000009
[    2.954005]  t5 : ffffffffde180000 t6 : ff2000000000bb08
[    2.954014] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
[    2.954047] [<ffffffff8016cbd2>] bpf_mem_alloc_init+0x326/0x32e
[    2.954087] [<ffffffff80e11426>] bpf_global_ma_init+0x1c/0x30
[    2.954097] [<ffffffff8000285e>] do_one_initcall+0x5c/0x238
[    2.954105] [<ffffffff80e011ae>] kernel_init_freeable+0x29a/0x30e
[    2.954115] [<ffffffff80c0312c>] kernel_init+0x1e/0x112
[    2.954124] [<ffffffff80003d82>] ret_from_fork+0xa/0x1c

Copying riscv maintainers and mailing list for feedback / comments.

Thanks,
Guenter

Re: [PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Emil Renner Berthing]

Guenter Roeck wrote:
> Hi,
>
> On Fri, Sep 08, 2023 at 09:39:22PM +0800, Hou Tao wrote:
> > From: Hou Tao <houtao1@huawei.com>
> >
> > Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
> > bpf_mem_cache is matched with the object_size of underlying slab cache.
> > If these two sizes are unmatched, print a warning once and return
> > -EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
> > the potential issue can be prevented.
> >
> > Suggested-by: Alexei Starovoitov <ast@kernel.org>
> > Signed-off-by: Hou Tao <houtao1@huawei.com>
>
> With this patch in place, I see the following backtrace on riscv systems.
>
> [    2.953088] bpf_mem_cache[0]: unexpected object size 128, expect 96
> [    2.953481] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:507 bpf_mem_alloc_init+0x326/0x32e
> [    2.953645] Modules linked in:
> [    2.953736] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.6.0-rc2-00244-g27bbf45eae9c #1
> [    2.953790] Hardware name: riscv-virtio,qemu (DT)
> [    2.953855] epc : bpf_mem_alloc_init+0x326/0x32e
> [    2.953891]  ra : bpf_mem_alloc_init+0x326/0x32e
> [    2.953909] epc : ffffffff8016cbd2 ra : ffffffff8016cbd2 sp : ff2000000000bd20
> [    2.953920]  gp : ffffffff81c39298 tp : ff60000002e80040 t0 : 0000000000000000
> [    2.953930]  t1 : ffffffffbbbabbc3 t2 : 635f6d656d5f6670 s0 : ff2000000000bdc0
> [    2.953940]  s1 : ffffffff8121c7da a0 : 0000000000000037 a1 : ffffffff81a93048
> [    2.953949]  a2 : 0000000000000010 a3 : 0000000000000001 a4 : 0000000000000000
> [    2.953959]  a5 : 0000000000000000 a6 : ffffffff81c4fe08 a7 : 0000000000000000
> [    2.953968]  s2 : 000000000000000b s3 : 0000000000000000 s4 : 0000000000000000
> [    2.953977]  s5 : 0000000000000000 s6 : 0000000000000100 s7 : ff5ffffffffd3128
> [    2.953986]  s8 : ffffffff81c3d1f8 s9 : 0000000000000060 s10: 0000000000000000
> [    2.953996]  s11: 0000000000000060 t3 : 0000000065a61b33 t4 : 0000000000000009
> [    2.954005]  t5 : ffffffffde180000 t6 : ff2000000000bb08
> [    2.954014] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> [    2.954047] [<ffffffff8016cbd2>] bpf_mem_alloc_init+0x326/0x32e
> [    2.954087] [<ffffffff80e11426>] bpf_global_ma_init+0x1c/0x30
> [    2.954097] [<ffffffff8000285e>] do_one_initcall+0x5c/0x238
> [    2.954105] [<ffffffff80e011ae>] kernel_init_freeable+0x29a/0x30e
> [    2.954115] [<ffffffff80c0312c>] kernel_init+0x1e/0x112
> [    2.954124] [<ffffffff80003d82>] ret_from_fork+0xa/0x1c
>
> Copying riscv maintainers and mailing list for feedback / comments.

If it makes a difference I also see this with 6.6-rc3 on my Nezha board
(Allwinner D1), but not on my VisionFive 2 (JH7110) running the same kernel.

/Emil

Re: [PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Lad, Prabhakar]

On Fri, Sep 29, 2023 at 7:52 PM Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
>
> Guenter Roeck wrote:
> > Hi,
> >
> > On Fri, Sep 08, 2023 at 09:39:22PM +0800, Hou Tao wrote:
> > > From: Hou Tao <houtao1@huawei.com>
> > >
> > > Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
> > > bpf_mem_cache is matched with the object_size of underlying slab cache.
> > > If these two sizes are unmatched, print a warning once and return
> > > -EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
> > > the potential issue can be prevented.
> > >
> > > Suggested-by: Alexei Starovoitov <ast@kernel.org>
> > > Signed-off-by: Hou Tao <houtao1@huawei.com>
> >
> > With this patch in place, I see the following backtrace on riscv systems.
> >
> > [    2.953088] bpf_mem_cache[0]: unexpected object size 128, expect 96
> > [    2.953481] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:507 bpf_mem_alloc_init+0x326/0x32e
> > [    2.953645] Modules linked in:
> > [    2.953736] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.6.0-rc2-00244-g27bbf45eae9c #1
> > [    2.953790] Hardware name: riscv-virtio,qemu (DT)
> > [    2.953855] epc : bpf_mem_alloc_init+0x326/0x32e
> > [    2.953891]  ra : bpf_mem_alloc_init+0x326/0x32e
> > [    2.953909] epc : ffffffff8016cbd2 ra : ffffffff8016cbd2 sp : ff2000000000bd20
> > [    2.953920]  gp : ffffffff81c39298 tp : ff60000002e80040 t0 : 0000000000000000
> > [    2.953930]  t1 : ffffffffbbbabbc3 t2 : 635f6d656d5f6670 s0 : ff2000000000bdc0
> > [    2.953940]  s1 : ffffffff8121c7da a0 : 0000000000000037 a1 : ffffffff81a93048
> > [    2.953949]  a2 : 0000000000000010 a3 : 0000000000000001 a4 : 0000000000000000
> > [    2.953959]  a5 : 0000000000000000 a6 : ffffffff81c4fe08 a7 : 0000000000000000
> > [    2.953968]  s2 : 000000000000000b s3 : 0000000000000000 s4 : 0000000000000000
> > [    2.953977]  s5 : 0000000000000000 s6 : 0000000000000100 s7 : ff5ffffffffd3128
> > [    2.953986]  s8 : ffffffff81c3d1f8 s9 : 0000000000000060 s10: 0000000000000000
> > [    2.953996]  s11: 0000000000000060 t3 : 0000000065a61b33 t4 : 0000000000000009
> > [    2.954005]  t5 : ffffffffde180000 t6 : ff2000000000bb08
> > [    2.954014] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> > [    2.954047] [<ffffffff8016cbd2>] bpf_mem_alloc_init+0x326/0x32e
> > [    2.954087] [<ffffffff80e11426>] bpf_global_ma_init+0x1c/0x30
> > [    2.954097] [<ffffffff8000285e>] do_one_initcall+0x5c/0x238
> > [    2.954105] [<ffffffff80e011ae>] kernel_init_freeable+0x29a/0x30e
> > [    2.954115] [<ffffffff80c0312c>] kernel_init+0x1e/0x112
> > [    2.954124] [<ffffffff80003d82>] ret_from_fork+0xa/0x1c
> >
> > Copying riscv maintainers and mailing list for feedback / comments.
>
> If it makes a difference I also see this with 6.6-rc3 on my Nezha board
> (Allwinner D1), but not on my VisionFive 2 (JH7110) running the same kernel.
>

Adding one more RISC-V board (Renesas RZ/Five) to list where I see this issue:
[    0.268936] ------------[ cut here ]------------
[    0.268953] bpf_mem_cache[0]: unexpected object size 128, expect 96
[    0.268993] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:507
bpf_mem_alloc_init+0x306/0x30e
[    0.269026] Modules linked in:
[    0.269038] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
6.6.0-rc3-00091-g6acfe6a7c746 #538
[    0.269049] Hardware name: Renesas SMARC EVK based on r9a07g043f01 (DT)
[    0.269054] epc : bpf_mem_alloc_init+0x306/0x30e
[    0.269066]  ra : bpf_mem_alloc_init+0x306/0x30e
[    0.269077] epc : ffffffff8010e7ac ra : ffffffff8010e7ac sp :
ffffffc80000bd30
[    0.269084]  gp : ffffffff81506d08 tp : ffffffd801938000 t0 :
ffffffff81419e40
[    0.269090]  t1 : ffffffffffffffff t2 : 2d2d2d2d2d2d2d2d s0 :
ffffffc80000bdd0
[    0.269096]  s1 : 000000000000000b a0 : 0000000000000037 a1 :
0000000200000020
[    0.269102]  a2 : 0000000000000000 a3 : 0000000000000001 a4 :
0000000000000000
[    0.269107]  a5 : 0000000000000000 a6 : 0000000000000000 a7 :
0000000000000000
[    0.269112]  s2 : 0000000000000000 s3 : 0000000000000000 s4 :
0000000000000100
[    0.269118]  s5 : ffffffff815081f8 s6 : ffffffff8153f610 s7 :
0000000000000060
[    0.269124]  s8 : 0000000000000060 s9 : ffffffd836fd2770 s10:
ffffffff80e18dd8
[    0.269130]  s11: 0000000000000000 t3 : ffffffff8151e174 t4 :
ffffffff8151e174
[    0.269135]  t5 : ffffffff8151e150 t6 : ffffffff8151e1b8
[    0.269140] status: 0000000200000120 badaddr: 0000000000000000
cause: 0000000000000003
[    0.269147] [<ffffffff8010e7ac>] bpf_mem_alloc_init+0x306/0x30e
[    0.269160] [<ffffffff80a0f3e4>] bpf_global_ma_init+0x1c/0x30
[    0.269174] [<ffffffff8000212c>] do_one_initcall+0x58/0x19c
[    0.269186] [<ffffffff80a00ffc>] kernel_init_freeable+0x200/0x26a
[    0.269203] [<ffffffff809195a4>] kernel_init+0x1e/0x10a
[    0.269213] [<ffffffff800035ee>] ret_from_fork+0xa/0x1c
[    0.269224] ---[ end trace 0000000000000000 ]---
[    0.281983] debug_vm_pgtable: [debug_vm_pgtable         ]:
Validating architecture page table helpers

Cheers,
Prabhakar

Re: [PATCH bpf 3/4] bpf: Ensure unit_size is matched with slab cache object size

[Alexei Starovoitov]

On Fri, Sep 29, 2023 at 1:24 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> On Fri, Sep 29, 2023 at 7:52 PM Emil Renner Berthing
> <emil.renner.berthing@canonical.com> wrote:
> >
> > Guenter Roeck wrote:
> > > Hi,
> > >
> > > On Fri, Sep 08, 2023 at 09:39:22PM +0800, Hou Tao wrote:
> > > > From: Hou Tao <houtao1@huawei.com>
> > > >
> > > > Add extra check in bpf_mem_alloc_init() to ensure the unit_size of
> > > > bpf_mem_cache is matched with the object_size of underlying slab cache.
> > > > If these two sizes are unmatched, print a warning once and return
> > > > -EINVAL in bpf_mem_alloc_init(), so the mismatch can be found early and
> > > > the potential issue can be prevented.
> > > >
> > > > Suggested-by: Alexei Starovoitov <ast@kernel.org>
> > > > Signed-off-by: Hou Tao <houtao1@huawei.com>
> > >
> > > With this patch in place, I see the following backtrace on riscv systems.
> > >
> > > [    2.953088] bpf_mem_cache[0]: unexpected object size 128, expect 96
> > > [    2.953481] WARNING: CPU: 0 PID: 1 at kernel/bpf/memalloc.c:507 bpf_mem_alloc_init+0x326/0x32e
> > > [    2.953645] Modules linked in:
> > > [    2.953736] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.6.0-rc2-00244-g27bbf45eae9c #1
> > > [    2.953790] Hardware name: riscv-virtio,qemu (DT)
> > > [    2.953855] epc : bpf_mem_alloc_init+0x326/0x32e
> > > [    2.953891]  ra : bpf_mem_alloc_init+0x326/0x32e
> > > [    2.953909] epc : ffffffff8016cbd2 ra : ffffffff8016cbd2 sp : ff2000000000bd20
> > > [    2.953920]  gp : ffffffff81c39298 tp : ff60000002e80040 t0 : 0000000000000000
> > > [    2.953930]  t1 : ffffffffbbbabbc3 t2 : 635f6d656d5f6670 s0 : ff2000000000bdc0
> > > [    2.953940]  s1 : ffffffff8121c7da a0 : 0000000000000037 a1 : ffffffff81a93048
> > > [    2.953949]  a2 : 0000000000000010 a3 : 0000000000000001 a4 : 0000000000000000
> > > [    2.953959]  a5 : 0000000000000000 a6 : ffffffff81c4fe08 a7 : 0000000000000000
> > > [    2.953968]  s2 : 000000000000000b s3 : 0000000000000000 s4 : 0000000000000000
> > > [    2.953977]  s5 : 0000000000000000 s6 : 0000000000000100 s7 : ff5ffffffffd3128
> > > [    2.953986]  s8 : ffffffff81c3d1f8 s9 : 0000000000000060 s10: 0000000000000000
> > > [    2.953996]  s11: 0000000000000060 t3 : 0000000065a61b33 t4 : 0000000000000009
> > > [    2.954005]  t5 : ffffffffde180000 t6 : ff2000000000bb08
> > > [    2.954014] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003
> > > [    2.954047] [<ffffffff8016cbd2>] bpf_mem_alloc_init+0x326/0x32e
> > > [    2.954087] [<ffffffff80e11426>] bpf_global_ma_init+0x1c/0x30
> > > [    2.954097] [<ffffffff8000285e>] do_one_initcall+0x5c/0x238
> > > [    2.954105] [<ffffffff80e011ae>] kernel_init_freeable+0x29a/0x30e
> > > [    2.954115] [<ffffffff80c0312c>] kernel_init+0x1e/0x112
> > > [    2.954124] [<ffffffff80003d82>] ret_from_fork+0xa/0x1c
> > >
> > > Copying riscv maintainers and mailing list for feedback / comments.
> >
> > If it makes a difference I also see this with 6.6-rc3 on my Nezha board
> > (Allwinner D1), but not on my VisionFive 2 (JH7110) running the same kernel.
> >
>
> Adding one more RISC-V board (Renesas RZ/Five) to list where I see this issue:

Could you please help test the proposed fix:
https://patchwork.kernel.org/project/netdevbpf/patch/20230928101558.2594068-1-houtao@huaweicloud.com/

[PATCH bpf 4/4] selftests/bpf: Test all valid alloc sizes for bpf mem allocator

[Hou Tao]

From: Hou Tao <houtao1@huawei.com>

Add a test to test all possible and valid allocation size for bpf
memory allocator. For each possible allocation size, the test uses
the following two steps to test the alloc and free path:

1) allocate N (N > high_watermark) objects to trigger the refill
   executed in irq_work.
2) free N objects to trigger the freeing executed in irq_work.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 .../selftests/bpf/prog_tests/test_bpf_ma.c    |  50 +++++++
 .../testing/selftests/bpf/progs/test_bpf_ma.c | 123 ++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_bpf_ma.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_bpf_ma.c

diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpf_ma.c b/tools/testing/selftests/bpf/prog_tests/test_bpf_ma.c
new file mode 100644
index 000000000000..0cca4e8ae38e
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_bpf_ma.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2023. Huawei Technologies Co., Ltd */
+#define _GNU_SOURCE
+#include <sched.h>
+#include <pthread.h>
+#include <stdbool.h>
+#include <bpf/btf.h>
+#include <test_progs.h>
+
+#include "test_bpf_ma.skel.h"
+
+void test_test_bpf_ma(void)
+{
+	struct test_bpf_ma *skel;
+	struct btf *btf;
+	int i, err;
+
+	skel = test_bpf_ma__open();
+	if (!ASSERT_OK_PTR(skel, "open"))
+		return;
+
+	btf = bpf_object__btf(skel->obj);
+	if (!ASSERT_OK_PTR(btf, "btf"))
+		goto out;
+
+	for (i = 0; i < ARRAY_SIZE(skel->rodata->data_sizes); i++) {
+		char name[32];
+		int id;
+
+		snprintf(name, sizeof(name), "bin_data_%u", skel->rodata->data_sizes[i]);
+		id = btf__find_by_name_kind(btf, name, BTF_KIND_STRUCT);
+		if (!ASSERT_GT(id, 0, "bin_data"))
+			goto out;
+		skel->rodata->data_btf_ids[i] = id;
+	}
+
+	err = test_bpf_ma__load(skel);
+	if (!ASSERT_OK(err, "load"))
+		goto out;
+
+	err = test_bpf_ma__attach(skel);
+	if (!ASSERT_OK(err, "attach"))
+		goto out;
+
+	skel->bss->pid = getpid();
+	usleep(1);
+	ASSERT_OK(skel->bss->err, "test error");
+out:
+	test_bpf_ma__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_bpf_ma.c b/tools/testing/selftests/bpf/progs/test_bpf_ma.c
new file mode 100644
index 000000000000..ecde41ae0fc8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_bpf_ma.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2023. Huawei Technologies Co., Ltd */
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_helpers.h>
+
+#include "bpf_experimental.h"
+#include "bpf_misc.h"
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#endif
+
+struct generic_map_value {
+	void *data;
+};
+
+char _license[] SEC("license") = "GPL";
+
+const unsigned int data_sizes[] = {8, 16, 32, 64, 96, 128, 192, 256, 512, 1024, 2048, 4096};
+const volatile unsigned int data_btf_ids[ARRAY_SIZE(data_sizes)] = {};
+
+int err = 0;
+int pid = 0;
+
+#define DEFINE_ARRAY_WITH_KPTR(_size) \
+	struct bin_data_##_size { \
+		char data[_size - sizeof(void *)]; \
+	}; \
+	struct map_value_##_size { \
+		struct bin_data_##_size __kptr * data; \
+		/* To emit BTF info for bin_data_xx */ \
+		struct bin_data_##_size not_used; \
+	}; \
+	struct { \
+		__uint(type, BPF_MAP_TYPE_ARRAY); \
+		__type(key, int); \
+		__type(value, struct map_value_##_size); \
+		__uint(max_entries, 128); \
+	} array_##_size SEC(".maps");
+
+static __always_inline void batch_alloc_free(struct bpf_map *map, unsigned int batch,
+					     unsigned int idx)
+{
+	struct generic_map_value *value;
+	unsigned int i, key;
+	void *old, *new;
+
+	for (i = 0; i < batch; i++) {
+		key = i;
+		value = bpf_map_lookup_elem(map, &key);
+		if (!value) {
+			err = 1;
+			return;
+		}
+		new = bpf_obj_new_impl(data_btf_ids[idx], NULL);
+		if (!new) {
+			err = 2;
+			return;
+		}
+		old = bpf_kptr_xchg(&value->data, new);
+		if (old) {
+			bpf_obj_drop(old);
+			err = 3;
+			return;
+		}
+	}
+	for (i = 0; i < batch; i++) {
+		key = i;
+		value = bpf_map_lookup_elem(map, &key);
+		if (!value) {
+			err = 4;
+			return;
+		}
+		old = bpf_kptr_xchg(&value->data, NULL);
+		if (!old) {
+			err = 5;
+			return;
+		}
+		bpf_obj_drop(old);
+	}
+}
+
+#define CALL_BATCH_ALLOC_FREE(size, batch, idx) \
+	batch_alloc_free((struct bpf_map *)(&array_##size), batch, idx)
+
+DEFINE_ARRAY_WITH_KPTR(8);
+DEFINE_ARRAY_WITH_KPTR(16);
+DEFINE_ARRAY_WITH_KPTR(32);
+DEFINE_ARRAY_WITH_KPTR(64);
+DEFINE_ARRAY_WITH_KPTR(96);
+DEFINE_ARRAY_WITH_KPTR(128);
+DEFINE_ARRAY_WITH_KPTR(192);
+DEFINE_ARRAY_WITH_KPTR(256);
+DEFINE_ARRAY_WITH_KPTR(512);
+DEFINE_ARRAY_WITH_KPTR(1024);
+DEFINE_ARRAY_WITH_KPTR(2048);
+DEFINE_ARRAY_WITH_KPTR(4096);
+
+SEC("fentry/" SYS_PREFIX "sys_nanosleep")
+int test_bpf_mem_alloc_free(void *ctx)
+{
+	if ((u32)bpf_get_current_pid_tgid() != pid)
+		return 0;
+
+	/* Alloc 128 8-bytes objects in batch to trigger refilling,
+	 * then free 128 8-bytes objects in batch to trigger freeing.
+	 */
+	CALL_BATCH_ALLOC_FREE(8, 128, 0);
+	CALL_BATCH_ALLOC_FREE(16, 128, 1);
+	CALL_BATCH_ALLOC_FREE(32, 128, 2);
+	CALL_BATCH_ALLOC_FREE(64, 128, 3);
+	CALL_BATCH_ALLOC_FREE(96, 128, 4);
+	CALL_BATCH_ALLOC_FREE(128, 128, 5);
+	CALL_BATCH_ALLOC_FREE(192, 128, 6);
+	CALL_BATCH_ALLOC_FREE(256, 128, 7);
+	CALL_BATCH_ALLOC_FREE(512, 64, 8);
+	CALL_BATCH_ALLOC_FREE(1024, 32, 9);
+	CALL_BATCH_ALLOC_FREE(2048, 16, 10);
+	CALL_BATCH_ALLOC_FREE(4096, 8, 11);
+
+	return 0;
+}
-- 
2.29.2

Re: [PATCH bpf 0/4] Fix the unmatched unit_size of bpf_mem_cache

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Fri,  8 Sep 2023 21:39:19 +0800 you wrote:
> From: Hou Tao <houtao1@huawei.com>
> 
> Hi,
> 
> The patchset aims to fix the reported warning [0] when the unit_size of
> bpf_mem_cache is mismatched with the object size of underly slab-cache.
> 
> [...]

Here is the summary with links:
  - [bpf,1/4] bpf: Adjust size_index according to the value of KMALLOC_MIN_SIZE
    https://git.kernel.org/bpf/bpf/c/d52b59315bf5
  - [bpf,2/4] bpf: Don't prefill for unused bpf_mem_cache
    https://git.kernel.org/bpf/bpf/c/b1d53958b693
  - [bpf,3/4] bpf: Ensure unit_size is matched with slab cache object size
    https://git.kernel.org/bpf/bpf/c/c93047255202
  - [bpf,4/4] selftests/bpf: Test all valid alloc sizes for bpf mem allocator
    https://git.kernel.org/bpf/bpf/c/f0a42ab5890f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html