Message-ID:
 <DS0PR11MB7765C2C39C92B3A23188E0DEDE092@DS0PR11MB7765.namprd11.prod.outlook.com>
Date: Mon, 15 Apr 2024 10:21:22 +0000
From: "Valerii Chernous -X (vchernou - GLOBALLOGIC INC at Cisco)"
	<vchernou@...co.com>
To: Masahiro Yamada <masahiroy@...nel.org>
CC: Nicolas Schier <nicolas@...sle.eu>,
        "Daniel Walker (danielwa)"
	<danielwa@...co.com>,
        Nicolas Schier <n.schier@....de>,
        Nathan Chancellor
	<nathan@...nel.org>,
        "xe-linux-external(mailer list)"
	<xe-linux-external@...co.com>,
        Jonathan Corbet <corbet@....net>,
        "linux-kbuild@...r.kernel.org" <linux-kbuild@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] Add MO(mod objs) variable to process ext modules with
 subdirs

Hi Masahiro,

Thank you for your response and for description of relative method with
"O" and "M" variables. I'm tried it before start upstreaming my patch.
It working but have strong restriction on external modules location correspond
to kernel sources/output objects.
Unfortunately, proper external module location for your method can't be guarantee
into all projects and my not exceptional one.

> The workaround described in the commit message
> (overwrite 'src') is different from what I know.
Yes, your method is relative method with using "O" and "M" variables and it
different from one that I used

> As I explained to Daniel before, the point is,
> O= refers to the kernel output directory, and
> M= specifies a relative path to your downstream
> module directory.

Thank you, again, for your explanation and I'm tried this before starting
discussion with You and other guys from kernel team

> Say, you have a linux source tree and external modules
> under ~/my-project-src/, and you want to output all
> build artifacts under ~/my-build-dir/.

> my-project-src
> |-- linux
> \-- my-modules

> masahiro@zoe:~/my-project-src/my-modules$ tree
> .
> |-- Kbuild
> |-- dir1
> |   |-- Kbuild
> |   |-- bar.c
> |   `-- dir2
> |       |-- Kbuild
> |       `-- baz.c
> `-- foo.c
> 3 directories, 6 files

> masahiro@zoe:~/my-project-src/my-modules$ cat Kbuild
> obj-m += foo.o
> obj-m += dir1/

> masahiro@zoe:~/my-project-src/my-modules$ cat dir1/Kbuild
> obj-m += bar.o
> obj-m += dir2/

> masahiro@zoe:~/my-project-src/my-modules$ cat dir1/dir2/Kbuild
> obj-m += baz.o

> First, build the kernel and external modules
> in separate output directories.

> masahiro@zoe:~/my-project-src/linux$ make O=~/my-build-dir/linux defconfig all
> [ snip ]

> masahiro@zoe:~/my-project-src/linux$ make O=~/my-build-dir/linux M=../my-modules
> make[1]: Entering directory '/home/masahiro/my-build-dir/linux'
>   CC [M]  ../my-modules/dir1/dir2/baz.o
>   CC [M]  ../my-modules/dir1/bar.o
>   CC [M]  ../my-modules/foo.o
>   MODPOST ../my-modules/Module.symvers
>   CC [M]  ../my-modules/foo.mod.o
>   LD [M]  ../my-modules/foo.ko
>   CC [M]  ../my-modules/dir1/bar.mod.o
>   LD [M]  ../my-modules/dir1/bar.ko
>   CC [M]  ../my-modules/dir1/dir2/baz.mod.o
>   LD [M]  ../my-modules/dir1/dir2/baz.ko
> make[1]: Leaving directory '/home/masahiro/my-build-dir/linux'

> masahiro@zoe:~/my-build-dir/my-modules$ tree
> .
> |-- Module.symvers
> |-- dir1
> |   |-- bar.ko
> |   |-- bar.mod
> |   |-- bar.mod.c
> |   |-- bar.mod.o
> |   |-- bar.o
> |   |-- dir2
> |   |   |-- baz.ko
> |   |   |-- baz.mod
> |   |   |-- baz.mod.c
> |   |   |-- baz.mod.o
> |   |   |-- baz.o
> |   |   `-- modules.order
> |   `-- modules.order
> |-- foo.ko
> |-- foo.mod
> |-- foo.mod.c
> |-- foo.mod.o
> |-- foo.o
> `-- modules.order
> 3 directories, 19 files

> I saw this before somewhere.
> I believe it is a well-known workaround
> that works with recursion.

I agree with You, Masahiro, this workaround works but it contain significant
restriction that can't be implemented into all projects and as you told - it's
workaround but not official method

> This patch submission is not helpful.

> Kbuild does not support the external module builds
> in a separate output directory.
> Most people know this limitation for a long time.
> You are not the first person to discover it.

Problem will be raised again and again, until somebody will implement it properly :).
Of cause I'm not first who trap to this issue but I'm also not the last :)

> Second, anybody can write a patch like yours
> in several minutes.

I'm totally agree with you, it's not a complex patch and that's a good news
because small change can provide required functionality :).

> There already exists a similar (but more correct) patch:

>   https://lore.kernel.org/linux-kbuild/e58cba84c90c40108ac678500f33655e@xiaomi.com/

> That one works more correctly than yours, because modpost
> works with no error, and 'make clean' works too.
> I am not suggesting to fix scripts/{Makefile.modpost,Makefile.clean}.

> If such a patch had been acceptable,
> it would have been accepted many years before.

> Things are, the decision is postponed until
> we are confident about a solution.
> I must avoid a situation where a bad solution
> is upstreamed as official.
> That is worse than nothing.

Ok, I'm understand your opinion.

> And, I am pretty sure that your patch is not
> the right solution.

> --
> Best Regards
> Masahiro Yamada

Ok, maybe one line patch that I submitted today will be more acceptable
compare to current one.
It don't create any new variables, it only fix "src" variable recursion.

Best regards,
Valerii

________________________________________
From: Masahiro Yamada <masahiroy@...nel.org>
Sent: Sunday, April 14, 2024 10:48 AM
To: Valerii Chernous -X (vchernou - GLOBALLOGIC INC at Cisco)
Cc: Nicolas Schier; Daniel Walker (danielwa); Nicolas Schier; Nathan Chancellor; xe-linux-external(mailer list); Jonathan Corbet; linux-kbuild@...r.kernel.org; linux-doc@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] Add MO(mod objs) variable to process ext modules with subdirs

The workaround described in the commit message
(overwrite 'src') is different from what I know.


As I explained to Daniel before, the point is,
O= refers to the kernel output directory, and
M= specifies a relative path to your downstream
module directory.


Say, you have a linux source tree and external modules
under ~/my-project-src/, and you want to output all
build artifacts under ~/my-build-dir/.


my-project-src
|-- linux
\-- my-modules




masahiro@zoe:~/my-project-src/my-modules$ tree
.
|-- Kbuild
|-- dir1
|   |-- Kbuild
|   |-- bar.c
|   `-- dir2
|       |-- Kbuild
|       `-- baz.c
`-- foo.c

3 directories, 6 files


masahiro@zoe:~/my-project-src/my-modules$ cat Kbuild
obj-m += foo.o
obj-m += dir1/

masahiro@zoe:~/my-project-src/my-modules$ cat dir1/Kbuild
obj-m += bar.o
obj-m += dir2/

masahiro@zoe:~/my-project-src/my-modules$ cat dir1/dir2/Kbuild
obj-m += baz.o



First, build the kernel and external modules
in separate output directories.

masahiro@zoe:~/my-project-src/linux$ make O=~/my-build-dir/linux defconfig all
[ snip ]


masahiro@zoe:~/my-project-src/linux$ make O=~/my-build-dir/linux M=../my-modules
make[1]: Entering directory '/home/masahiro/my-build-dir/linux'
  CC [M]  ../my-modules/dir1/dir2/baz.o
  CC [M]  ../my-modules/dir1/bar.o
  CC [M]  ../my-modules/foo.o
  MODPOST ../my-modules/Module.symvers
  CC [M]  ../my-modules/foo.mod.o
  LD [M]  ../my-modules/foo.ko
  CC [M]  ../my-modules/dir1/bar.mod.o
  LD [M]  ../my-modules/dir1/bar.ko
  CC [M]  ../my-modules/dir1/dir2/baz.mod.o
  LD [M]  ../my-modules/dir1/dir2/baz.ko
make[1]: Leaving directory '/home/masahiro/my-build-dir/linux'


masahiro@zoe:~/my-build-dir/my-modules$ tree
.
|-- Module.symvers
|-- dir1
|   |-- bar.ko
|   |-- bar.mod
|   |-- bar.mod.c
|   |-- bar.mod.o
|   |-- bar.o
|   |-- dir2
|   |   |-- baz.ko
|   |   |-- baz.mod
|   |   |-- baz.mod.c
|   |   |-- baz.mod.o
|   |   |-- baz.o
|   |   `-- modules.order
|   `-- modules.order
|-- foo.ko
|-- foo.mod
|-- foo.mod.c
|-- foo.mod.o
|-- foo.o
`-- modules.order

3 directories, 19 files


I saw this before somewhere.
I believe it is a well-known workaround
that works with recursion.






This patch submission is not helpful.


Kbuild does not support the external module builds
in a separate output directory.
Most people know this limitation for a long time.
You are not the first person to discover it.


Second, anybody can write a patch like yours
in several minutes.


There already exists a similar (but more correct) patch:

  https://lore.kernel.org/linux-kbuild/e58cba84c90c40108ac678500f33655e@xiaomi.com/


That one works more correctly than yours, because modpost
works with no error, and 'make clean' works too.


I am not suggesting to fix scripts/{Makefile.modpost,Makefile.clean}.


If such a patch had been acceptable,
it would have been accepted many years before.


Things are, the decision is postponed until
we are confident about a solution.
I must avoid a situation where a bad solution
is upstreamed as official.
That is worse than nothing.

And, I am pretty sure that your patch is not
the right solution.








--
Best Regards


Masahiro Yamada

Powered by blists - more mailing lists