Message-ID: <87a58z3cmn.fsf@intel.com>
Date: Tue, 01 Apr 2025 22:28:16 +0300
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Masahiro Yamada <masahiroy@...nel.org>, Linus Torvalds
 <torvalds@...ux-foundation.org>
Cc: Jason Gunthorpe <jgg@...dia.com>, Dave Airlie <airlied@...il.com>,
 dri-devel <dri-devel@...ts.freedesktop.org>, LKML
 <linux-kernel@...r.kernel.org>
Subject: Re: [git pull] drm for 6.15-rc1

On Wed, 02 Apr 2025, Masahiro Yamada <masahiroy@...nel.org> wrote:
> On Wed, Apr 2, 2025 at 1:12 AM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> On Tue, 1 Apr 2025 at 05:21, Jani Nikula <jani.nikula@...ux.intel.com> wrote:
>> >
>> > The header checks have existed for uapi headers before, including the,
>> > uh, turds, but apparently adding them in drm broke the camel's back.
>>
>> The uapi header test things never caused any problems for me [*],
>> because they don't actually pollute the source tree.
>>
>> Why? Because they all end up in that generated 'usr/include/' directory.
>>
>> So when I look at the source files, filename completion is entirely
>> unaffected, and it all works fine.
>>
>> Look, I can complete something like
>>
>>     include/uapi/asm-generic/poll.h
>>
>> perfectly fine, because there is *not* some generated turd that affects it all.
>>
>> Because for the uapi files those hdrtest files end up being in
>>
>>     ./usr/include/asm-generic/poll.hdrtest
>>
>> and I never have any reason to really look at that subdirectory at
>> all, since it's all generated.
>>
>> Or put another way - if I _were_ to look at it, it would be exactly
>> because I want to see some generated file, in which case the 'hdrtest'
>> turd would be part of it.
>>
>> (Although I cannot recall that ever having actually happened, to be
>> honest - but looking at various header files is common, and I hit the
>> drm case immediately)
>>
>> Would you mind taking more of that uapi approach than creating that
>> hidden directory specific to the drm tree? Maybe this could *all* be
>> generalized?
>>
>>            Linus
>>
>> [*] I say "never caused any problems for me", but maybe it did way in
>> the past and it was fixed and I just don't recall. I have definitely
>> complained about pathname completion issues to people before.
>
> I know you dislike this, and I NACKed this before (but too late):
> https://lore.kernel.org/dri-devel/CAK7LNATHXwEkjJHP7b-ZmhzLfyyuOdsyimna-=r-sJk+DxigrA@mail.gmail.com/
>
> Compile-testing UAPI headers is useful
> (and I believe this is the only case where it is useful)
> because userspace is independent of any CONFIG option,
> and we have no control over the include order in
> userspace programs.
>
> In contrast, kernel-space headers are conditionally parsed,
> depending on CONFIG options.
>
> You dislike this feature for the reason of TAB-completion,
> but I am negative about this feature from another perspective.
>
> We cannot avoid a false-positive:
> https://lore.kernel.org/all/20190718130835.GA28520@lst.de/
>
> It is not <drm/*.h> but <linux/*.h>
> However, it is annoying to make every header self-contained
> "just because we are checking this".

As I explained earlier in the thread, it's not *just* about the headers
being self-contained. It's *also* about checking header guards and
kernel-doc, maybe other things in the future.

If it's possible to do opt-in build checks for this, and catch all of
these pre-merge, why shouldn't we? We can catch a whole class of build
issues and bypass the subsequent fixes with this, and make life easier
for us.

> Actually, it is not a real problem when the relevant
> CONFIG option is disabled.
> I did not interrupt him because he was doing this
> checkunder drivers/gpu/drm/i915/.
> ()
> But, I am opposed to expanding it to more-global include/drm/,
> or any other subsystem.
>
> In my opinion, the right fix is
> "git revert 62ae45687e43"
>
>
> If we continue this, maybe leave a caution
> in capital letters?

Or maybe let the subsystem and driver maintainers decide?

I don't think there's a *single* header under include/drm where them
being self-contained depends on a config option. I'm sure there are
plenty in include/linux, but I wouldn't say they *all* do.

Maybe help and support us in generalizing this in scripts/Makefile.build
to avoid the boilerplate, and make Linus happy, instead of trying to
block us from having nice things? Please?


BR,
Jani.


>
>
> diff --git a/include/Kbuild b/include/Kbuild
> index 5e76a599e2dd..4dff23ae51a4 100644
> --- a/include/Kbuild
> +++ b/include/Kbuild
> @@ -1 +1,3 @@
> +# The drm subsystem is strict about the self-containedness of header files.
> +# OTHER SUBSYSTEMS SHOULD NOT FOLLOW THIS PRACTICE.
>  obj-$(CONFIG_DRM_HEADER_TEST)  += drm/
>
>
>
> --
> Best Regards
>
> Masahiro Yamada

-- 
Jani Nikula, Intel

Powered by blists - more mailing lists