Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit

Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit

[Daniel Vetter]

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

Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit

[Daniel Vetter]

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

Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module

[Andrew Donnellan]

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

Re: [PATCH 02/13] module: add a module_loaded helper

[Christophe Leroy]

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

RE: [PATCH 02/13] module: add a module_loaded helper

[David Laight]

> 
> 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

Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c

[Josh Poimboeuf]

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

Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c

[Jessica Yu]

+++ 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

Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c

[Petr Mladek]

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

Re: [PATCH 03/13] livepatch: refactor klp_init_object

[Petr Mladek]

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

Re: [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL*

[Jessica Yu]

+++ 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