| Message ID | 20230222121453.91915-22-nick.alcock@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/27] kbuild, power: reset: keystone-reset: remove MODULE_LICENSE in non-modules | expand |
Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> 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?
On Wed, Feb 22, 2023 at 03:48:56PM +0100, Christoph Hellwig wrote: > Looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > 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
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 <hch@lst.de> >> >> 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...
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
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.
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
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 <song.bao.hua@hisilicon.com>"); MODULE_DESCRIPTION("dma_map benchmark driver"); -MODULE_LICENSE("GPL");
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 <nick.alcock@oracle.com> Suggested-by: Luis Chamberlain <mcgrof@kernel.org> Cc: Luis Chamberlain <mcgrof@kernel.org> Cc: linux-modules@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: Hitomi Hasegawa <hasegawa-hitomi@fujitsu.com> Cc: Christoph Hellwig <hch@lst.de> Cc: Marek Szyprowski <m.szyprowski@samsung.com> Cc: iommu@lists.linux.dev --- kernel/dma/map_benchmark.c | 1 - 1 file changed, 1 deletion(-)