* Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit [not found] ` <20210121074959.313333-9-hch@lst.de> @ 2021-01-21 8:25 ` Daniel Vetter [not found] ` <20210121082820.GA25719@lst.de> 0 siblings, 1 reply; 11+ messages in thread From: Daniel Vetter @ 2021-01-21 8:25 UTC (permalink / raw) To: Christoph Hellwig Cc: Petr Mladek, Jiri Kosina, Andrew Donnellan, linux-kbuild, David Airlie, Masahiro Yamada, Josh Poimboeuf, Linux Kernel Mailing List, Michal Marek, Joe Lawrence, dri-devel, Thomas Zimmermann, Jessica Yu, Frederic Barrat, live-patching, Miroslav Benes, linuxppc-dev On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig <hch@lst.de> wrote: > > drm_fb_helper_modinit has a lot of boilerplate for what is not very > simple functionality. Just open code it in the only caller using > IS_ENABLED and IS_MODULE. > > Signed-off-by: Christoph Hellwig <hch@lst.de> I didn't spot any dependencies with your series, should I just merge this through drm trees? Or do you want an ack? -Daniel > --- > drivers/gpu/drm/drm_crtc_helper_internal.h | 10 --------- > drivers/gpu/drm/drm_fb_helper.c | 16 ------------- > drivers/gpu/drm/drm_kms_helper_common.c | 26 +++++++++++----------- > 3 files changed, 13 insertions(+), 39 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h > index 25ce42e799952c..61e09f8a8d0ff0 100644 > --- a/drivers/gpu/drm/drm_crtc_helper_internal.h > +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h > @@ -32,16 +32,6 @@ > #include <drm/drm_encoder.h> > #include <drm/drm_modes.h> > > -/* drm_fb_helper.c */ > -#ifdef CONFIG_DRM_FBDEV_EMULATION > -int drm_fb_helper_modinit(void); > -#else > -static inline int drm_fb_helper_modinit(void) > -{ > - return 0; > -} > -#endif > - > /* drm_dp_aux_dev.c */ > #ifdef CONFIG_DRM_DP_AUX_CHARDEV > int drm_dp_aux_dev_init(void); > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index ce6d63ca75c32a..0b9f1ae1b7864c 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -2499,19 +2499,3 @@ void drm_fbdev_generic_setup(struct drm_device *dev, > drm_client_register(&fb_helper->client); > } > EXPORT_SYMBOL(drm_fbdev_generic_setup); > - > -/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT) > - * but the module doesn't depend on any fb console symbols. At least > - * attempt to load fbcon to avoid leaving the system without a usable console. > - */ > -int __init drm_fb_helper_modinit(void) > -{ > -#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT) > - const char name[] = "fbcon"; > - > - if (!module_loaded(name)) > - request_module_nowait(name); > -#endif > - return 0; > -} > -EXPORT_SYMBOL(drm_fb_helper_modinit); > diff --git a/drivers/gpu/drm/drm_kms_helper_common.c b/drivers/gpu/drm/drm_kms_helper_common.c > index 221a8528c9937a..b694a7da632eae 100644 > --- a/drivers/gpu/drm/drm_kms_helper_common.c > +++ b/drivers/gpu/drm/drm_kms_helper_common.c > @@ -64,19 +64,19 @@ MODULE_PARM_DESC(edid_firmware, > > static int __init drm_kms_helper_init(void) > { > - int ret; > - > - /* Call init functions from specific kms helpers here */ > - ret = drm_fb_helper_modinit(); > - if (ret < 0) > - goto out; > - > - ret = drm_dp_aux_dev_init(); > - if (ret < 0) > - goto out; > - > -out: > - return ret; > + /* > + * The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT) > + * but the module doesn't depend on any fb console symbols. At least > + * attempt to load fbcon to avoid leaving the system without a usable > + * console. > + */ > + if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) && > + IS_MODULE(CONFIG_FRAMEBUFFER_CONSOLE) && > + !IS_ENABLED(CONFIG_EXPERT) && > + !module_loaded("fbcon")) > + request_module_nowait("fbcon"); > + > + return drm_dp_aux_dev_init(); > } > > static void __exit drm_kms_helper_exit(void) > -- > 2.29.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20210121082820.GA25719@lst.de>]
* Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit [not found] ` <20210121082820.GA25719@lst.de> @ 2021-01-21 8:39 ` Daniel Vetter 0 siblings, 0 replies; 11+ messages in thread From: Daniel Vetter @ 2021-01-21 8:39 UTC (permalink / raw) To: Christoph Hellwig Cc: Petr Mladek, Jiri Kosina, Andrew Donnellan, linux-kbuild, David Airlie, Masahiro Yamada, Josh Poimboeuf, Linux Kernel Mailing List, Michal Marek, Joe Lawrence, dri-devel, Thomas Zimmermann, Jessica Yu, Frederic Barrat, live-patching, Miroslav Benes, linuxppc-dev On Thu, Jan 21, 2021 at 9:28 AM Christoph Hellwig <hch@lst.de> wrote: > > On Thu, Jan 21, 2021 at 09:25:40AM +0100, Daniel Vetter wrote: > > On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig <hch@lst.de> wrote: > > > > > > drm_fb_helper_modinit has a lot of boilerplate for what is not very > > > simple functionality. Just open code it in the only caller using > > > IS_ENABLED and IS_MODULE. > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > > I didn't spot any dependencies with your series, should I just merge > > this through drm trees? Or do you want an ack? > > I'd prefer an ACK - module_loaded() is only introduced earlier in this > series. I was looking for that but didn't find the hunk touching drm somehow ... Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20210121074959.313333-2-hch@lst.de>]
* Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module [not found] ` <20210121074959.313333-2-hch@lst.de> @ 2021-01-21 9:09 ` Andrew Donnellan 0 siblings, 0 replies; 11+ messages in thread From: Andrew Donnellan @ 2021-01-21 9:09 UTC (permalink / raw) To: Christoph Hellwig, Frederic Barrat, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel, dri-devel, live-patching, linuxppc-dev On 21/1/21 6:49 pm, Christoph Hellwig wrote: > The static inline get_cxl_module function is entirely unused, > remove it. > > Signed-off-by: Christoph Hellwig <hch@lst.de> The one user of this was removed in 8bf6b91a5125a ("Revert "powerpc/powernv: Add support for the cxl kernel api on the real phb"). Thanks for picking this up. Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> -- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20210121074959.313333-3-hch@lst.de>]
* Re: [PATCH 02/13] module: add a module_loaded helper [not found] ` <20210121074959.313333-3-hch@lst.de> @ 2021-01-21 10:00 ` Christophe Leroy [not found] ` <20210121171119.GA29916@lst.de> 0 siblings, 1 reply; 11+ messages in thread From: Christophe Leroy @ 2021-01-21 10:00 UTC (permalink / raw) To: Christoph Hellwig, Frederic Barrat, Andrew Donnellan, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel, dri-devel, live-patching, linuxppc-dev Le 21/01/2021 à 08:49, Christoph Hellwig a écrit : > Add a helper that takes modules_mutex and uses find_module to check if a > given module is loaded. This provides a better abstraction for the two > callers, and allows to unexport modules_mutex and find_module. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/gpu/drm/drm_fb_helper.c | 7 +------ > include/linux/module.h | 3 +++ > kernel/module.c | 14 ++++++++++++-- > kernel/trace/trace_kprobe.c | 4 +--- > 4 files changed, 17 insertions(+), 11 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index 7a0bcb5b1ffccd..b4654f8a408134 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -589,6 +589,9 @@ static inline bool within_module(unsigned long addr, const struct module *mod) > /* Search for module by name: must hold module_mutex. */ > struct module *find_module(const char *name); > +/* Check if a module is loaded. */ > +bool module_loaded(const char *name); Maybe module_is_loaded() would be a better name. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20210121171119.GA29916@lst.de>]
* RE: [PATCH 02/13] module: add a module_loaded helper [not found] ` <20210121171119.GA29916@lst.de> @ 2021-01-21 17:44 ` David Laight 0 siblings, 0 replies; 11+ messages in thread From: David Laight @ 2021-01-21 17:44 UTC (permalink / raw) To: 'Christoph Hellwig', Christophe Leroy Cc: Petr Mladek, Michal Marek, Andrew Donnellan, Jessica Yu, linux-kbuild, David Airlie, Masahiro Yamada, Jiri Kosina, linux-kernel, Joe Lawrence, dri-devel, Thomas Zimmermann, Josh Poimboeuf, Frederic Barrat, live-patching, Miroslav Benes, linuxppc-dev > > On Thu, Jan 21, 2021 at 11:00:20AM +0100, Christophe Leroy wrote: > > > +bool module_loaded(const char *name); > > > > Maybe module_is_loaded() would be a better name. > > Fine with me. It does look like both callers aren't 'unsafe'. But it is a bit strange returning a stale value. It did make be look a bit more deeply at try_module_get(). For THIS_MODULE (eg to get an extra reference for a kthread) I doubt it can ever fail. But the other cases require a 'module' structure be passed in. ISTM that unloading the module could invalidate the pointer that has just been read from some other structure. I'm guessing that some other lock must always be held. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20210121074959.313333-5-hch@lst.de>]
* Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c [not found] ` <20210121074959.313333-5-hch@lst.de> @ 2021-01-21 21:45 ` Josh Poimboeuf 2021-01-26 14:25 ` Jessica Yu 1 sibling, 0 replies; 11+ messages in thread From: Josh Poimboeuf @ 2021-01-21 21:45 UTC (permalink / raw) To: Christoph Hellwig Cc: Petr Mladek, Joe Lawrence, Andrew Donnellan, linux-kbuild, David Airlie, Masahiro Yamada, Jiri Kosina, linux-kernel, live-patching, Michal Marek, dri-devel, Thomas Zimmermann, Jessica Yu, Frederic Barrat, Miroslav Benes, linuxppc-dev On Thu, Jan 21, 2021 at 08:49:50AM +0100, Christoph Hellwig wrote: > @@ -820,14 +796,25 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj) > const char *name; > > obj->patched = false; > - obj->mod = NULL; Why was this line removed? > if (klp_is_module(obj)) { > if (strlen(obj->name) >= MODULE_NAME_LEN) > return -EINVAL; > name = obj->name; > > - klp_find_object_module(obj); > + /* > + * We do not want to block removal of patched modules and > + * therefore we do not take a reference here. The patches are > + * removed by klp_module_going() instead. > + * > + * Do not mess work of klp_module_coming() and > + * klp_module_going(). Note that the patch might still be > + * needed before klp_module_going() is called. Module functions > + * can be called even in the GOING state until mod->exit() > + * finishes. This is especially important for patches that > + * modify semantic of the functions. > + */ > + obj->mod = find_klp_module(obj->name); These comments don't make sense in this context, they should be kept with the code in find_klp_module(). -- Josh _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c [not found] ` <20210121074959.313333-5-hch@lst.de> 2021-01-21 21:45 ` [PATCH 04/13] livepatch: move klp_find_object_module to module.c Josh Poimboeuf @ 2021-01-26 14:25 ` Jessica Yu 2021-01-27 11:55 ` Petr Mladek 1 sibling, 1 reply; 11+ messages in thread From: Jessica Yu @ 2021-01-26 14:25 UTC (permalink / raw) To: Christoph Hellwig Cc: Petr Mladek, Joe Lawrence, Andrew Donnellan, linux-kbuild, David Airlie, Masahiro Yamada, Jiri Kosina, linux-kernel, live-patching, Michal Marek, dri-devel, Thomas Zimmermann, Josh Poimboeuf, Frederic Barrat, Miroslav Benes, linuxppc-dev +++ Christoph Hellwig [21/01/21 08:49 +0100]: >To uncouple the livepatch code from module loader internals move a >slightly refactored version of klp_find_object_module to module.c >This allows to mark find_module static and removes one of the last >users of module_mutex outside of module.c. > >Signed-off-by: Christoph Hellwig <hch@lst.de> >--- > include/linux/module.h | 3 +-- > kernel/livepatch/core.c | 39 +++++++++++++-------------------------- > kernel/module.c | 17 ++++++++++++++++- > 3 files changed, 30 insertions(+), 29 deletions(-) > >diff --git a/include/linux/module.h b/include/linux/module.h >index b4654f8a408134..8588482bde4116 100644 >--- a/include/linux/module.h >+++ b/include/linux/module.h >@@ -586,8 +586,7 @@ static inline bool within_module(unsigned long addr, const struct module *mod) > return within_module_init(addr, mod) || within_module_core(addr, mod); > } > >-/* Search for module by name: must hold module_mutex. */ >-struct module *find_module(const char *name); >+struct module *find_klp_module(const char *name); > > /* Check if a module is loaded. */ > bool module_loaded(const char *name); >diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c >index a7f625dc24add3..878759baadd81c 100644 >--- a/kernel/livepatch/core.c >+++ b/kernel/livepatch/core.c >@@ -49,30 +49,6 @@ static bool klp_is_module(struct klp_object *obj) > return obj->name; > } > >-/* sets obj->mod if object is not vmlinux and module is found */ >-static void klp_find_object_module(struct klp_object *obj) >-{ >- struct module *mod; >- >- mutex_lock(&module_mutex); >- /* >- * We do not want to block removal of patched modules and therefore >- * we do not take a reference here. The patches are removed by >- * klp_module_going() instead. >- */ >- mod = find_module(obj->name); >- /* >- * Do not mess work of klp_module_coming() and klp_module_going(). >- * Note that the patch might still be needed before klp_module_going() >- * is called. Module functions can be called even in the GOING state >- * until mod->exit() finishes. This is especially important for >- * patches that modify semantic of the functions. >- */ >- if (mod && mod->klp_alive) >- obj->mod = mod; >- mutex_unlock(&module_mutex); >-} Hmm, I am not a huge fan of moving more livepatch code into module.c, I wonder if we can keep them separate. Why not have module_is_loaded() kill two birds with one stone? That is, just have it return a module pointer to signify that the module is loaded, NULL if not. Then we don't need an extra find_klp_module() function just to call find_module() and return a pointer, as module_is_loaded() can just do that for us. As for the mod->klp_alive check, I believe this function (klp_find_object_module()) is called with klp_mutex held, and mod->klp_alive is only modified under klp_mutex. Also, if klp_alive is true, the module is at least COMING and cannot be GOING until it acquires the klp_mutex again in klp_module_going(). So does that hunk really need to be under module_mutex? It has been a long time since I've looked at livepatch code so it would be great if someone could double check. Jessica _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c 2021-01-26 14:25 ` Jessica Yu @ 2021-01-27 11:55 ` Petr Mladek 0 siblings, 0 replies; 11+ messages in thread From: Petr Mladek @ 2021-01-27 11:55 UTC (permalink / raw) To: Jessica Yu Cc: Joe Lawrence, Andrew Donnellan, linux-kbuild, David Airlie, Masahiro Yamada, Jiri Kosina, linux-kernel, live-patching, Michal Marek, dri-devel, Thomas Zimmermann, Josh Poimboeuf, Frederic Barrat, Miroslav Benes, linuxppc-dev, Christoph Hellwig On Tue 2021-01-26 15:25:16, Jessica Yu wrote: > +++ Christoph Hellwig [21/01/21 08:49 +0100]: > > To uncouple the livepatch code from module loader internals move a > > slightly refactored version of klp_find_object_module to module.c > > This allows to mark find_module static and removes one of the last > > users of module_mutex outside of module.c. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > include/linux/module.h | 3 +-- > > kernel/livepatch/core.c | 39 +++++++++++++-------------------------- > > kernel/module.c | 17 ++++++++++++++++- > > 3 files changed, 30 insertions(+), 29 deletions(-) > > > > diff --git a/include/linux/module.h b/include/linux/module.h > > index b4654f8a408134..8588482bde4116 100644 > > --- a/include/linux/module.h > > +++ b/include/linux/module.h > > @@ -586,8 +586,7 @@ static inline bool within_module(unsigned long addr, const struct module *mod) > > return within_module_init(addr, mod) || within_module_core(addr, mod); > > } > > > > -/* Search for module by name: must hold module_mutex. */ > > -struct module *find_module(const char *name); > > +struct module *find_klp_module(const char *name); > > > > /* Check if a module is loaded. */ > > bool module_loaded(const char *name); > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > index a7f625dc24add3..878759baadd81c 100644 > > --- a/kernel/livepatch/core.c > > +++ b/kernel/livepatch/core.c > > @@ -49,30 +49,6 @@ static bool klp_is_module(struct klp_object *obj) > > return obj->name; > > } > > > > -/* sets obj->mod if object is not vmlinux and module is found */ > > -static void klp_find_object_module(struct klp_object *obj) > > -{ > > - struct module *mod; > > - > > - mutex_lock(&module_mutex); > > - /* > > - * We do not want to block removal of patched modules and therefore > > - * we do not take a reference here. The patches are removed by > > - * klp_module_going() instead. > > - */ > > - mod = find_module(obj->name); > > - /* > > - * Do not mess work of klp_module_coming() and klp_module_going(). > > - * Note that the patch might still be needed before klp_module_going() > > - * is called. Module functions can be called even in the GOING state > > - * until mod->exit() finishes. This is especially important for > > - * patches that modify semantic of the functions. > > - */ > > - if (mod && mod->klp_alive) > > - obj->mod = mod; > > - mutex_unlock(&module_mutex); > > -} > > Hmm, I am not a huge fan of moving more livepatch code into module.c, I > wonder if we can keep them separate. > > Why not have module_is_loaded() kill two birds with one stone? That > is, just have it return a module pointer to signify that the module is > loaded, NULL if not. Then we don't need an extra find_klp_module() > function just to call find_module() and return a pointer, as > module_is_loaded() can just do that for us. > > As for the mod->klp_alive check, I believe this function > (klp_find_object_module()) is called with klp_mutex held, and > mod->klp_alive is only modified under klp_mutex. Also, if klp_alive is > true, the module is at least COMING and cannot be GOING until it > acquires the klp_mutex again in klp_module_going(). So does that hunk > really need to be under module_mutex? It has been a long time since > I've looked at livepatch code so it would be great if someone could > double check. We need to make sure that the module is not freed before we manipulate mod->klp_alive. One solution would be to take the reference and block it during this operation. Alternatively it might be to rely on RCU. It seems that the struct is protected by RCU because of kallsyms. But I am not sure if it is safe in all module states. But it should be. We find the module via the same list like kallsyms. Best Regards, Petr _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20210121074959.313333-4-hch@lst.de>]
* Re: [PATCH 03/13] livepatch: refactor klp_init_object [not found] ` <20210121074959.313333-4-hch@lst.de> @ 2021-01-27 12:58 ` Petr Mladek 0 siblings, 0 replies; 11+ messages in thread From: Petr Mladek @ 2021-01-27 12:58 UTC (permalink / raw) To: Christoph Hellwig Cc: Jiri Kosina, Andrew Donnellan, linux-kbuild, David Airlie, Masahiro Yamada, Josh Poimboeuf, linux-kernel, live-patching, Michal Marek, Joe Lawrence, dri-devel, Thomas Zimmermann, Jessica Yu, Frederic Barrat, Miroslav Benes, linuxppc-dev On Thu 2021-01-21 08:49:49, Christoph Hellwig wrote: > Merge three calls to klp_is_module (including one hidden inside > klp_find_object_module) into a single one to simplify the code a bit. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > kernel/livepatch/core.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index f76fdb9255323d..a7f625dc24add3 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -54,9 +54,6 @@ static void klp_find_object_module(struct klp_object *obj) > { > struct module *mod; > > - if (!klp_is_module(obj)) > - return; > - We need to either update the function description or keep this check. I prefer to keep the check. The function does the right thing also for the object "vmlinux". Also the livepatch code includes many similar paranoid checks that makes the code less error prone against any further changes. Of course, it is a matter of taste. > mutex_lock(&module_mutex); > /* > * We do not want to block removal of patched modules and therefore Otherwise, the patch looks fine. Best Regards, Petr _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20210121074959.313333-14-hch@lst.de>]
* Re: [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL* [not found] ` <20210121074959.313333-14-hch@lst.de> @ 2021-01-27 13:49 ` Jessica Yu 0 siblings, 0 replies; 11+ messages in thread From: Jessica Yu @ 2021-01-27 13:49 UTC (permalink / raw) To: Christoph Hellwig Cc: Petr Mladek, Joe Lawrence, Andrew Donnellan, linux-kbuild, David Airlie, Masahiro Yamada, Jiri Kosina, linux-kernel, live-patching, Michal Marek, dri-devel, Thomas Zimmermann, Josh Poimboeuf, Frederic Barrat, Miroslav Benes, linuxppc-dev +++ Christoph Hellwig [21/01/21 08:49 +0100]: >EXPORT_UNUSED_SYMBOL* is not actually used anywhere. Remove the >unused functionality as we generally just remove unused code anyway. > >Signed-off-by: Christoph Hellwig <hch@lst.de> >--- > arch/arm/configs/bcm2835_defconfig | 1 - > arch/arm/configs/mxs_defconfig | 1 - > arch/mips/configs/nlm_xlp_defconfig | 1 - > arch/mips/configs/nlm_xlr_defconfig | 1 - > arch/parisc/configs/generic-32bit_defconfig | 1 - > arch/parisc/configs/generic-64bit_defconfig | 1 - > arch/powerpc/configs/ppc6xx_defconfig | 1 - > arch/s390/configs/debug_defconfig | 1 - > arch/s390/configs/defconfig | 1 - > arch/sh/configs/edosk7760_defconfig | 1 - > arch/sh/configs/sdk7780_defconfig | 1 - > arch/x86/configs/i386_defconfig | 1 - > arch/x86/configs/x86_64_defconfig | 1 - > arch/x86/tools/relocs.c | 4 +- > include/asm-generic/vmlinux.lds.h | 28 --------- > include/linux/export.h | 8 --- > include/linux/module.h | 13 ---- > init/Kconfig | 17 ----- > kernel/module.c | 69 ++------------------- > scripts/checkpatch.pl | 6 +- > scripts/mod/modpost.c | 39 +----------- > scripts/mod/modpost.h | 2 - > scripts/module.lds.S | 4 -- > tools/include/linux/export.h | 2 - > 24 files changed, 13 insertions(+), 192 deletions(-) > >diff --git a/arch/arm/configs/bcm2835_defconfig b/arch/arm/configs/bcm2835_defconfig >index 44ff9cd88d8161..d6c6c2e031c43a 100644 >--- a/arch/arm/configs/bcm2835_defconfig >+++ b/arch/arm/configs/bcm2835_defconfig >@@ -177,7 +177,6 @@ CONFIG_BOOT_PRINTK_DELAY=y > CONFIG_DYNAMIC_DEBUG=y > CONFIG_DEBUG_INFO=y > # CONFIG_ENABLE_MUST_CHECK is not set >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_DEBUG_MEMORY_INIT=y > CONFIG_LOCKUP_DETECTOR=y > CONFIG_SCHED_TRACER=y >diff --git a/arch/arm/configs/mxs_defconfig b/arch/arm/configs/mxs_defconfig >index a9c6f32a9b1c9d..ca32446b187f5d 100644 >--- a/arch/arm/configs/mxs_defconfig >+++ b/arch/arm/configs/mxs_defconfig >@@ -164,7 +164,6 @@ CONFIG_FONTS=y > CONFIG_PRINTK_TIME=y > CONFIG_DEBUG_INFO=y > CONFIG_FRAME_WARN=2048 >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_MAGIC_SYSRQ=y > CONFIG_DEBUG_KERNEL=y > CONFIG_SOFTLOCKUP_DETECTOR=y >diff --git a/arch/mips/configs/nlm_xlp_defconfig b/arch/mips/configs/nlm_xlp_defconfig >index 72a211d2d556fd..32c29061172325 100644 >--- a/arch/mips/configs/nlm_xlp_defconfig >+++ b/arch/mips/configs/nlm_xlp_defconfig >@@ -549,7 +549,6 @@ CONFIG_PRINTK_TIME=y > CONFIG_DEBUG_INFO=y > # CONFIG_ENABLE_MUST_CHECK is not set > CONFIG_FRAME_WARN=1024 >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_DEBUG_MEMORY_INIT=y > CONFIG_DETECT_HUNG_TASK=y > CONFIG_SCHEDSTATS=y >diff --git a/arch/mips/configs/nlm_xlr_defconfig b/arch/mips/configs/nlm_xlr_defconfig >index 4ecb157e56d427..bf9b9244929ecd 100644 >--- a/arch/mips/configs/nlm_xlr_defconfig >+++ b/arch/mips/configs/nlm_xlr_defconfig >@@ -500,7 +500,6 @@ CONFIG_CRC7=m > CONFIG_PRINTK_TIME=y > CONFIG_DEBUG_INFO=y > # CONFIG_ENABLE_MUST_CHECK is not set >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_DEBUG_MEMORY_INIT=y > CONFIG_DETECT_HUNG_TASK=y > CONFIG_SCHEDSTATS=y >diff --git a/arch/parisc/configs/generic-32bit_defconfig b/arch/parisc/configs/generic-32bit_defconfig >index 3cbcfad5f7249d..7611d48c599e01 100644 >--- a/arch/parisc/configs/generic-32bit_defconfig >+++ b/arch/parisc/configs/generic-32bit_defconfig >@@ -22,7 +22,6 @@ CONFIG_PCI_LBA=y > CONFIG_MODULES=y > CONFIG_MODULE_UNLOAD=y > CONFIG_MODULE_FORCE_UNLOAD=y >-CONFIG_UNUSED_SYMBOLS=y > # CONFIG_BLK_DEV_BSG is not set > # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set > CONFIG_BINFMT_MISC=m >diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig >index 8f81fcbf04c413..53054b81461a10 100644 >--- a/arch/parisc/configs/generic-64bit_defconfig >+++ b/arch/parisc/configs/generic-64bit_defconfig >@@ -31,7 +31,6 @@ CONFIG_MODULE_FORCE_LOAD=y > CONFIG_MODULE_UNLOAD=y > CONFIG_MODULE_FORCE_UNLOAD=y > CONFIG_MODVERSIONS=y >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_BLK_DEV_INTEGRITY=y > CONFIG_BINFMT_MISC=m > # CONFIG_COMPACTION is not set >diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig >index ef09f3cce1fa85..34c3859040f9f7 100644 >--- a/arch/powerpc/configs/ppc6xx_defconfig >+++ b/arch/powerpc/configs/ppc6xx_defconfig >@@ -1072,7 +1072,6 @@ CONFIG_NLS_ISO8859_15=m > CONFIG_NLS_KOI8_R=m > CONFIG_NLS_KOI8_U=m > CONFIG_DEBUG_INFO=y >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_HEADERS_INSTALL=y > CONFIG_MAGIC_SYSRQ=y > CONFIG_DEBUG_KERNEL=y >diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig >index c4f6ff98a612cd..58e54d17e3154b 100644 >--- a/arch/s390/configs/debug_defconfig >+++ b/arch/s390/configs/debug_defconfig >@@ -71,7 +71,6 @@ CONFIG_MODULE_FORCE_UNLOAD=y > CONFIG_MODVERSIONS=y > CONFIG_MODULE_SRCVERSION_ALL=y > CONFIG_MODULE_SIG_SHA256=y >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_BLK_DEV_INTEGRITY=y > CONFIG_BLK_DEV_THROTTLING=y > CONFIG_BLK_WBT=y >diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig >index 51135893cffe34..b5e62c0d3e23e0 100644 >--- a/arch/s390/configs/defconfig >+++ b/arch/s390/configs/defconfig >@@ -66,7 +66,6 @@ CONFIG_MODULE_FORCE_UNLOAD=y > CONFIG_MODVERSIONS=y > CONFIG_MODULE_SRCVERSION_ALL=y > CONFIG_MODULE_SIG_SHA256=y >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_BLK_DEV_THROTTLING=y > CONFIG_BLK_WBT=y > CONFIG_BLK_CGROUP_IOLATENCY=y >diff --git a/arch/sh/configs/edosk7760_defconfig b/arch/sh/configs/edosk7760_defconfig >index 02ba622985769d..d77f54e906fd04 100644 >--- a/arch/sh/configs/edosk7760_defconfig >+++ b/arch/sh/configs/edosk7760_defconfig >@@ -102,7 +102,6 @@ CONFIG_NLS_UTF8=y > CONFIG_PRINTK_TIME=y > # CONFIG_ENABLE_MUST_CHECK is not set > CONFIG_MAGIC_SYSRQ=y >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_DEBUG_KERNEL=y > CONFIG_DEBUG_SHIRQ=y > CONFIG_DETECT_HUNG_TASK=y >diff --git a/arch/sh/configs/sdk7780_defconfig b/arch/sh/configs/sdk7780_defconfig >index d10a0414123a51..d53c4595fb2e98 100644 >--- a/arch/sh/configs/sdk7780_defconfig >+++ b/arch/sh/configs/sdk7780_defconfig >@@ -130,7 +130,6 @@ CONFIG_NLS_ISO8859_15=y > CONFIG_NLS_UTF8=y > # CONFIG_ENABLE_MUST_CHECK is not set > CONFIG_MAGIC_SYSRQ=y >-CONFIG_UNUSED_SYMBOLS=y > CONFIG_DEBUG_KERNEL=y > CONFIG_DETECT_HUNG_TASK=y > # CONFIG_SCHED_DEBUG is not set >diff --git a/arch/x86/configs/i386_defconfig b/arch/x86/configs/i386_defconfig >index 78210793d357cf..9c9c4a888b1dbf 100644 >--- a/arch/x86/configs/i386_defconfig >+++ b/arch/x86/configs/i386_defconfig >@@ -50,7 +50,6 @@ CONFIG_JUMP_LABEL=y > CONFIG_MODULES=y > CONFIG_MODULE_UNLOAD=y > CONFIG_MODULE_FORCE_UNLOAD=y >-# CONFIG_UNUSED_SYMBOLS is not set > CONFIG_BINFMT_MISC=y > CONFIG_NET=y > CONFIG_PACKET=y >diff --git a/arch/x86/configs/x86_64_defconfig b/arch/x86/configs/x86_64_defconfig >index 9936528e19393a..b60bd2d8603499 100644 >--- a/arch/x86/configs/x86_64_defconfig >+++ b/arch/x86/configs/x86_64_defconfig >@@ -48,7 +48,6 @@ CONFIG_JUMP_LABEL=y > CONFIG_MODULES=y > CONFIG_MODULE_UNLOAD=y > CONFIG_MODULE_FORCE_UNLOAD=y >-# CONFIG_UNUSED_SYMBOLS is not set > CONFIG_BINFMT_MISC=y > CONFIG_NET=y > CONFIG_PACKET=y >diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c >index 0d210d0e83e241..b9c577a3cacca6 100644 >--- a/arch/x86/tools/relocs.c >+++ b/arch/x86/tools/relocs.c >@@ -61,8 +61,8 @@ static const char * const sym_regex_kernel[S_NSYMTYPES] = { > "(__iommu_table|__apicdrivers|__smp_locks)(|_end)|" > "__(start|end)_pci_.*|" > "__(start|end)_builtin_fw|" >- "__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl)|" >- "__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl)|" >+ "__(start|stop)___ksymtab(|_gpl)|" >+ "__(start|stop)___kcrctab(|_gpl)|" > "__(start|stop)___param|" > "__(start|stop)___modver|" > "__(start|stop)___bug_table|" >diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h >index 83243506e68b00..1fa338ac6a5477 100644 >--- a/include/asm-generic/vmlinux.lds.h >+++ b/include/asm-generic/vmlinux.lds.h >@@ -481,20 +481,6 @@ > __stop___ksymtab_gpl = .; \ > } \ > \ >- /* Kernel symbol table: Normal unused symbols */ \ >- __ksymtab_unused : AT(ADDR(__ksymtab_unused) - LOAD_OFFSET) { \ >- __start___ksymtab_unused = .; \ >- KEEP(*(SORT(___ksymtab_unused+*))) \ >- __stop___ksymtab_unused = .; \ >- } \ >- \ >- /* Kernel symbol table: GPL-only unused symbols */ \ >- __ksymtab_unused_gpl : AT(ADDR(__ksymtab_unused_gpl) - LOAD_OFFSET) { \ >- __start___ksymtab_unused_gpl = .; \ >- KEEP(*(SORT(___ksymtab_unused_gpl+*))) \ >- __stop___ksymtab_unused_gpl = .; \ >- } \ >- \ > /* Kernel symbol table: Normal symbols */ \ > __kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \ > __start___kcrctab = .; \ >@@ -509,20 +495,6 @@ > __stop___kcrctab_gpl = .; \ > } \ > \ >- /* Kernel symbol table: Normal unused symbols */ \ >- __kcrctab_unused : AT(ADDR(__kcrctab_unused) - LOAD_OFFSET) { \ >- __start___kcrctab_unused = .; \ >- KEEP(*(SORT(___kcrctab_unused+*))) \ >- __stop___kcrctab_unused = .; \ >- } \ >- \ >- /* Kernel symbol table: GPL-only unused symbols */ \ >- __kcrctab_unused_gpl : AT(ADDR(__kcrctab_unused_gpl) - LOAD_OFFSET) { \ >- __start___kcrctab_unused_gpl = .; \ >- KEEP(*(SORT(___kcrctab_unused_gpl+*))) \ >- __stop___kcrctab_unused_gpl = .; \ >- } \ >- \ > /* Kernel symbol table: strings */ \ > __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) { \ > *(__ksymtab_strings) \ >diff --git a/include/linux/export.h b/include/linux/export.h >index 362b64f8d4a7c2..6271a5d9c988fa 100644 >--- a/include/linux/export.h >+++ b/include/linux/export.h >@@ -160,14 +160,6 @@ struct kernel_symbol { > #define EXPORT_SYMBOL_NS(sym, ns) __EXPORT_SYMBOL(sym, "", #ns) > #define EXPORT_SYMBOL_NS_GPL(sym, ns) __EXPORT_SYMBOL(sym, "_gpl", #ns) > >-#ifdef CONFIG_UNUSED_SYMBOLS >-#define EXPORT_UNUSED_SYMBOL(sym) _EXPORT_SYMBOL(sym, "_unused") >-#define EXPORT_UNUSED_SYMBOL_GPL(sym) _EXPORT_SYMBOL(sym, "_unused_gpl") >-#else >-#define EXPORT_UNUSED_SYMBOL(sym) >-#define EXPORT_UNUSED_SYMBOL_GPL(sym) >-#endif >- > #endif /* !__ASSEMBLY__ */ > > #endif /* _LINUX_EXPORT_H */ >diff --git a/include/linux/module.h b/include/linux/module.h >index 8f4d577d4707c2..0e70596c9a704a 100644 >--- a/include/linux/module.h >+++ b/include/linux/module.h >@@ -392,18 +392,6 @@ struct module { > const s32 *gpl_crcs; > bool using_gplonly_symbols; > >-#ifdef CONFIG_UNUSED_SYMBOLS >- /* unused exported symbols. */ >- const struct kernel_symbol *unused_syms; >- const s32 *unused_crcs; >- unsigned int num_unused_syms; >- >- /* GPL-only, unused exported symbols. */ >- unsigned int num_unused_gpl_syms; >- const struct kernel_symbol *unused_gpl_syms; >- const s32 *unused_gpl_crcs; >-#endif >- > #ifdef CONFIG_MODULE_SIG > /* Signature was verified. */ > bool sig_ok; >@@ -592,7 +580,6 @@ struct symsearch { > GPL_ONLY, > WILL_BE_GPL_ONLY, > } license; >- bool unused; > }; Thanks for the cleanups. While we're here, I noticed that struct symsearch is only used internally in kernel/module.c, so I don't think it actually needs to be in include/linux/module.h. I don't see it used anywhere else. We could move maybe that to kernel/module-internal.h. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <20210202121334.1361503-1-hch@lst.de>]
[parent not found: <20210202121334.1361503-2-hch@lst.de>]
* Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module [not found] ` <20210202121334.1361503-2-hch@lst.de> @ 2021-02-04 10:44 ` Michael Ellerman 0 siblings, 0 replies; 11+ messages in thread From: Michael Ellerman @ 2021-02-04 10:44 UTC (permalink / raw) To: Christoph Hellwig, Frederic Barrat, Andrew Donnellan, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter, Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence Cc: Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel, dri-devel, live-patching, linuxppc-dev Christoph Hellwig <hch@lst.de> writes: > The static inline get_cxl_module function is entirely unused since commit > 8bf6b91a5125a ("Revert "powerpc/powernv: Add support for the cxl kernel > api on the real phb"), so remove it. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> > --- > arch/powerpc/platforms/powernv/pci-cxl.c | 22 ---------------------- > 1 file changed, 22 deletions(-) Acked-by: Michael Ellerman <mpe@ellerman.id.au> cheers _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-02-04 11:21 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20210121074959.313333-1-hch@lst.de> [not found] ` <20210121074959.313333-9-hch@lst.de> 2021-01-21 8:25 ` [PATCH 08/13] drm: remove drm_fb_helper_modinit Daniel Vetter [not found] ` <20210121082820.GA25719@lst.de> 2021-01-21 8:39 ` Daniel Vetter [not found] ` <20210121074959.313333-2-hch@lst.de> 2021-01-21 9:09 ` [PATCH 01/13] powerpc/powernv: remove get_cxl_module Andrew Donnellan [not found] ` <20210121074959.313333-3-hch@lst.de> 2021-01-21 10:00 ` [PATCH 02/13] module: add a module_loaded helper Christophe Leroy [not found] ` <20210121171119.GA29916@lst.de> 2021-01-21 17:44 ` David Laight [not found] ` <20210121074959.313333-5-hch@lst.de> 2021-01-21 21:45 ` [PATCH 04/13] livepatch: move klp_find_object_module to module.c Josh Poimboeuf 2021-01-26 14:25 ` Jessica Yu 2021-01-27 11:55 ` Petr Mladek [not found] ` <20210121074959.313333-4-hch@lst.de> 2021-01-27 12:58 ` [PATCH 03/13] livepatch: refactor klp_init_object Petr Mladek [not found] ` <20210121074959.313333-14-hch@lst.de> 2021-01-27 13:49 ` [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL* Jessica Yu [not found] <20210202121334.1361503-1-hch@lst.de> [not found] ` <20210202121334.1361503-2-hch@lst.de> 2021-02-04 10:44 ` [PATCH 01/13] powerpc/powernv: remove get_cxl_module Michael Ellerman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).