Hi Krzysztof,

On Wed, 26 Aug 2026 at 18:39, Krzysztof Niemiec
<[email protected]> wrote:
> I got unnecessarily annoyed/angry in my previous reply, I'm sorry for that.
> It clouded my judgement a bit then but I still have some comments now
>
> On 2026-08-25 at 14:15:34 +0200, Geert Uytterhoeven wrote:
> > On Tue, 25 Aug 2026 at 13:39, Krzysztof Niemiec
> > <[email protected]> wrote:
> > > On 2026-08-24 at 17:12:30 +0200, Geert Uytterhoeven wrote:
> > > > Enabling a (modular) test should not silently enable additional kernel
> > > > functionality, as that may increase the attack vector for a product.
> > >
> > > What vector? What product, exactly? As far as I know buddy allocator code 
> > > is
> >
> > Any product that uses DRM and has CONFIG_KUNIT_ALL_TESTS=m, so
> > developers can easily run any relevant test when the need arises.
> >
> > > currently loaded when a) loading a drm driver (which would need that code
> >
> > Not just "a drm driver": very few DRM drivers use it.
> > Hence one can have a product with DRM that does not need the buddy
> > allocator.
>
> And the buddy allocator was moved out of DRM in anticipation of a
> possible non-DRM driver that wants the buddy allocator. So there's no
> implication between DRM and buddy in either way anymore.
>
> Currently it's only DRM drivers that actually use it, but there's already
> scaffolding in rust for it that does not depend on DRM, and once
> there's a non-DRM driver that wants to use the allocator it would have
> to undo the dependency added in this patch anyway.

Then the dependency can be lifted.  There are hundreds (thousands?)
of Kconfig symbols that will need to get heir dependencies updated
one day.
However, missing dependencies means more questions for all users
configuring kernels.

> > > > --- a/drivers/gpu/Kconfig
> > > > +++ b/drivers/gpu/Kconfig
> > > > @@ -1,14 +1,15 @@
> > > >  # SPDX-License-Identifier: GPL-2.0
> > > >
> > > >  config GPU_BUDDY
> > > > -       bool
> > > > +       bool "Page based buddy allocator for GPU memory" if KUNIT
> > > > +       depends on DRM || COMPILE_TEST
> > >
> > > What's the justification for COMPILE_TEST here? It might be needed but
> > > you didn't explain why or mention it in the patch description at all.
> >
> > It is customary to add "|| COMPILE_TEST" when adding dependencies that
> > are not strictly needed for a successful build.
>
> I can't tell if this is something so obvious it needs no comment; I feel
> like it should be documented explicitly in the commit log.
>
> If the DRM dependency would be dropped would the COMPILE_TEST have to
> stay? Because then we're forcing a non-drm user to enable it if they want
> to use the buddy alloc at all.

If the DRM dependency would be dropped, the COMPILE_TEST can be
dropped too.
If a (not always enabled) non-drm user appears, it can be added to the
dependency list.

> I think just leaving the "if KUNIT" part would check all the boxes,
> since the tests are actually runnable with KUnit via .kunitconfig
> without building an entire DRM driver, GPU_BUDDY also won't be randomly
> enabled with KUNIT_ALL_TESTS on systems that don't care about it, and
> we're not forcing anyone to enable DRM on a feature that was explictly
> made independent from it.

There are systems that do not use DRM, but still want tests enabled.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Reply via email to