On Fri, 24 Apr 2026, Albert Esteve <[email protected]> wrote: > On Tue, Apr 21, 2026 at 1:51 PM Jani Nikula <[email protected]> wrote: >> >> On Tue, 21 Apr 2026, Albert Esteve <[email protected]> wrote: >> > On Mon, Apr 20, 2026 at 4:47 PM Peter Zijlstra <[email protected]> >> > wrote: >> >> >> >> On Mon, Apr 20, 2026 at 02:28:06PM +0200, Albert Esteve wrote: >> >> > From: Guenter Roeck <[email protected]> >> >> > >> >> > The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests >> >> > intentionally trigger warning backtraces by providing bad parameters to >> >> > the tested functions. What is tested is the return value, not the >> >> > existence >> >> > of a warning backtrace. Suppress the backtraces to avoid clogging the >> >> > kernel log and distraction from real problems. >> >> > >> >> > Tested-by: Linux Kernel Functional Testing <[email protected]> >> >> > Acked-by: Dan Carpenter <[email protected]> >> >> > Acked-by: Maíra Canal <[email protected]> >> >> > Cc: Maarten Lankhorst <[email protected]> >> >> > Cc: David Airlie <[email protected]> >> >> > Cc: Daniel Vetter <[email protected]> >> >> > Signed-off-by: Guenter Roeck <[email protected]> >> >> > Signed-off-by: Alessandro Carminati <[email protected]> >> >> > Signed-off-by: Albert Esteve <[email protected]> >> >> > --- >> >> > drivers/gpu/drm/tests/drm_rect_test.c | 14 ++++++++++++++ >> >> > 1 file changed, 14 insertions(+) >> >> > >> >> > diff --git a/drivers/gpu/drm/tests/drm_rect_test.c >> >> > b/drivers/gpu/drm/tests/drm_rect_test.c >> >> > index 17e1f34b76101..1dd7d819165e7 100644 >> >> > --- a/drivers/gpu/drm/tests/drm_rect_test.c >> >> > +++ b/drivers/gpu/drm/tests/drm_rect_test.c >> >> > @@ -409,8 +409,15 @@ static void drm_test_rect_calc_hscale(struct kunit >> >> > *test) >> >> > const struct drm_rect_scale_case *params = test->param_value; >> >> > int scaling_factor; >> >> > >> >> > + /* >> >> > + * drm_rect_calc_hscale() generates a warning backtrace whenever >> >> > bad >> >> > + * parameters are passed to it. This affects all unit tests with >> >> > an >> >> > + * error code in expected_scaling_factor. >> >> > + */ >> >> > + KUNIT_START_SUPPRESSED_WARNING(test); >> >> > scaling_factor = drm_rect_calc_hscale(¶ms->src, ¶ms->dst, >> >> > params->min_range, >> >> > params->max_range); >> >> > + KUNIT_END_SUPPRESSED_WARNING(test); >> >> >> >> Would not something like: >> >> >> >> scoped_kunit_suppress() { >> >> scaling_factor = drm_rect_calc_hscale(¶ms->src, >> >> ¶ms->dst, >> >> params->min_range, >> >> params->max_range); >> >> } >> >> >> >> be better? >> > >> > Since KUnit already has a few macros in its API it didn't occur to me. >> > Good idea, I like it. And I guess the scope approach matches well with >> > your __cleanup comment in the first patch. If no one opposes, I will >> > work toward that pattern for the next version. >> >> There's a catch with kunit and __cleanup and thus (scoped) guards. Kunit >> runs in ktreads, asserts lead to kthread_exit() and the __cleanup won't >> be called. > > Hi Jani, > > Good point. In this specific case, the actual cleanup is handled by > kunit_add_action_or_reset(), so __cleanup not firing on assert is > harmless.
Right. > >> >> Warning suppression being part of kunit infrastructure, asserts can and >> should end the suppression too. But setting the example (scoped) guards >> are safe in kunit tests in general feels like a trap waiting to happen. >> > > ... but I agree it sets a misleading precedent. I'll stick with the > explicit start/end API, then? Or maybe we can clearly document why the > scoped approach is safe in this case and use it. I don't know what the right approach is, just wanted to make sure you're aware of this particular gotcha with guards and kunit. BR, Jani. > >> >> BR, >> Jani. >> >> >> > >> >> >> >> Also, how can you stand all this screaming in the code? >> >> >> > >> > Again, KUnit already contains many macros, so this use didn't register >> > as such. Now I will not be able to unsee it. >> > >> > >> >> -- >> Jani Nikula, Intel >> > -- Jani Nikula, Intel
