diff mbox series

[v1] module.h: define __symbol_get_gpl() as a regular __symbol_get()

Message ID 20240131190251.4668-1-andrew.kanner@gmail.com
State New
Headers
Series [v1] module.h: define __symbol_get_gpl() as a regular __symbol_get() |

Commit Message

Andrew Kanner Jan. 31, 2024, 7:02 p.m. UTC 
  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 <andrew.kanner@gmail.com>
---
 include/linux/module.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Christoph Hellwig Feb. 1, 2024, 5:29 a.m. UTC | #1 
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?
Andrew Kanner Feb. 1, 2024, 9:29 a.m. UTC | #2 
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?
Luis Chamberlain Feb. 1, 2024, 6:13 p.m. UTC | #3 
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
Christoph Hellwig Feb. 2, 2024, 7:27 a.m. UTC | #4 
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 :)
Andrew Kanner Feb. 13, 2024, 11:10 a.m. UTC | #5 
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.
diff mbox series

Patch

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. */