Hi all,
After merging the amdgpu tree, today's linux-next build (x86_64
allmodconfig) failed like this:
ERROR: modpost: "get_mm_exe_file" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
Caused by commit
f4f80155e6e8 ("amd/display: only require overlay plane to cover whole CRTC on ChromeOS")
I have used the amdgpu tree from next-20211007 for today.
--
Cheers,
Stephen Rothwell
On Friday, October 8th, 2021 at 02:31, Stephen Rothwell <[email protected]> wrote:
> Hi all,
>
> After merging the amdgpu tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> ERROR: modpost: "get_mm_exe_file" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
>
> Caused by commit
>
> f4f80155e6e8 ("amd/display: only require overlay plane to cover whole CRTC on ChromeOS")
>
> I have used the amdgpu tree from next-20211007 for today.
Not sure why this would happen. This commit builds fine on my machine, and I
don't think it's possible to disable this function with Kconfig?
Hi Simon,
On Fri, 08 Oct 2021 06:27:05 +0000 Simon Ser <[email protected]> wrote:
>
> On Friday, October 8th, 2021 at 02:31, Stephen Rothwell <[email protected]> wrote:
>
> > Hi all,
> >
> > After merging the amdgpu tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > ERROR: modpost: "get_mm_exe_file" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
> >
> > Caused by commit
> >
> > f4f80155e6e8 ("amd/display: only require overlay plane to cover whole CRTC on ChromeOS")
> >
> > I have used the amdgpu tree from next-20211007 for today.
>
> Not sure why this would happen. This commit builds fine on my machine, and I
> don't think it's possible to disable this function with Kconfig?
That symbol (get_mm_exe_file) is not exported to modules.
--
Cheers,
Stephen Rothwell
On Friday, October 8th, 2021 at 10:29, Stephen Rothwell <[email protected]> wrote:
> That symbol (get_mm_exe_file) is not exported to modules.
I see this:
EXPORT_SYMBOL(get_mm_exe_file);
in kernel/fork.c
On Fri, Oct 8, 2021 at 5:22 AM Simon Ser <[email protected]> wrote:
>
> On Friday, October 8th, 2021 at 10:29, Stephen Rothwell <[email protected]> wrote:
>
> > That symbol (get_mm_exe_file) is not exported to modules.
>
> I see this:
>
> EXPORT_SYMBOL(get_mm_exe_file);
>
> in kernel/fork.c
Was recently removed:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/kernel/fork.c?id=05da8113c9ba63a8913e6c73dc06ed01cae55f68
I guess we need to rethink that patch.
Alex
On Friday, October 8th, 2021 at 16:11, Alex Deucher <[email protected]> wrote:
> On Fri, Oct 8, 2021 at 5:22 AM Simon Ser [email protected] wrote:
>
> > On Friday, October 8th, 2021 at 10:29, Stephen Rothwell [email protected] wrote:
> >
> > > That symbol (get_mm_exe_file) is not exported to modules.
> >
> > I see this:
> >
> > EXPORT_SYMBOL(get_mm_exe_file);
> >
> >
> > in kernel/fork.c
>
> Was recently removed:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/kernel/fork.c?id=05da8113c9ba63a8913e6c73dc06ed01cae55f68
>
> I guess we need to rethink that patch.
CC Christoph
Would it be reasonable to re-export get_mm_exe_file? amdgpu uses it here:
https://gitlab.freedesktop.org/agd5f/linux/-/commit/0d4da915c7098eca2aa6f559f42e33b5e9c7c5e8
Hi Simon,
On Fri, 08 Oct 2021 09:22:16 +0000 Simon Ser <[email protected]> wrote:
>
> On Friday, October 8th, 2021 at 10:29, Stephen Rothwell <[email protected]> wrote:
>
> > That symbol (get_mm_exe_file) is not exported to modules.
>
> I see this:
>
> EXPORT_SYMBOL(get_mm_exe_file);
>
> in kernel/fork.c
That was remove by commit
05da8113c9ba ("kernel/fork.c: unexport get_{mm,task}_exe_file")
which is in v5.15-rc1.
--
Cheers,
Stephen Rothwell
On Fri, Oct 08, 2021 at 06:07:33PM +0000, Simon Ser wrote:
> Would it be reasonable to re-export get_mm_exe_file? amdgpu uses it here:
>
> https://gitlab.freedesktop.org/agd5f/linux/-/commit/0d4da915c7098eca2aa6f559f42e33b5e9c7c5e8
Seriously? No, it obviously not. Unexporting it is important to catch
utter crap like in that commit which should have never made it into a
maintainer tree.
On Monday, October 11th, 2021 at 09:33, Christoph Hellwig <[email protected]> wrote:
> On Fri, Oct 08, 2021 at 06:07:33PM +0000, Simon Ser wrote:
>
> > Would it be reasonable to re-export get_mm_exe_file? amdgpu uses it here:
> >
> > https://gitlab.freedesktop.org/agd5f/linux/-/commit/0d4da915c7098eca2aa6f559f42e33b5e9c7c5e8
>
> Seriously? No, it obviously not. Unexporting it is important to catch
> utter crap like in that commit which should have never made it into a
> maintainer tree.
I don't understand. Can you elaborate why you think this commit is
"utter crap"?
I'd also appreciate if you could be a bit less aggressive. There's
nothing "obvious" about this from my point of view.
On Mon, Oct 11, 2021 at 07:39:52AM +0000, Simon Ser wrote:
> I don't understand. Can you elaborate why you think this commit is
> "utter crap"?
A kernel driver has absolutely no business making decissions based
on current->comm, which can be changed by any userspace process. This
is kernel programming 101.
Independent of that a check for a specific program as the callers makes
no sense whatsoever as a given program and change over time. This is
not even something kernel specific but something that ever software
engineer should do.
> I'd also appreciate if you could be a bit less aggressive. There's
> nothing "obvious" about this from my point of view.
I'm not agressive. I'm just really disappointed by the amoubt of crap
that gets shovelled into the kernel and even more disappointed by the
abslutely lack of knowledge of some of the contributors.
On Mon, Oct 11, 2021 at 07:49:44AM +0000, Simon Ser wrote:
> Have you heard about the kernel no-regression rule? Here, we can't enable a new
> feature because that would regress user-space which mis-uses the kernel uAPI.
Then you can't enable the feature without an explicit opt-in from
userspace. This ain't rocket science.
> If your reply wasn't aggressive, I don't know what it is.
If there is one thing I find agressive it is your extreme passive
aggressive behavior.
On Monday, October 11th, 2021 at 09:51, Christoph Hellwig <[email protected]> wrote:
> On Mon, Oct 11, 2021 at 07:49:44AM +0000, Simon Ser wrote:
> > Have you heard about the kernel no-regression rule? Here, we can't enable a new
> > feature because that would regress user-space which mis-uses the kernel uAPI.
>
> Then you can't enable the feature without an explicit opt-in from
> userspace. This ain't rocket science.
No, we can't have a "I_AM_NOT_BROKEN" ioctl for each and every uAPI mis-use.
User-space detection has been determined to be the best course of action.
> > If your reply wasn't aggressive, I don't know what it is.
>
> If there is one thing I find agressive it is your extreme passive
> aggressive behavior.
Come on. I'm just asking you to be civil, that's all. But that seems too much.
* * *
I guess I'll just inline these functions in the driver then, if a revert will
be NACK'ed by you?
On Monday, October 11th, 2021 at 09:43, Christoph Hellwig <[email protected]> wrote:
> On Mon, Oct 11, 2021 at 07:39:52AM +0000, Simon Ser wrote:
> > I don't understand. Can you elaborate why you think this commit is
> > "utter crap"?
>
> A kernel driver has absolutely no business making decissions based
> on current->comm, which can be changed by any userspace process. This
> is kernel programming 101.
>
> Independent of that a check for a specific program as the callers makes
> no sense whatsoever as a given program and change over time. This is
> not even something kernel specific but something that ever software
> engineer should do.
Have you heard about the kernel no-regression rule? Here, we can't enable a new
feature because that would regress user-space which mis-uses the kernel uAPI.
This isn't unheard of. Core drm already detects Xorg with current->comm, and
force-disables atomic KMS.
> > I'd also appreciate if you could be a bit less aggressive. There's
> > nothing "obvious" about this from my point of view.
>
> I'm not agressive. I'm just really disappointed by the amoubt of crap
> that gets shovelled into the kernel and even more disappointed by the
> abslutely lack of knowledge of some of the contributors.
If your reply wasn't aggressive, I don't know what it is.
On Monday, October 11th, 2021 at 10:01, Christoph Hellwig <[email protected]> wrote:
> On Mon, Oct 11, 2021 at 07:57:17AM +0000, Simon Ser wrote:
> > No, we can't have a "I_AM_NOT_BROKEN" ioctl for each and every uAPI mis-use.
> > User-space detection has been determined to be the best course of action.
>
> If your API addition breaks userspace, yes you need an add-in.
It's not an API addition. It's a ChromeOS fix that breaks my user-space.
> With your completely broken change you cement in a mapping of an executable
> name to map to what you consider a "bug" without any way to fix it up.
If that's the only concern, it'd be very easy to add a CAP_ATOMIC >= 2 check
like we have for Xorg. This would make it so ChromeOS can eventually opt-out
of the quirk.
On Mon, Oct 11, 2021 at 07:57:17AM +0000, Simon Ser wrote:
> No, we can't have a "I_AM_NOT_BROKEN" ioctl for each and every uAPI mis-use.
> User-space detection has been determined to be the best course of action.
If your API addition breaks userspace, yes you need an add-in.
> I guess I'll just inline these functions in the driver then, if a revert will
> be NACK'ed by you?
It will be NACKed, and I will also complain to Linus about any PR containing
buggy code like this.
With your completely broken change you cement in a mapping of an executable
name to map to what you consider a "bug" without any way to fix it up.
Which is even worse for a something fast moving like chrome/chromeos which
will eventually gets its act together and fix things while you'll keep a
weird feature mismatch just for it around forever.
No wonder our graphics stack is stuch a convoluted buggy mess if you keep
piling broken workarounds over workarounds instead of sorting things out
properly.
Hi Stephen,
On Fri, 8 Oct 2021 11:31:16 +1100 Stephen Rothwell <[email protected]> wrote:
>
> Hi all,
>
> After merging the amdgpu tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> ERROR: modpost: "get_mm_exe_file" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
>
> Caused by commit
>
> f4f80155e6e8 ("amd/display: only require overlay plane to cover whole CRTC on ChromeOS")
>
> I have used the amdgpu tree from next-20211007 for today.
This failure has returned today ...
--
Cheers,
Stephen Rothwell
On Mon, Oct 11, 2021 at 10:25 PM Stephen Rothwell <[email protected]> wrote:
>
> Hi Stephen,
>
> On Fri, 8 Oct 2021 11:31:16 +1100 Stephen Rothwell <[email protected]> wrote:
> >
> > Hi all,
> >
> > After merging the amdgpu tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > ERROR: modpost: "get_mm_exe_file" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
> >
> > Caused by commit
> >
> > f4f80155e6e8 ("amd/display: only require overlay plane to cover whole CRTC on ChromeOS")
> >
> > I have used the amdgpu tree from next-20211007 for today.
>
> This failure has returned today ...
Whoops, pushed the wrong branch. Fixed.
Alex
> --
> Cheers,
> Stephen Rothwell