[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAK7LNAQThGkgtKgquRPv8Ysi_omedRthF1_++apKda-xWeWcbA@mail.gmail.com>
Date: Wed, 2 Apr 2025 03:46:34 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>, 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, 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".
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?
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
Powered by blists - more mailing lists