2024-01-31 19:03:54

by Andrew Kanner

[permalink] [raw]
Subject: [PATCH v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

Prototype for __symbol_get_gpl() was introduced in the initial git
commit 1da177e4c3f4 ("Linux-2.6.12-rc2"), but was not used after that.

In commit 9011e49d54dc ("modules: only allow symbol_get of
EXPORT_SYMBOL_GPL modules") Christoph Hellwig switched __symbol_get()
to process GPL symbols only, most likely this is what
__symbol_get_gpl() was designed to do.

We might either define __symbol_get_gpl() as __symbol_get() or remove
it completely as suggested by Mauro Carvalho Chehab.

Link: https://lore.kernel.org/lkml/5f001015990a76c0da35a4c3cf08e457ec353ab2.1652113087.git.mchehab@kernel.org/
Signed-off-by: Andrew Kanner <[email protected]>
---
include/linux/module.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 96bc462872c0..8a660c81ac3d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -299,7 +299,7 @@ struct notifier_block;
extern int modules_disabled; /* for sysctl */
/* Get/put a kernel symbol (calls must be symmetric) */
void *__symbol_get(const char *symbol);
-void *__symbol_get_gpl(const char *symbol);
+#define __symbol_get_gpl(x) (__symbol_get(x))
#define symbol_get(x) ((typeof(&x))(__symbol_get(__stringify(x))))

/* modules using other modules: kdb wants to see this. */
--
2.39.3



2024-02-01 05:30:13

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

On Wed, Jan 31, 2024 at 10:02:52PM +0300, Andrew Kanner wrote:
> Prototype for __symbol_get_gpl() was introduced in the initial git
> commit 1da177e4c3f4 ("Linux-2.6.12-rc2"), but was not used after that.
>
> In commit 9011e49d54dc ("modules: only allow symbol_get of
> EXPORT_SYMBOL_GPL modules") Christoph Hellwig switched __symbol_get()
> to process GPL symbols only, most likely this is what
> __symbol_get_gpl() was designed to do.
>
> We might either define __symbol_get_gpl() as __symbol_get() or remove
> it completely as suggested by Mauro Carvalho Chehab.

Just remove it, there is no need to keep unused funtionality around.

Btw, where did the discussion start? I hope you're not trying to
add new symbol_get users?


2024-02-01 09:30:00

by Andrew Kanner

[permalink] [raw]
Subject: Re: [PATCH v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

On Thu, Feb 01, 2024 at 06:29:58AM +0100, Christoph Hellwig wrote:
> On Wed, Jan 31, 2024 at 10:02:52PM +0300, Andrew Kanner wrote:
> > Prototype for __symbol_get_gpl() was introduced in the initial git
> > commit 1da177e4c3f4 ("Linux-2.6.12-rc2"), but was not used after that.
> >
> > In commit 9011e49d54dc ("modules: only allow symbol_get of
> > EXPORT_SYMBOL_GPL modules") Christoph Hellwig switched __symbol_get()
> > to process GPL symbols only, most likely this is what
> > __symbol_get_gpl() was designed to do.
> >
> > We might either define __symbol_get_gpl() as __symbol_get() or remove
> > it completely as suggested by Mauro Carvalho Chehab.
>
> Just remove it, there is no need to keep unused funtionality around.
>
> Btw, where did the discussion start? I hope you're not trying to
> add new symbol_get users?
>

Of course not, no new users needed.

I haven't discussed it directly. I found the unused __symbol_get_gpl()
myself, but during investigation of wether it was ever used somewhere
found the old patch series suggested by Mauro Carvalho Chehab (in Cc).

Link: https://lore.kernel.org/lkml/5f001015990a76c0da35a4c3cf08e457ec353ab2.1652113087.git.mchehab@kernel.org/

The patch series is from 2022 and not merged. You can take [PATCH v6
1/4] which removes the unused symbol from the link.

Or I can resend v2 with my commit msg. But not sure about how it works
in such a case - will adding Suggested-by tag (if no objections from
Mauro) with the Link be ok?

--
Andrew Kanner

2024-02-01 23:23:09

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

On Thu, Feb 01, 2024 at 12:29:46PM +0300, Andrew Kanner wrote:
> On Thu, Feb 01, 2024 at 06:29:58AM +0100, Christoph Hellwig wrote:
> > On Wed, Jan 31, 2024 at 10:02:52PM +0300, Andrew Kanner wrote:
> > > Prototype for __symbol_get_gpl() was introduced in the initial git
> > > commit 1da177e4c3f4 ("Linux-2.6.12-rc2"), but was not used after that.
> > >
> > > In commit 9011e49d54dc ("modules: only allow symbol_get of
> > > EXPORT_SYMBOL_GPL modules") Christoph Hellwig switched __symbol_get()
> > > to process GPL symbols only, most likely this is what
> > > __symbol_get_gpl() was designed to do.
> > >
> > > We might either define __symbol_get_gpl() as __symbol_get() or remove
> > > it completely as suggested by Mauro Carvalho Chehab.
> >
> > Just remove it, there is no need to keep unused funtionality around.
> >
> > Btw, where did the discussion start? I hope you're not trying to
> > add new symbol_get users?
> >
>
> Of course not, no new users needed.
>
> I haven't discussed it directly. I found the unused __symbol_get_gpl()
> myself, but during investigation of wether it was ever used somewhere
> found the old patch series suggested by Mauro Carvalho Chehab (in Cc).
>
> Link: https://lore.kernel.org/lkml/5f001015990a76c0da35a4c3cf08e457ec353ab2.1652113087.git.mchehab@kernel.org/
>
> The patch series is from 2022 and not merged. You can take [PATCH v6
> 1/4] which removes the unused symbol from the link.
>
> Or I can resend v2 with my commit msg. But not sure about how it works
> in such a case - will adding Suggested-by tag (if no objections from
> Mauro) with the Link be ok?

While you're at it, if you want to try it, you could see if you can
improve the situation more by looking at symbol_get() users that remain
and seeing if you can instead fix it with proper Kconfig dependency and
at build time. Then we can just remove it as well.

Luis

2024-02-02 07:36:32

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

On Thu, Feb 01, 2024 at 12:29:46PM +0300, Andrew Kanner wrote:
> Of course not, no new users needed.
>
> I haven't discussed it directly. I found the unused __symbol_get_gpl()
> myself, but during investigation of wether it was ever used somewhere
> found the old patch series suggested by Mauro Carvalho Chehab (in Cc).

Ah, ok.

>
> Link: https://lore.kernel.org/lkml/5f001015990a76c0da35a4c3cf08e457ec353ab2.1652113087.git.mchehab@kernel.org/
>
> The patch series is from 2022 and not merged. You can take [PATCH v6
> 1/4] which removes the unused symbol from the link.
>
> Or I can resend v2 with my commit msg. But not sure about how it works
> in such a case - will adding Suggested-by tag (if no objections from
> Mauro) with the Link be ok?

Either is fine. I actually have a patch removing it somewhere in an
unused tree as well :)

2024-02-13 21:36:58

by Andrew Kanner

[permalink] [raw]
Subject: Re: [PATCH v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

On Thu, Feb 01, 2024 at 10:13:54AM -0800, Luis Chamberlain wrote:
>
> While you're at it, if you want to try it, you could see if you can
> improve the situation more by looking at symbol_get() users that remain
> and seeing if you can instead fix it with proper Kconfig dependency and
> at build time. Then we can just remove it as well.
>
> Luis

Sorry for the late reply.

Luis, can you give more details of your idea? I re-read it once, then
came back and still don't understand.

I see that there are ~10 users for symbol_get() currently. Do you want
to stringify symbol names at build time to completely remove
symbol_get() from module.h? Correct me if I'm wrong since using of a
fuction which is not declared anywhere sounds confusing.

--
Andrew Kanner

2024-03-12 22:27:48

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

On Tue, Feb 13, 2024 at 02:10:45PM +0300, Andrew Kanner wrote:
> On Thu, Feb 01, 2024 at 10:13:54AM -0800, Luis Chamberlain wrote:
> >
> > While you're at it, if you want to try it, you could see if you can
> > improve the situation more by looking at symbol_get() users that remain
> > and seeing if you can instead fix it with proper Kconfig dependency and
> > at build time. Then we can just remove it as well.
> >
> > Luis
>
> Sorry for the late reply.
>
> Luis, can you give more details of your idea? I re-read it once, then
> came back and still don't understand.
>
> I see that there are ~10 users for symbol_get() currently. Do you want
> to stringify symbol names at build time to completely remove
> symbol_get() from module.h? Correct me if I'm wrong since using of a
> fuction which is not declared anywhere sounds confusing.

As an example look at the code and see if there's a sensible way to make
some calls built-in instead of part of the module, then the module can
have a kconfig builtin option, that adds to the built-in code which
means you don't need the symbol_get().

For some other pieces of code it may require other strategies.

Luis

2024-03-13 01:12:13

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

On Tue, Mar 12, 2024 at 03:25:27PM -0700, Luis Chamberlain wrote:
> On Tue, Feb 13, 2024 at 02:10:45PM +0300, Andrew Kanner wrote:
> > On Thu, Feb 01, 2024 at 10:13:54AM -0800, Luis Chamberlain wrote:
> > >
> > > While you're at it, if you want to try it, you could see if you can
> > > improve the situation more by looking at symbol_get() users that remain
> > > and seeing if you can instead fix it with proper Kconfig dependency and
> > > at build time. Then we can just remove it as well.
> > >
> > > Luis
> >
> > Sorry for the late reply.
> >
> > Luis, can you give more details of your idea? I re-read it once, then
> > came back and still don't understand.
> >
> > I see that there are ~10 users for symbol_get() currently. Do you want
> > to stringify symbol names at build time to completely remove
> > symbol_get() from module.h? Correct me if I'm wrong since using of a
> > fuction which is not declared anywhere sounds confusing.
>
> As an example look at the code and see if there's a sensible way to make
> some calls built-in instead of part of the module, then the module can
> have a kconfig builtin option, that adds to the built-in code which
> means you don't need the symbol_get().
>
> For some other pieces of code it may require other strategies.

An example is FW_LOADER_USER_HELPER which is bool only, and is selected
by users. It didn't use symbol_get() before, however its an example of
how through Kconfig you can align requirements and define built-in
components, even if they do come from a module.

Luis