| Message ID | 20240512075909.2688-2-Arunpravin.PaneerSelvam@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] drm/buddy: Fix the range bias clear memory allocation issue | expand |
On 12/05/2024 08:59, Arunpravin Paneer Selvam wrote: > Allocate cleared blocks in the bias range when the DRM > buddy's clear avail is zero. This will validate the bias > range allocation in scenarios like system boot when no > cleared blocks are available and exercise the fallback > path too. The resulting blocks should always be dirty. > > Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> > --- > drivers/gpu/drm/tests/drm_buddy_test.c | 35 ++++++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > > diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c > index e3b50e240d36..a194f271bc55 100644 > --- a/drivers/gpu/drm/tests/drm_buddy_test.c > +++ b/drivers/gpu/drm/tests/drm_buddy_test.c > @@ -26,6 +26,8 @@ static void drm_test_buddy_alloc_range_bias(struct kunit *test) > u32 mm_size, ps, bias_size, bias_start, bias_end, bias_rem; > DRM_RND_STATE(prng, random_seed); > unsigned int i, count, *order; > + struct drm_buddy_block *block; > + unsigned long flags; > struct drm_buddy mm; > LIST_HEAD(allocated); > > @@ -222,6 +224,39 @@ static void drm_test_buddy_alloc_range_bias(struct kunit *test) > > drm_buddy_free_list(&mm, &allocated, 0); > drm_buddy_fini(&mm); > + > + /* > + * Allocate cleared blocks in the bias range when the DRM buddy's clear avail is > + * zero. This will validate the bias range allocation in scenarios like system boot > + * when no cleared blocks are available and exercise the fallback path too. The resulting > + * blocks should always be dirty. > + */ > + > + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, ps), > + "buddy_init failed\n"); > + mm.clear_avail = 0; Should already be zero, right? Maybe make this an assert instead? > + > + bias_start = round_up(prandom_u32_state(&prng) % (mm_size - ps), ps); > + bias_end = round_up(bias_start + prandom_u32_state(&prng) % (mm_size - bias_start), ps); > + bias_end = max(bias_end, bias_start + ps); > + bias_rem = bias_end - bias_start; > + > + flags = DRM_BUDDY_CLEAR_ALLOCATION | DRM_BUDDY_RANGE_ALLOCATION; > + u32 size = max(round_up(prandom_u32_state(&prng) % bias_rem, ps), ps); u32 declaration should be moved to above? Otherwise, Reviewed-by: Matthew Auld <matthew.auld@intel.com> > + > + KUNIT_ASSERT_FALSE_MSG(test, > + drm_buddy_alloc_blocks(&mm, bias_start, > + bias_end, size, ps, > + &allocated, > + flags), > + "buddy_alloc failed with bias(%x-%x), size=%u, ps=%u\n", > + bias_start, bias_end, size, ps); > + > + list_for_each_entry(block, &allocated, link) > + KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false); > + > + drm_buddy_free_list(&mm, &allocated, 0); > + drm_buddy_fini(&mm); > } > > static void drm_test_buddy_alloc_clear(struct kunit *test)
Hi Matthew, On 5/13/2024 1:49 PM, Matthew Auld wrote: > On 12/05/2024 08:59, Arunpravin Paneer Selvam wrote: >> Allocate cleared blocks in the bias range when the DRM >> buddy's clear avail is zero. This will validate the bias >> range allocation in scenarios like system boot when no >> cleared blocks are available and exercise the fallback >> path too. The resulting blocks should always be dirty. >> >> Signed-off-by: Arunpravin Paneer Selvam >> <Arunpravin.PaneerSelvam@amd.com> >> --- >> drivers/gpu/drm/tests/drm_buddy_test.c | 35 ++++++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c >> b/drivers/gpu/drm/tests/drm_buddy_test.c >> index e3b50e240d36..a194f271bc55 100644 >> --- a/drivers/gpu/drm/tests/drm_buddy_test.c >> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c >> @@ -26,6 +26,8 @@ static void drm_test_buddy_alloc_range_bias(struct >> kunit *test) >> u32 mm_size, ps, bias_size, bias_start, bias_end, bias_rem; >> DRM_RND_STATE(prng, random_seed); >> unsigned int i, count, *order; >> + struct drm_buddy_block *block; >> + unsigned long flags; >> struct drm_buddy mm; >> LIST_HEAD(allocated); >> @@ -222,6 +224,39 @@ static void >> drm_test_buddy_alloc_range_bias(struct kunit *test) >> drm_buddy_free_list(&mm, &allocated, 0); >> drm_buddy_fini(&mm); >> + >> + /* >> + * Allocate cleared blocks in the bias range when the DRM >> buddy's clear avail is >> + * zero. This will validate the bias range allocation in >> scenarios like system boot >> + * when no cleared blocks are available and exercise the >> fallback path too. The resulting >> + * blocks should always be dirty. >> + */ >> + >> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, ps), >> + "buddy_init failed\n"); >> + mm.clear_avail = 0; > > Should already be zero, right? Maybe make this an assert instead? No, since the mm declared as a local variable in the test case, mm.clear_avail is not zero. > >> + >> + bias_start = round_up(prandom_u32_state(&prng) % (mm_size - ps), >> ps); >> + bias_end = round_up(bias_start + prandom_u32_state(&prng) % >> (mm_size - bias_start), ps); >> + bias_end = max(bias_end, bias_start + ps); >> + bias_rem = bias_end - bias_start; >> + >> + flags = DRM_BUDDY_CLEAR_ALLOCATION | DRM_BUDDY_RANGE_ALLOCATION; >> + u32 size = max(round_up(prandom_u32_state(&prng) % bias_rem, >> ps), ps); > > u32 declaration should be moved to above? Sure. Thanks, Arun. > > Otherwise, > Reviewed-by: Matthew Auld <matthew.auld@intel.com> > >> + >> + KUNIT_ASSERT_FALSE_MSG(test, >> + drm_buddy_alloc_blocks(&mm, bias_start, >> + bias_end, size, ps, >> + &allocated, >> + flags), >> + "buddy_alloc failed with bias(%x-%x), size=%u, >> ps=%u\n", >> + bias_start, bias_end, size, ps); >> + >> + list_for_each_entry(block, &allocated, link) >> + KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false); >> + >> + drm_buddy_free_list(&mm, &allocated, 0); >> + drm_buddy_fini(&mm); >> } >> static void drm_test_buddy_alloc_clear(struct kunit *test)
On 13/05/2024 16:11, Paneer Selvam, Arunpravin wrote: > Hi Matthew, > > On 5/13/2024 1:49 PM, Matthew Auld wrote: >> On 12/05/2024 08:59, Arunpravin Paneer Selvam wrote: >>> Allocate cleared blocks in the bias range when the DRM >>> buddy's clear avail is zero. This will validate the bias >>> range allocation in scenarios like system boot when no >>> cleared blocks are available and exercise the fallback >>> path too. The resulting blocks should always be dirty. >>> >>> Signed-off-by: Arunpravin Paneer Selvam >>> <Arunpravin.PaneerSelvam@amd.com> >>> --- >>> drivers/gpu/drm/tests/drm_buddy_test.c | 35 ++++++++++++++++++++++++++ >>> 1 file changed, 35 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c >>> b/drivers/gpu/drm/tests/drm_buddy_test.c >>> index e3b50e240d36..a194f271bc55 100644 >>> --- a/drivers/gpu/drm/tests/drm_buddy_test.c >>> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c >>> @@ -26,6 +26,8 @@ static void drm_test_buddy_alloc_range_bias(struct >>> kunit *test) >>> u32 mm_size, ps, bias_size, bias_start, bias_end, bias_rem; >>> DRM_RND_STATE(prng, random_seed); >>> unsigned int i, count, *order; >>> + struct drm_buddy_block *block; >>> + unsigned long flags; >>> struct drm_buddy mm; >>> LIST_HEAD(allocated); >>> @@ -222,6 +224,39 @@ static void >>> drm_test_buddy_alloc_range_bias(struct kunit *test) >>> drm_buddy_free_list(&mm, &allocated, 0); >>> drm_buddy_fini(&mm); >>> + >>> + /* >>> + * Allocate cleared blocks in the bias range when the DRM >>> buddy's clear avail is >>> + * zero. This will validate the bias range allocation in >>> scenarios like system boot >>> + * when no cleared blocks are available and exercise the >>> fallback path too. The resulting >>> + * blocks should always be dirty. >>> + */ >>> + >>> + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, ps), >>> + "buddy_init failed\n"); >>> + mm.clear_avail = 0; >> >> Should already be zero, right? Maybe make this an assert instead? > No, since the mm declared as a local variable in the test case, > mm.clear_avail is not zero. That sounds like a bug IMO. The init() should initialize it, like it does for mm.avail and everything else. >> >>> + >>> + bias_start = round_up(prandom_u32_state(&prng) % (mm_size - ps), >>> ps); >>> + bias_end = round_up(bias_start + prandom_u32_state(&prng) % >>> (mm_size - bias_start), ps); >>> + bias_end = max(bias_end, bias_start + ps); >>> + bias_rem = bias_end - bias_start; >>> + >>> + flags = DRM_BUDDY_CLEAR_ALLOCATION | DRM_BUDDY_RANGE_ALLOCATION; >>> + u32 size = max(round_up(prandom_u32_state(&prng) % bias_rem, >>> ps), ps); >> >> u32 declaration should be moved to above? > Sure. > > Thanks, > Arun. >> >> Otherwise, >> Reviewed-by: Matthew Auld <matthew.auld@intel.com> >> >>> + >>> + KUNIT_ASSERT_FALSE_MSG(test, >>> + drm_buddy_alloc_blocks(&mm, bias_start, >>> + bias_end, size, ps, >>> + &allocated, >>> + flags), >>> + "buddy_alloc failed with bias(%x-%x), size=%u, >>> ps=%u\n", >>> + bias_start, bias_end, size, ps); >>> + >>> + list_for_each_entry(block, &allocated, link) >>> + KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false); >>> + >>> + drm_buddy_free_list(&mm, &allocated, 0); >>> + drm_buddy_fini(&mm); >>> } >>> static void drm_test_buddy_alloc_clear(struct kunit *test) >
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c index e3b50e240d36..a194f271bc55 100644 --- a/drivers/gpu/drm/tests/drm_buddy_test.c +++ b/drivers/gpu/drm/tests/drm_buddy_test.c @@ -26,6 +26,8 @@ static void drm_test_buddy_alloc_range_bias(struct kunit *test) u32 mm_size, ps, bias_size, bias_start, bias_end, bias_rem; DRM_RND_STATE(prng, random_seed); unsigned int i, count, *order; + struct drm_buddy_block *block; + unsigned long flags; struct drm_buddy mm; LIST_HEAD(allocated); @@ -222,6 +224,39 @@ static void drm_test_buddy_alloc_range_bias(struct kunit *test) drm_buddy_free_list(&mm, &allocated, 0); drm_buddy_fini(&mm); + + /* + * Allocate cleared blocks in the bias range when the DRM buddy's clear avail is + * zero. This will validate the bias range allocation in scenarios like system boot + * when no cleared blocks are available and exercise the fallback path too. The resulting + * blocks should always be dirty. + */ + + KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_init(&mm, mm_size, ps), + "buddy_init failed\n"); + mm.clear_avail = 0; + + bias_start = round_up(prandom_u32_state(&prng) % (mm_size - ps), ps); + bias_end = round_up(bias_start + prandom_u32_state(&prng) % (mm_size - bias_start), ps); + bias_end = max(bias_end, bias_start + ps); + bias_rem = bias_end - bias_start; + + flags = DRM_BUDDY_CLEAR_ALLOCATION | DRM_BUDDY_RANGE_ALLOCATION; + u32 size = max(round_up(prandom_u32_state(&prng) % bias_rem, ps), ps); + + KUNIT_ASSERT_FALSE_MSG(test, + drm_buddy_alloc_blocks(&mm, bias_start, + bias_end, size, ps, + &allocated, + flags), + "buddy_alloc failed with bias(%x-%x), size=%u, ps=%u\n", + bias_start, bias_end, size, ps); + + list_for_each_entry(block, &allocated, link) + KUNIT_EXPECT_EQ(test, drm_buddy_block_is_clear(block), false); + + drm_buddy_free_list(&mm, &allocated, 0); + drm_buddy_fini(&mm); } static void drm_test_buddy_alloc_clear(struct kunit *test)
Allocate cleared blocks in the bias range when the DRM buddy's clear avail is zero. This will validate the bias range allocation in scenarios like system boot when no cleared blocks are available and exercise the fallback path too. The resulting blocks should always be dirty. Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> --- drivers/gpu/drm/tests/drm_buddy_test.c | 35 ++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)