2023-02-22 14:03:18

by Nick Alcock

[permalink] [raw]
Subject: [PATCH 21/27] kbuild, dma-mapping: benchmark: remove MODULE_LICENSE in non-modules

Since commit 8b41fc4454e ("kbuild: create modules.builtin without
Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
are used to identify modules. As a consequence, uses of the macro
in non-modules will cause modprobe to misidentify their containing
object file as a module when it is not (false positives), and modprobe
might succeed rather than failing with a suitable error message.

So remove it in the files in this commit, none of which can be built as
modules.

Signed-off-by: Nick Alcock <[email protected]>
Suggested-by: Luis Chamberlain <[email protected]>
Cc: Luis Chamberlain <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Hitomi Hasegawa <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Marek Szyprowski <[email protected]>
Cc: [email protected]
---
kernel/dma/map_benchmark.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
index 0520a8f4fb1d..02205ab53b7e 100644
--- a/kernel/dma/map_benchmark.c
+++ b/kernel/dma/map_benchmark.c
@@ -356,4 +356,3 @@ module_exit(map_benchmark_cleanup);

MODULE_AUTHOR("Barry Song <[email protected]>");
MODULE_DESCRIPTION("dma_map benchmark driver");
-MODULE_LICENSE("GPL");
--
2.39.1.268.g9de2f9a303



2023-02-22 14:50:37

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 21/27] kbuild, dma-mapping: benchmark: remove MODULE_LICENSE in non-modules

Looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

On Wed, Feb 22, 2023 at 12:14:47PM +0000, Nick Alcock wrote:
> Since commit 8b41fc4454e ("kbuild: create modules.builtin without
> Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
> are used to identify modules.

.. but this seems like a really odd design. How is this going to
continue working once we can autogenerate the module license section
from the SPDX tags, which we eventually really should?

2023-02-22 22:52:27

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH 21/27] kbuild, dma-mapping: benchmark: remove MODULE_LICENSE in non-modules

On Wed, Feb 22, 2023 at 03:48:56PM +0100, Christoph Hellwig wrote:
> Looks good:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
>
> On Wed, Feb 22, 2023 at 12:14:47PM +0000, Nick Alcock wrote:
> > Since commit 8b41fc4454e ("kbuild: create modules.builtin without
> > Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
> > are used to identify modules.
>
> .. but this seems like a really odd design. How is this going to
> continue working once we can autogenerate the module license section
> from the SPDX tags, which we eventually really should?

Yes I totally agree we should. But I think we should take this by steps.
First, we ensure we have only MODULE_LICENSE() macros upstream on things which
are really possible modules, ie we remove the false positives. We then put a
stop-gap script which can complain if it finds new usecases which are buggy.

Then we look for an optimal way to address the final step:

* remove all MODULE_LICENSE() and autogenerate them from SPDX

The difficulty in this will be that we want to upkeep existing build
heuristics and avoid to have to traverse the tree twice (see details
on commit 8b41fc4454e). I can't think of an easy way to do this that
does not involve using kconfig tristate somehow. This is a bit of
tricky homework we have. Perhaps Masahiro can come up with something
clever.

Luis

2023-02-23 15:32:37

by Nick Alcock

[permalink] [raw]
Subject: Re: [PATCH 21/27] kbuild, dma-mapping: benchmark: remove MODULE_LICENSE in non-modules

On 22 Feb 2023, Luis Chamberlain spake thusly:

> On Wed, Feb 22, 2023 at 03:48:56PM +0100, Christoph Hellwig wrote:
>> Looks good:
>>
>> Reviewed-by: Christoph Hellwig <[email protected]>
>>
>> On Wed, Feb 22, 2023 at 12:14:47PM +0000, Nick Alcock wrote:
>> > Since commit 8b41fc4454e ("kbuild: create modules.builtin without
>> > Makefile.modbuiltin or tristate.conf"), MODULE_LICENSE declarations
>> > are used to identify modules.
>>
>> .. but this seems like a really odd design. How is this going to
>> continue working once we can autogenerate the module license section
>> from the SPDX tags, which we eventually really should?
>
> Yes I totally agree we should. But I think we should take this by steps.
> First, we ensure we have only MODULE_LICENSE() macros upstream on things which
> are really possible modules, ie we remove the false positives. We then put a
> stop-gap script which can complain if it finds new usecases which are buggy.

(and we have such a script already, though it's not in-tree: I used it
to generate the list of affected files that make up this series. I'll
keep running it at least once per release cycle to identify regressions
in this area, and fix them as they come up.)

> Then we look for an optimal way to address the final step:
>
> * remove all MODULE_LICENSE() and autogenerate them from SPDX

Ooh that would be nice!

> The difficulty in this will be that we want to upkeep existing build
> heuristics and avoid to have to traverse the tree twice (see details
> on commit 8b41fc4454e). I can't think of an easy way to do this that
> does not involve using kconfig tristate somehow.

Nor can I -- and more generally I can't figure out a way to get from the
Kconfig symbols to the source files that constitute them without
retraversing the tree, since the only place the relationship is recorded
is in makefiles, and those makefiles use a lot of make functionality
(including more or less arbitrary make functions).

(restating the underlying difficulty here in case, like me, you lost
track of it over the last few months)

Of course the build process is doing that traversal anyway -- the
problem is that the only approach we have to get from tristate to a list
of modules-or-builtins involves emitting *different values* for CONFIG_
symbols (uppercase rather than lowercase) and then triggering on those
to do things -- and if you do that you can't simultaneously use those
CONFIG_ variables for their normal purpose.

We can't rename those variables for this purpose because we're depending
on makefiles all across the tree expanding them. I tried to arrange for
their expansion to have side effects (so that evaluating $(CONFIG_FOO)
produced both 'y' or 'm' and *also* did... *something* that produced an
object file list for our consumption) but that also doesn't work because
unfortunately the tristate determination code needs a *mapping* from
CONFIG_ variable value to the result of the variable expansion, and a
line like

foo-$(CONFIG_FOO) := x y z

doesn't let the expansion of CONFIG_FOO have any sort of access to the
result of the assignment to foo-y / foo-m, and after the assignment's
happened there's no way to tell that the $(foo-m) -> x y z mapping was
generated by the expansion of CONFIG_FOO in particlar. So multiple
evaluations (which means, in effect, multiple make invocations) seems to
be the only way.

I do hope Masahiro has some brilliant idea here. Mind you I'm not sure
I'm clever enough to have come up with the original Makefile.modbuiltin
scheme either...

2023-02-23 20:51:38

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH 21/27] kbuild, dma-mapping: benchmark: remove MODULE_LICENSE in non-modules

On Thu, Feb 23, 2023 at 03:31:50PM +0000, Nick Alcock wrote:
> On 22 Feb 2023, Luis Chamberlain spake thusly:
> > Then we look for an optimal way to address the final step:
> >
> > * remove all MODULE_LICENSE() and autogenerate them from SPDX
>
> Ooh that would be nice!
>
> > The difficulty in this will be that we want to upkeep existing build
> > heuristics and avoid to have to traverse the tree twice (see details
> > on commit 8b41fc4454e). I can't think of an easy way to do this that
> > does not involve using kconfig tristate somehow.
>
> Nor can I -- and more generally I can't figure out a way to get from the
> Kconfig symbols to the source files that constitute them without
> retraversing the tree, since the only place the relationship is recorded
> is in makefiles, and those makefiles use a lot of make functionality
> (including more or less arbitrary make functions).

$ grep "_MODULE 1" ./include/generated/autoconf.h| wc -l
560

$ grep "_MODULE 1" ./include/generated/autoconf.h| grep XFS
#define CONFIG_XFS_FS_MODULE 1

I *think* the trick will likely be to have new a possibilities.h or just
agument autoconf.h with POSSIBLE_MODULE for each module.

The next complexity lies in inferring the license and doing the license
output given a combination. I *think* you already figured out the objs
from the module, and in fact your new kallsyms extension I think prints
these out right (which I find highly useful)? If so then we use these as
input source for an SPDX license lookup. Instead of having this
relationship grep'd after at build time, I wonder if might be good to
just collect all license associates to all files in a header file
similar to _MODULE prefix so maybe SPDX_$(file_path)_LICENSE_$license
which creates a header file 1-1 mapping.

Not sure if that's too much noise.

Just a thought, to get the wheels spinning.

Luis

2023-02-24 14:20:53

by Nick Alcock

[permalink] [raw]
Subject: Re: [PATCH 21/27] kbuild, dma-mapping: benchmark: remove MODULE_LICENSE in non-modules

On 23 Feb 2023, Luis Chamberlain outgrape:

> On Thu, Feb 23, 2023 at 03:31:50PM +0000, Nick Alcock wrote:
>> Nor can I -- and more generally I can't figure out a way to get from the
>> Kconfig symbols to the source files that constitute them without
>> retraversing the tree, since the only place the relationship is recorded
>> is in makefiles, and those makefiles use a lot of make functionality
>> (including more or less arbitrary make functions).
>
> $ grep "_MODULE 1" ./include/generated/autoconf.h| wc -l
> 560
>
> $ grep "_MODULE 1" ./include/generated/autoconf.h| grep XFS
> #define CONFIG_XFS_FS_MODULE 1
>
> I *think* the trick will likely be to have new a possibilities.h or just
> agument autoconf.h with POSSIBLE_MODULE for each module.
>
> The next complexity lies in inferring the license and doing the license
> output given a combination. I *think* you already figured out the objs
> from the module, and in fact your new kallsyms extension I think prints
> these out right (which I find highly useful)?

Oh, of course, I forgot about that (how stupid of me). Of course the
double-traversal is only necessary if we're trying to *compare* the
results of tristate in Kconfig with the result of the modinfo objs=: if
we assume the modinfo objs= is right (which it should be in the end), we
can just rely on it and then we don't need to double-traverse after all,
except when verifying that (which is a rare intermittent maintenance
task).

(Of course, kallmodsyms elides all objnames that aren't necessary for
symbol disambiguation and reduces the length of what it keeps as far as
it can, but I'm open to an option that just stores the lot, unelided: it
would eat ~750KiB in the kernel image for all but very small kernels,
but for debugging that's fine. Saving more space than that requires
storing the things in a per-path-component tree or something, and would
likely still eat >500K because the leaves are extremely numerous.)

> If so then we use these as
> input source for an SPDX license lookup. Instead of having this
> relationship grep'd after at build time, I wonder if might be good to
> just collect all license associates to all files in a header file
> similar to _MODULE prefix so maybe SPDX_$(file_path)_LICENSE_$license
> which creates a header file 1-1 mapping.
>
> Not sure if that's too much noise.
>
> Just a thought, to get the wheels spinning.

The only problem that I can see with that is that this stops us using
MODULE_LICENSE for modinfo construction, since right now things like the
objs= and the module name are dependent on per-module #defines passed in
via -D, which obviously can only have one value while compiling a single
file. But it would be perfectly doable to rename MODULE_LICENSE() to
something like a zero-arg MODULE_INFO() and relieve it of the
responsibility for setting the license, so we could put the license info
into a single file as you suggest. Non-builtin modules could just stuff
a single MODULE_LICENSE line in the mod.c.

--
NULL && (void)

2023-02-24 17:18:59

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH 21/27] kbuild, dma-mapping: benchmark: remove MODULE_LICENSE in non-modules

On Fri, Feb 24, 2023 at 02:20:16PM +0000, Nick Alcock wrote:
> The only problem that I can see with that is that this stops us using
> MODULE_LICENSE for modinfo construction,

Yes, the requirement shifts to having us *write* the module license
from the combination computation of files from SPDX, so although not
present anymore what changes is a new target / goal in ordering which has
us construct that define for the file.

Easier said than done of course.

Luis