| Message ID | 20240131190251.4668-1-andrew.kanner@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] module.h: define __symbol_get_gpl() as a regular __symbol_get() | expand |
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?
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?
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
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 :)
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.
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
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
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. */
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(-)