[PATCH 1/2] drm_buddy: fix power-of-2 rounding errs

Jim Cromie posted 2 patches 1 month ago
[PATCH 1/2] drm_buddy: fix power-of-2 rounding errs
Posted by Jim Cromie 1 month ago
On DRM-CI, I encountered this kunit:arm32 failure.

[23:19:40] [PASSED] drm_test_buddy_alloc_clear
[23:19:40] [PASSED] drm_test_buddy_alloc_range_bias
[23:19:41] [PASSED] drm_test_buddy_fragmentation_performance
[23:19:41]     # drm_test_buddy_alloc_exceeds_max_order: EXPECTATION FAILED at drivers/gpu/drm/tests/drm_buddy_test.c:889
[23:19:41]     Expected err == -22, but
[23:19:41]         err == 0 (0x0)
[23:19:41] ------------[ cut here ]------------
[23:19:41] WARNING: drivers/gpu/drm/drm_buddy.c:405 at drm_buddy_fini+0x114/0x1b8, CPU#0: kunit_try_catch/74
[23:19:41] CPU: 0 UID: 0 PID: 74 Comm: kunit_try_catch Tainted: G                 N  7.0.0-rc1-gdfb0bcedd08a #1 VOLUNTARY
[23:19:41] Tainted: [N]=TEST
[23:19:41] Hardware name: Generic DT based system
[23:19:41] Call trace:
[23:19:41]  unwind_backtrace from show_stack+0x10/0x14
[23:19:41]  show_stack from dump_stack_lvl+0x3c/0x4c
[23:19:41]  dump_stack_lvl from __warn+0xe8/0x1c4
[23:19:41]  __warn from warn_slowpath_fmt+0xa4/0xc0
[23:19:41]  warn_slowpath_fmt from drm_buddy_fini+0x114/0x1b8
[23:19:41]  drm_buddy_fini from drm_test_buddy_alloc_exceeds_max_order+0x1c8/0x36c
[23:19:41]  drm_test_buddy_alloc_exceeds_max_order from kunit_try_run_case+0x78/0x1c8
[23:19:41]  kunit_try_run_case from kunit_generic_run_threadfn_adapter+0x1c/0x34
[23:19:41]  kunit_generic_run_threadfn_adapter from kthread+0x108/0x134
[23:19:41]  kthread from ret_from_fork+0x14/0x28
[23:19:41] Exception stack(0xf0bd5fb0 to 0xf0bd5ff8)
[23:19:41] 5fa0:                                     00000000 00000000 00000000 00000000
[23:19:41] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[23:19:41] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[23:19:41] ---[ end trace 0000000000000000 ]---
[23:19:41]     # drm_test_buddy_alloc_exceeds_max_order: drivers/gpu/drm/drm_buddy.c:406: buddy_fini() root
[23:19:41] ------------[ cut here ]------------
[23:19:41] WARNING: drivers/gpu/drm/drm_buddy.c:414 at drm_buddy_fini+0x1b4/0x1b8, CPU#0: kunit_try_catch/74
[23:19:41] CPU: 0 UID: 0 PID: 74 Comm: kunit_try_catch Tainted: G        W        N  7.0.0-rc1-gdfb0bcedd08a #1 VOLUNTARY
[23:19:41] Tainted: [W]=WARN, [N]=TEST
[23:19:41] Hardware name: Generic DT based system
[23:19:41] Call trace:
[23:19:41]  unwind_backtrace from show_stack+0x10/0x14
[23:19:41]  show_stack from dump_stack_lvl+0x3c/0x4c
[23:19:41]  dump_stack_lvl from __warn+0xe8/0x1c4
[23:19:41]  __warn from warn_slowpath_fmt+0xa4/0xc0
[23:19:41]  warn_slowpath_fmt from drm_buddy_fini+0x1b4/0x1b8
[23:19:41]  drm_buddy_fini from drm_test_buddy_alloc_exceeds_max_order+0x1c8/0x36c
[23:19:41]  drm_test_buddy_alloc_exceeds_max_order from kunit_try_run_case+0x78/0x1c8
[23:19:41]  kunit_try_run_case from kunit_generic_run_threadfn_adapter+0x1c/0x34
[23:19:41]  kunit_generic_run_threadfn_adapter from kthread+0x108/0x134
[23:19:41]  kthread from ret_from_fork+0x14/0x28
[23:19:41] Exception stack(0xf0bd5fb0 to 0xf0bd5ff8)
[23:19:41] 5fa0:                                     00000000 00000000 00000000 00000000
[23:19:41] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[23:19:41] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[23:19:41] ---[ end trace 0000000000000000 ]---
[23:19:41] [FAILED] drm_test_buddy_alloc_exceeds_max_order
[23:19:41]     # drm_buddy: Testing DRM buddy manager, with random_seed=0xacce106c
[23:19:41]     # module: drm_buddy_test
[23:19:41] # drm_buddy: pass:8 fail:1 skip:0 total:9

This error in drm_test_buddy_alloc_exceeds_max_order (Expected -22,
got 0) is caused by a 64-bit truncation bug in the DRM buddy
allocator's core logic.

On 32-bit architectures (like arm32), the standard
roundup_pow_of_two() and rounddown_pow_of_two() macros use unsigned
long internally. When these are called with u64 memory sizes (as
drm_buddy.c does), any size above 4GB is truncated.

The test tries to allocate ~9GB on a ~10GB manager.  Due to
truncation, it actually asks for ~1GB on a ~2GB manager, which
unexpectedly succeeds.  This then causes a mm->avail mismatch warning
when the test finishes.

Use 64-bit safe power-of-two calculations. Since ilog2() is already
safe for u64, I'll use it to implement safe rounding.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 drivers/gpu/drm/drm_buddy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index dbf984f8e301..8f23fb615d47 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -919,7 +919,7 @@ static int __alloc_contig_try_harder(struct drm_buddy *mm,
 	u64 modify_size;
 	int err;
 
-	modify_size = rounddown_pow_of_two(size);
+	modify_size = 1ULL << ilog2(size);
 	pages = modify_size >> ilog2(mm->chunk_size);
 	order = fls(pages) - 1;
 	if (order == 0)
@@ -1140,7 +1140,7 @@ int drm_buddy_alloc_blocks(struct drm_buddy *mm,
 
 	/* Roundup the size to power of 2 */
 	if (flags & DRM_BUDDY_CONTIGUOUS_ALLOCATION) {
-		size = roundup_pow_of_two(size);
+		size = 1ULL << (ilog2(size - 1) + 1);
 		min_block_size = size;
 	/* Align size value to min_block_size */
 	} else if (!IS_ALIGNED(size, min_block_size)) {
-- 
2.53.0