Re: [PATCH 10/17] tty: remove MODULE_LICENSE in non-modules

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Greg,

On Fri, Mar 24, 2023 at 10:14 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Fri, Mar 24, 2023 at 10:08:17AM +0100, Geert Uytterhoeven wrote:
> > On Fri, Mar 10, 2023 at 8:42 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > > On Fri, Mar 10, 2023 at 08:31:30AM +0100, Greg Kroah-Hartman wrote:
> > > > On Thu, Mar 09, 2023 at 02:38:10PM -0800, Luis Chamberlain wrote:
> > > > > On Thu, Mar 09, 2023 at 05:15:42PM +0100, Greg Kroah-Hartman wrote:
> > > > > > On Thu, Mar 02, 2023 at 09:17:52PM +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. 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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > > > Cc: Jiri Slaby <jirislaby@kernel.org>
> > > > > > > ---
> > > > > > >  drivers/tty/n_null.c | 1 -
> > > > > > >  1 file changed, 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/tty/n_null.c b/drivers/tty/n_null.c
> > > > > > > index f913b665af725..c24f75942c49d 100644
> > > > > > > --- a/drivers/tty/n_null.c
> > > > > > > +++ b/drivers/tty/n_null.c
> > > > > > > @@ -63,7 +63,6 @@ static void __exit n_null_exit(void)
> > > > > > >  module_init(n_null_init);
> > > > > > >  module_exit(n_null_exit);
> > > > > > >
> > > > > > > -MODULE_LICENSE("GPL");
> > > > > > >  MODULE_AUTHOR("Alan Cox");
> > > > > > >  MODULE_ALIAS_LDISC(N_NULL);
> > > > > > >  MODULE_DESCRIPTION("Null ldisc driver");
> > > > > > > --
> > > > > > > 2.39.1.268.g9de2f9a303
> > > > > > >
> > > > > >
> > > > > > Nope, sorry, this is not good to do, please fix kbuild instead of
> > > > > > forcing a tree-wide change like this.
> > > > >
> > > > > Masahiro Yamada already NACK'd it such effort:
> > > > >
> > > > > https://lkml.kernel.org/r/CAK7LNAQLttPD=Ae==e0CYeQtS78=o_JZFK+zxa29JnUYio52Ug@mail.gmail.com
> > > > >
> > > > > And his descriptiuon of the reasoning and logic is explained here:
> > > > >
> > > > > https://lore.kernel.org/all/CAK7LNASL7_RgfASstBvN6AzhR=nMU=HsQvODf5q13Xud8tBWRQ@mail.gmail.com/
> > > > >
> > > > > Let me summarize it though with a few quotes from him:
> > > > >
> > > > > "Having false-positives in modules.builtin should be OK"
> > > > > "In this sense, having always-builtin entries in module.builtin is OK."
> > > >
> > > > None of that matters, sorry.
> > > >
> > > > Again, all I am saying is that you can not have some MODULE_() macros
> > > > that are ok for code that is built in, and some that are not, for
> > > > "reasons" that have to do how you all are treating the build system
> > > > infrastructure as you are now putting arbritrary requirements for all
> > > > driver authors (of which there are thousands) to know this.
> > >
> > > As noted once again, it is not putting hard requirement. Future tooling
> > > not yet added would just not benefit from distinguishing symbols for
> > > your modules.
> > >
> > > I'm happy to live with module authors not wanting to remove the module
> > > license tag from their modules if they can never actually be modules
> > > from not benefitting from the above tooling gains as its just cherry
> > > on top tooling gains.
> >
> > Apparently lots of these patches have not arrived in linux-next
> > without Acks (we're still discussing about this, right???).
> >
> > And some of the modified files have no SPDX-License-Identifier
> > lines yet, so we are losing important licensing information:
> >
> > $ git grep -L SPDX-License-Identifier -- $(git show $(git log
> > --oneline v6.3-rc1..linux-next/master | grep "remove MODULE_LICENSE in
> > non-modules" | cut -d " " -f 1) | lsdiff --strip=1)
> > drivers/bus/arm-cci.c
> > drivers/bus/imx-weim.c
> > drivers/bus/simple-pm-bus.c
> > drivers/gpu/drm/drm_mipi_dsi.c
> > drivers/irqchip/irq-mvebu-pic.c
> > drivers/reset/reset-axs10x.c
> > drivers/reset/reset-hsdk.c
> > drivers/soc/sunxi/sunxi_sram.c
> > drivers/video/fbdev/asiliantfb.c
> > drivers/video/fbdev/gbefb.c
> > drivers/video/fbdev/imsttfb.c
> > drivers/xen/xenbus/xenbus_probe.c
> > lib/glob.c
>
> Ick, that's not ok at all.
>
> Again, I strongly feel that removing MODULE_LICENSE() lines from files
> that just don't happen to be built as a module is not ok as no other
> MODULE_*() macro has this arbitrary restriction.

I (still) agree with that, and I saw similar comments from others as well.
Unfortunately these comments are spread across tens of threads :-(

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds