live-patching.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* module loader dead code removal and cleanusp
@ 2021-01-21  7:49 Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 01/13] powerpc/powernv: remove get_cxl_module Christoph Hellwig
                   ` (12 more replies)
  0 siblings, 13 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

Hi all,

this series removes support for long term unused export types and
cleans up various loose ends in the module loader.

Diffstat:
 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/powerpc/platforms/powernv/pci-cxl.c    |   22 -
 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 
 drivers/gpu/drm/drm_crtc_helper_internal.h  |   10 
 drivers/gpu/drm/drm_fb_helper.c             |   21 -
 drivers/gpu/drm/drm_kms_helper_common.c     |   26 +-
 include/asm-generic/vmlinux.lds.h           |   42 ---
 include/linux/export.h                      |    9 
 include/linux/kallsyms.h                    |   17 -
 include/linux/module.h                      |   42 ---
 init/Kconfig                                |   17 -
 kernel/kallsyms.c                           |    8 
 kernel/livepatch/core.c                     |   61 +----
 kernel/module.c                             |  319 ++++++++++------------------
 kernel/trace/trace_kprobe.c                 |    4 
 lib/bug.c                                   |    3 
 scripts/checkpatch.pl                       |    6 
 scripts/mod/modpost.c                       |   50 ----
 scripts/mod/modpost.h                       |    3 
 scripts/module.lds.S                        |    6 
 tools/include/linux/export.h                |    3 
 33 files changed, 181 insertions(+), 505 deletions(-)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 01/13] powerpc/powernv: remove get_cxl_module
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  9:09   ` Andrew Donnellan
  2021-01-21  7:49 ` [PATCH 02/13] module: add a module_loaded helper Christoph Hellwig
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

The static inline get_cxl_module function is entirely unused,
remove it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/platforms/powernv/pci-cxl.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-cxl.c b/arch/powerpc/platforms/powernv/pci-cxl.c
index 8c739c94ed28d6..53172862d23bd3 100644
--- a/arch/powerpc/platforms/powernv/pci-cxl.c
+++ b/arch/powerpc/platforms/powernv/pci-cxl.c
@@ -150,25 +150,3 @@ int pnv_cxl_ioda_msi_setup(struct pci_dev *dev, unsigned int hwirq,
 	return 0;
 }
 EXPORT_SYMBOL(pnv_cxl_ioda_msi_setup);
-
-#if IS_MODULE(CONFIG_CXL)
-static inline int get_cxl_module(void)
-{
-	struct module *cxl_module;
-
-	mutex_lock(&module_mutex);
-
-	cxl_module = find_module("cxl");
-	if (cxl_module)
-		__module_get(cxl_module);
-
-	mutex_unlock(&module_mutex);
-
-	if (!cxl_module)
-		return -ENODEV;
-
-	return 0;
-}
-#else
-static inline int get_cxl_module(void) { return 0; }
-#endif
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 02/13] module: add a module_loaded helper
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 01/13] powerpc/powernv: remove get_cxl_module Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21 10:00   ` Christophe Leroy
  2021-01-21  7:49 ` [PATCH 03/13] livepatch: refactor klp_init_object Christoph Hellwig
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

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/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4b81195106875d..ce6d63ca75c32a 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2508,13 +2508,8 @@ int __init drm_fb_helper_modinit(void)
 {
 #if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
 	const char name[] = "fbcon";
-	struct module *fbcon;
 
-	mutex_lock(&module_mutex);
-	fbcon = find_module(name);
-	mutex_unlock(&module_mutex);
-
-	if (!fbcon)
+	if (!module_loaded(name))
 		request_module_nowait(name);
 #endif
 	return 0;
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);
+
 struct symsearch {
 	const struct kernel_symbol *start, *stop;
 	const s32 *crcs;
diff --git a/kernel/module.c b/kernel/module.c
index 4bf30e4b3eaaa1..619ea682e64cd1 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -88,7 +88,6 @@
  * (delete and add uses RCU list operations).
  */
 DEFINE_MUTEX(module_mutex);
-EXPORT_SYMBOL_GPL(module_mutex);
 static LIST_HEAD(modules);
 
 /* Work queue for freeing init sections in success case */
@@ -672,7 +671,18 @@ struct module *find_module(const char *name)
 	module_assert_mutex();
 	return find_module_all(name, strlen(name), false);
 }
-EXPORT_SYMBOL_GPL(find_module);
+
+bool module_loaded(const char *name)
+{
+	bool ret;
+
+	mutex_lock(&module_mutex);
+	ret = !!find_module(name);
+	mutex_unlock(&module_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(module_loaded);
 
 #ifdef CONFIG_SMP
 
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e6fba1798771b4..c2e453f88bce70 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -124,9 +124,7 @@ static nokprobe_inline bool trace_kprobe_module_exist(struct trace_kprobe *tk)
 	if (!p)
 		return true;
 	*p = '\0';
-	mutex_lock(&module_mutex);
-	ret = !!find_module(tk->symbol);
-	mutex_unlock(&module_mutex);
+	ret = module_loaded(tk->symbol);
 	*p = ':';
 
 	return ret;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 03/13] livepatch: refactor klp_init_object
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 01/13] powerpc/powernv: remove get_cxl_module Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 02/13] module: add a module_loaded helper Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-27 12:58   ` Petr Mladek
  2021-01-21  7:49 ` [PATCH 04/13] livepatch: move klp_find_object_module to module.c Christoph Hellwig
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

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;
-
 	mutex_lock(&module_mutex);
 	/*
 	 * We do not want to block removal of patched modules and therefore
@@ -73,7 +70,6 @@ static void klp_find_object_module(struct klp_object *obj)
 	 */
 	if (mod && mod->klp_alive)
 		obj->mod = mod;
-
 	mutex_unlock(&module_mutex);
 }
 
@@ -823,15 +819,19 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
 	int ret;
 	const char *name;
 
-	if (klp_is_module(obj) && strlen(obj->name) >= MODULE_NAME_LEN)
-		return -EINVAL;
-
 	obj->patched = false;
 	obj->mod = NULL;
 
-	klp_find_object_module(obj);
+	if (klp_is_module(obj)) {
+		if (strlen(obj->name) >= MODULE_NAME_LEN)
+			return -EINVAL;
+		name = obj->name;
+
+		klp_find_object_module(obj);
+	} else {
+		name = "vmlinux";
+	}
 
-	name = klp_is_module(obj) ? obj->name : "vmlinux";
 	ret = kobject_add(&obj->kobj, &patch->kobj, "%s", name);
 	if (ret)
 		return ret;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 04/13] livepatch: move klp_find_object_module to module.c
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 03/13] livepatch: refactor klp_init_object Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21 21:45   ` Josh Poimboeuf
  2021-01-26 14:25   ` Jessica Yu
  2021-01-21  7:49 ` [PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol Christoph Hellwig
                   ` (8 subsequent siblings)
  12 siblings, 2 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

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);
-}
-
 static bool klp_initialized(void)
 {
 	return !!klp_root_kobj;
@@ -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;
 
 	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);
 	} else {
 		name = "vmlinux";
 	}
diff --git a/kernel/module.c b/kernel/module.c
index 619ea682e64cd1..299cbac0775cf2 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -666,7 +666,7 @@ static struct module *find_module_all(const char *name, size_t len,
 	return NULL;
 }
 
-struct module *find_module(const char *name)
+static struct module *find_module(const char *name)
 {
 	module_assert_mutex();
 	return find_module_all(name, strlen(name), false);
@@ -684,6 +684,21 @@ bool module_loaded(const char *name)
 }
 EXPORT_SYMBOL_GPL(module_loaded);
 
+#ifdef CONFIG_LIVEPATCH
+struct module *find_klp_module(const char *name)
+{
+	struct module *mod;
+
+	mutex_lock(&module_mutex);
+	mod = find_module(name);
+	if (mod && !mod->klp_alive)
+		mod = NULL;
+	mutex_unlock(&module_mutex);
+
+	return mod;
+}
+#endif /* CONFIG_LIVEPATCH */
+
 #ifdef CONFIG_SMP
 
 static inline void __percpu *mod_percpu(struct module *mod)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (3 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 04/13] livepatch: move klp_find_object_module to module.c Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 06/13] kallsyms: only build {,module_}kallsyms_on_each_symbol when required Christoph Hellwig
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

Require an explicit cll to module_kallsyms_on_each_symbol to look
for symbols in modules instead of the call from kallsyms_on_each_symbol,
and acquire module_mutex inside of module_kallsyms_on_each_symbol instead
of leaving that up to the caller.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 kernel/kallsyms.c       | 6 +++++-
 kernel/livepatch/core.c | 6 +-----
 kernel/module.c         | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index fe9de067771c34..a0d3f0865916f9 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -177,6 +177,10 @@ unsigned long kallsyms_lookup_name(const char *name)
 	return module_kallsyms_lookup_name(name);
 }
 
+/*
+ * Iterate over all symbols in vmlinux.  For symbols from modules use
+ * module_kallsyms_on_each_symbol instead.
+ */
 int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 				      unsigned long),
 			    void *data)
@@ -192,7 +196,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 		if (ret != 0)
 			return ret;
 	}
-	return module_kallsyms_on_each_symbol(fn, data);
+	return 0;
 }
 
 static unsigned long get_symbol_pos(unsigned long addr,
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 878759baadd81c..8063b9089bd2f8 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -135,12 +135,8 @@ static int klp_find_object_symbol(const char *objname, const char *name,
 		.pos = sympos,
 	};
 
-	mutex_lock(&module_mutex);
-	if (objname)
+	if (objname || !kallsyms_on_each_symbol(klp_find_callback, &args))
 		module_kallsyms_on_each_symbol(klp_find_callback, &args);
-	else
-		kallsyms_on_each_symbol(klp_find_callback, &args);
-	mutex_unlock(&module_mutex);
 
 	/*
 	 * Ensure an address was found. If sympos is 0, ensure symbol is unique;
diff --git a/kernel/module.c b/kernel/module.c
index 299cbac0775cf2..885feec64c1b6f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4407,8 +4407,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
 	unsigned int i;
 	int ret;
 
-	module_assert_mutex();
-
+	mutex_lock(&module_mutex);
 	list_for_each_entry(mod, &modules, list) {
 		/* We hold module_mutex: no need for rcu_dereference_sched */
 		struct mod_kallsyms *kallsyms = mod->kallsyms;
@@ -4424,10 +4423,11 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
 			ret = fn(data, kallsyms_symbol_name(kallsyms, i),
 				 mod, kallsyms_symbol_value(sym));
 			if (ret != 0)
-				return ret;
+				break;
 		}
 	}
-	return 0;
+	mutex_unlock(&module_mutex);
+	return ret;
 }
 #endif /* CONFIG_KALLSYMS */
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 06/13] kallsyms: only build {,module_}kallsyms_on_each_symbol when required
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (4 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 07/13] module: mark module_mutex static Christoph Hellwig
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

kallsyms_on_each_symbol and module_kallsyms_on_each_symbol are only used
by the livepatching code, so don't build them if livepatching is not
enabled.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/kallsyms.h | 17 ++++-------------
 include/linux/module.h   | 16 ++++------------
 kernel/kallsyms.c        |  2 ++
 kernel/module.c          |  2 ++
 4 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 481273f0c72d42..465060acc9816f 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -71,15 +71,14 @@ static inline void *dereference_symbol_descriptor(void *ptr)
 	return ptr;
 }
 
-#ifdef CONFIG_KALLSYMS
-/* Lookup the address for a symbol. Returns 0 if not found. */
-unsigned long kallsyms_lookup_name(const char *name);
-
-/* Call a function on each kallsyms symbol in the core kernel */
 int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 				      unsigned long),
 			    void *data);
 
+#ifdef CONFIG_KALLSYMS
+/* Lookup the address for a symbol. Returns 0 if not found. */
+unsigned long kallsyms_lookup_name(const char *name);
+
 extern int kallsyms_lookup_size_offset(unsigned long addr,
 				  unsigned long *symbolsize,
 				  unsigned long *offset);
@@ -108,14 +107,6 @@ static inline unsigned long kallsyms_lookup_name(const char *name)
 	return 0;
 }
 
-static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
-						    struct module *,
-						    unsigned long),
-					  void *data)
-{
-	return 0;
-}
-
 static inline int kallsyms_lookup_size_offset(unsigned long addr,
 					      unsigned long *symbolsize,
 					      unsigned long *offset)
diff --git a/include/linux/module.h b/include/linux/module.h
index 8588482bde4116..695f127745af10 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -610,10 +610,6 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
 /* Look for this name: can be of form module:name. */
 unsigned long module_kallsyms_lookup_name(const char *name);
 
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
-					     struct module *, unsigned long),
-				   void *data);
-
 extern void __noreturn __module_put_and_exit(struct module *mod,
 			long code);
 #define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)
@@ -797,14 +793,6 @@ static inline unsigned long module_kallsyms_lookup_name(const char *name)
 	return 0;
 }
 
-static inline int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
-							   struct module *,
-							   unsigned long),
-						 void *data)
-{
-	return 0;
-}
-
 static inline int register_module_notifier(struct notifier_block *nb)
 {
 	/* no events will happen anyway, so this can always succeed */
@@ -893,4 +881,8 @@ static inline bool module_sig_ok(struct module *module)
 }
 #endif	/* CONFIG_MODULE_SIG */
 
+int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
+					     struct module *, unsigned long),
+				   void *data);
+
 #endif /* _LINUX_MODULE_H */
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index a0d3f0865916f9..8043a90aa50ed3 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -177,6 +177,7 @@ unsigned long kallsyms_lookup_name(const char *name)
 	return module_kallsyms_lookup_name(name);
 }
 
+#ifdef CONFIG_LIVEPATCH
 /*
  * Iterate over all symbols in vmlinux.  For symbols from modules use
  * module_kallsyms_on_each_symbol instead.
@@ -198,6 +199,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 	}
 	return 0;
 }
+#endif /* CONFIG_LIVEPATCH */
 
 static unsigned long get_symbol_pos(unsigned long addr,
 				    unsigned long *symbolsize,
diff --git a/kernel/module.c b/kernel/module.c
index 885feec64c1b6f..e141e5d1d7beaf 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4399,6 +4399,7 @@ unsigned long module_kallsyms_lookup_name(const char *name)
 	return ret;
 }
 
+#ifdef CONFIG_LIVEPATCH
 int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
 					     struct module *, unsigned long),
 				   void *data)
@@ -4429,6 +4430,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
 	mutex_unlock(&module_mutex);
 	return ret;
 }
+#endif /* CONFIG_LIVEPATCH */
 #endif /* CONFIG_KALLSYMS */
 
 /* Maximum number of characters written by module_flags() */
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 07/13] module: mark module_mutex static
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (5 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 06/13] kallsyms: only build {,module_}kallsyms_on_each_symbol when required Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 08/13] drm: remove drm_fb_helper_modinit Christoph Hellwig
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

Except for two lockdep asserts module_mutex is only used in module.c.
Remove the two asserts given that the functions they are in are not
exported and just called from the module code, and mark module_mutex
static.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/module.h | 2 --
 kernel/module.c        | 2 +-
 lib/bug.c              | 3 ---
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 695f127745af10..c92c30a285144f 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -550,8 +550,6 @@ static inline unsigned long kallsyms_symbol_value(const Elf_Sym *sym)
 }
 #endif
 
-extern struct mutex module_mutex;
-
 /* FIXME: It'd be nice to isolate modules during init, too, so they
    aren't used before they (may) fail.  But presently too much code
    (IDE & SCSI) require entry into the module during init.*/
diff --git a/kernel/module.c b/kernel/module.c
index e141e5d1d7beaf..d163c78ca8ed69 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -87,7 +87,7 @@
  * 3) module_addr_min/module_addr_max.
  * (delete and add uses RCU list operations).
  */
-DEFINE_MUTEX(module_mutex);
+static DEFINE_MUTEX(module_mutex);
 static LIST_HEAD(modules);
 
 /* Work queue for freeing init sections in success case */
diff --git a/lib/bug.c b/lib/bug.c
index 7103440c0ee1af..8f9d537bfb2a59 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -91,8 +91,6 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
 	char *secstrings;
 	unsigned int i;
 
-	lockdep_assert_held(&module_mutex);
-
 	mod->bug_table = NULL;
 	mod->num_bugs = 0;
 
@@ -118,7 +116,6 @@ void module_bug_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
 
 void module_bug_cleanup(struct module *mod)
 {
-	lockdep_assert_held(&module_mutex);
 	list_del_rcu(&mod->bug_list);
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 08/13] drm: remove drm_fb_helper_modinit
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (6 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 07/13] module: mark module_mutex static Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  8:25   ` Daniel Vetter
  2021-01-21  7:49 ` [PATCH 09/13] module: remove each_symbol_in_section Christoph Hellwig
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

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


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 09/13] module: remove each_symbol_in_section
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (7 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 08/13] drm: remove drm_fb_helper_modinit Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 10/13] module: merge each_symbol_section into find_symbol Christoph Hellwig
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

each_symbol_in_section just contains a trivial loop over its arguments.
Just open code the loop in the two callers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 kernel/module.c | 29 +++++++----------------------
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index d163c78ca8ed69..a9d092765c4eab 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,30 +433,13 @@ extern const s32 __start___kcrctab_unused_gpl[];
 #define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
 #endif
 
-static bool each_symbol_in_section(const struct symsearch *arr,
-				   unsigned int arrsize,
-				   struct module *owner,
-				   bool (*fn)(const struct symsearch *syms,
-					      struct module *owner,
-					      void *data),
-				   void *data)
-{
-	unsigned int j;
-
-	for (j = 0; j < arrsize; j++) {
-		if (fn(&arr[j], owner, data))
-			return true;
-	}
-
-	return false;
-}
-
 /* Returns true as soon as fn returns true, otherwise false. */
 static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
 				    struct module *owner,
 				    void *data),
 			 void *data)
 {
+	unsigned int i;
 	struct module *mod;
 	static const struct symsearch arr[] = {
 		{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
@@ -479,8 +462,9 @@ static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
 
 	module_assert_mutex_or_preempt();
 
-	if (each_symbol_in_section(arr, ARRAY_SIZE(arr), NULL, fn, data))
-		return true;
+	for (i = 0; i < ARRAY_SIZE(arr); i++)
+		if (fn(&arr[i], NULL, data))
+			return true;
 
 	list_for_each_entry_rcu(mod, &modules, list,
 				lockdep_is_held(&module_mutex)) {
@@ -509,8 +493,9 @@ static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
 
-		if (each_symbol_in_section(arr, ARRAY_SIZE(arr), mod, fn, data))
-			return true;
+		for (i = 0; i < ARRAY_SIZE(arr); i++)
+			if (fn(&arr[i], mod, data))
+				return true;
 	}
 	return false;
 }
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 10/13] module: merge each_symbol_section into find_symbol
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (8 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 09/13] module: remove each_symbol_in_section Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 11/13] module: pass struct find_symbol_args to find_symbol Christoph Hellwig
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

each_symbol_section is only called by find_symbol, so merge the two
functions.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 kernel/module.c | 148 ++++++++++++++++++++++--------------------------
 1 file changed, 69 insertions(+), 79 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index a9d092765c4eab..644dda52dae38c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -433,73 +433,6 @@ extern const s32 __start___kcrctab_unused_gpl[];
 #define symversion(base, idx) ((base != NULL) ? ((base) + (idx)) : NULL)
 #endif
 
-/* Returns true as soon as fn returns true, otherwise false. */
-static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
-				    struct module *owner,
-				    void *data),
-			 void *data)
-{
-	unsigned int i;
-	struct module *mod;
-	static const struct symsearch arr[] = {
-		{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
-		  NOT_GPL_ONLY, false },
-		{ __start___ksymtab_gpl, __stop___ksymtab_gpl,
-		  __start___kcrctab_gpl,
-		  GPL_ONLY, false },
-		{ __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
-		  __start___kcrctab_gpl_future,
-		  WILL_BE_GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
-		{ __start___ksymtab_unused, __stop___ksymtab_unused,
-		  __start___kcrctab_unused,
-		  NOT_GPL_ONLY, true },
-		{ __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
-		  __start___kcrctab_unused_gpl,
-		  GPL_ONLY, true },
-#endif
-	};
-
-	module_assert_mutex_or_preempt();
-
-	for (i = 0; i < ARRAY_SIZE(arr); i++)
-		if (fn(&arr[i], NULL, data))
-			return true;
-
-	list_for_each_entry_rcu(mod, &modules, list,
-				lockdep_is_held(&module_mutex)) {
-		struct symsearch arr[] = {
-			{ mod->syms, mod->syms + mod->num_syms, mod->crcs,
-			  NOT_GPL_ONLY, false },
-			{ mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
-			  mod->gpl_crcs,
-			  GPL_ONLY, false },
-			{ mod->gpl_future_syms,
-			  mod->gpl_future_syms + mod->num_gpl_future_syms,
-			  mod->gpl_future_crcs,
-			  WILL_BE_GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
-			{ mod->unused_syms,
-			  mod->unused_syms + mod->num_unused_syms,
-			  mod->unused_crcs,
-			  NOT_GPL_ONLY, true },
-			{ mod->unused_gpl_syms,
-			  mod->unused_gpl_syms + mod->num_unused_gpl_syms,
-			  mod->unused_gpl_crcs,
-			  GPL_ONLY, true },
-#endif
-		};
-
-		if (mod->state == MODULE_STATE_UNFORMED)
-			continue;
-
-		for (i = 0; i < ARRAY_SIZE(arr); i++)
-			if (fn(&arr[i], mod, data))
-				return true;
-	}
-	return false;
-}
-
 struct find_symbol_arg {
 	/* Input */
 	const char *name;
@@ -610,24 +543,81 @@ static const struct kernel_symbol *find_symbol(const char *name,
 					bool gplok,
 					bool warn)
 {
-	struct find_symbol_arg fsa;
+	static const struct symsearch arr[] = {
+		{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
+		  NOT_GPL_ONLY, false },
+		{ __start___ksymtab_gpl, __stop___ksymtab_gpl,
+		  __start___kcrctab_gpl,
+		  GPL_ONLY, false },
+		{ __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
+		  __start___kcrctab_gpl_future,
+		  WILL_BE_GPL_ONLY, false },
+#ifdef CONFIG_UNUSED_SYMBOLS
+		{ __start___ksymtab_unused, __stop___ksymtab_unused,
+		  __start___kcrctab_unused,
+		  NOT_GPL_ONLY, true },
+		{ __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
+		  __start___kcrctab_unused_gpl,
+		  GPL_ONLY, true },
+#endif
+	};
+	struct find_symbol_arg fsa = {
+		.name = name,
+		.gplok = gplok,
+		.warn = warn,
+	};
+	struct module *mod;
+	unsigned int i;
+
+	module_assert_mutex_or_preempt();
+
+	for (i = 0; i < ARRAY_SIZE(arr); i++)
+		if (find_exported_symbol_in_section(&arr[i], NULL, &fsa))
+			goto found;
 
-	fsa.name = name;
-	fsa.gplok = gplok;
-	fsa.warn = warn;
+	list_for_each_entry_rcu(mod, &modules, list,
+				lockdep_is_held(&module_mutex)) {
+		struct symsearch arr[] = {
+			{ mod->syms, mod->syms + mod->num_syms, mod->crcs,
+			  NOT_GPL_ONLY, false },
+			{ mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
+			  mod->gpl_crcs,
+			  GPL_ONLY, false },
+			{ mod->gpl_future_syms,
+			  mod->gpl_future_syms + mod->num_gpl_future_syms,
+			  mod->gpl_future_crcs,
+			  WILL_BE_GPL_ONLY, false },
+#ifdef CONFIG_UNUSED_SYMBOLS
+			{ mod->unused_syms,
+			  mod->unused_syms + mod->num_unused_syms,
+			  mod->unused_crcs,
+			  NOT_GPL_ONLY, true },
+			{ mod->unused_gpl_syms,
+			  mod->unused_gpl_syms + mod->num_unused_gpl_syms,
+			  mod->unused_gpl_crcs,
+			  GPL_ONLY, true },
+#endif
+		};
 
-	if (each_symbol_section(find_exported_symbol_in_section, &fsa)) {
-		if (owner)
-			*owner = fsa.owner;
-		if (crc)
-			*crc = fsa.crc;
-		if (license)
-			*license = fsa.license;
-		return fsa.sym;
+		if (mod->state == MODULE_STATE_UNFORMED)
+			continue;
+
+		for (i = 0; i < ARRAY_SIZE(arr); i++)
+			if (find_exported_symbol_in_section(&arr[i], mod, &fsa))
+				goto found;
 	}
 
 	pr_debug("Failed to find symbol %s\n", name);
 	return NULL;
+
+found:
+	if (owner)
+		*owner = fsa.owner;
+	if (crc)
+		*crc = fsa.crc;
+	if (license)
+		*license = fsa.license;
+	return fsa.sym;
 }
 
 /*
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 11/13] module: pass struct find_symbol_args to find_symbol
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (9 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 10/13] module: merge each_symbol_section into find_symbol Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 12/13] module: remove EXPORT_SYMBOL_GPL_FUTURE Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL* Christoph Hellwig
  12 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

Simplify the calling convention by passing the find_symbol_args structure
to find_symbol instead of initializing it inside the function.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 kernel/module.c | 113 ++++++++++++++++++++++--------------------------
 1 file changed, 52 insertions(+), 61 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 644dda52dae38c..7a88b71736ff5c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -536,12 +536,7 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
  * Find an exported symbol and return it, along with, (optional) crc and
  * (optional) module which owns it.  Needs preempt disabled or module_mutex.
  */
-static const struct kernel_symbol *find_symbol(const char *name,
-					struct module **owner,
-					const s32 **crc,
-					enum mod_license *license,
-					bool gplok,
-					bool warn)
+static bool find_symbol(struct find_symbol_arg *fsa)
 {
 	static const struct symsearch arr[] = {
 		{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
@@ -561,19 +556,14 @@ static const struct kernel_symbol *find_symbol(const char *name,
 		  GPL_ONLY, true },
 #endif
 	};
-	struct find_symbol_arg fsa = {
-		.name = name,
-		.gplok = gplok,
-		.warn = warn,
-	};
 	struct module *mod;
 	unsigned int i;
 
 	module_assert_mutex_or_preempt();
 
 	for (i = 0; i < ARRAY_SIZE(arr); i++)
-		if (find_exported_symbol_in_section(&arr[i], NULL, &fsa))
-			goto found;
+		if (find_exported_symbol_in_section(&arr[i], NULL, fsa))
+			return true;
 
 	list_for_each_entry_rcu(mod, &modules, list,
 				lockdep_is_held(&module_mutex)) {
@@ -603,21 +593,12 @@ static const struct kernel_symbol *find_symbol(const char *name,
 			continue;
 
 		for (i = 0; i < ARRAY_SIZE(arr); i++)
-			if (find_exported_symbol_in_section(&arr[i], mod, &fsa))
-				goto found;
+			if (find_exported_symbol_in_section(&arr[i], mod, fsa))
+				return true;
 	}
 
-	pr_debug("Failed to find symbol %s\n", name);
-	return NULL;
-
-found:
-	if (owner)
-		*owner = fsa.owner;
-	if (crc)
-		*crc = fsa.crc;
-	if (license)
-		*license = fsa.license;
-	return fsa.sym;
+	pr_debug("Failed to find symbol %s\n", fsa->name);
+	return false;
 }
 
 /*
@@ -1107,12 +1088,15 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
 
 void __symbol_put(const char *symbol)
 {
-	struct module *owner;
+	struct find_symbol_arg fsa = {
+		.name	= symbol,
+		.gplok	= true,
+	};
 
 	preempt_disable();
-	if (!find_symbol(symbol, &owner, NULL, NULL, true, false))
+	if (!find_symbol(&fsa))
 		BUG();
-	module_put(owner);
+	module_put(fsa.owner);
 	preempt_enable();
 }
 EXPORT_SYMBOL(__symbol_put);
@@ -1381,19 +1365,22 @@ static int check_version(const struct load_info *info,
 static inline int check_modstruct_version(const struct load_info *info,
 					  struct module *mod)
 {
-	const s32 *crc;
+	struct find_symbol_arg fsa = {
+		.name	= "module_layout",
+		.gplok	= true,
+	};
 
 	/*
 	 * Since this should be found in kernel (which can't be removed), no
 	 * locking is necessary -- use preempt_disable() to placate lockdep.
 	 */
 	preempt_disable();
-	if (!find_symbol("module_layout", NULL, &crc, NULL, true, false)) {
+	if (!find_symbol(&fsa)) {
 		preempt_enable();
 		BUG();
 	}
 	preempt_enable();
-	return check_version(info, "module_layout", mod, crc);
+	return check_version(info, "module_layout", mod, fsa.crc);
 }
 
 /* First part is kernel version, which we ignore if module has crcs. */
@@ -1487,10 +1474,11 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
 						  const char *name,
 						  char ownername[])
 {
-	struct module *owner;
-	const struct kernel_symbol *sym;
-	const s32 *crc;
-	enum mod_license license;
+	struct find_symbol_arg fsa = {
+		.name	= name,
+		.gplok	= !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)),
+		.warn	= true,
+	};
 	int err;
 
 	/*
@@ -1500,42 +1488,40 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
 	 */
 	sched_annotate_sleep();
 	mutex_lock(&module_mutex);
-	sym = find_symbol(name, &owner, &crc, &license,
-			  !(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)), true);
-	if (!sym)
+	if (!find_symbol(&fsa))
 		goto unlock;
 
-	if (license == GPL_ONLY)
+	if (fsa.license == GPL_ONLY)
 		mod->using_gplonly_symbols = true;
 
-	if (!inherit_taint(mod, owner)) {
-		sym = NULL;
+	if (!inherit_taint(mod, fsa.owner)) {
+		fsa.sym = NULL;
 		goto getname;
 	}
 
-	if (!check_version(info, name, mod, crc)) {
-		sym = ERR_PTR(-EINVAL);
+	if (!check_version(info, name, mod, fsa.crc)) {
+		fsa.sym = ERR_PTR(-EINVAL);
 		goto getname;
 	}
 
-	err = verify_namespace_is_imported(info, sym, mod);
+	err = verify_namespace_is_imported(info, fsa.sym, mod);
 	if (err) {
-		sym = ERR_PTR(err);
+		fsa.sym = ERR_PTR(err);
 		goto getname;
 	}
 
-	err = ref_module(mod, owner);
+	err = ref_module(mod, fsa.owner);
 	if (err) {
-		sym = ERR_PTR(err);
+		fsa.sym = ERR_PTR(err);
 		goto getname;
 	}
 
 getname:
 	/* We must make copy under the lock if we failed to get ref. */
-	strncpy(ownername, module_name(owner), MODULE_NAME_LEN);
+	strncpy(ownername, module_name(fsa.owner), MODULE_NAME_LEN);
 unlock:
 	mutex_unlock(&module_mutex);
-	return sym;
+	return fsa.sym;
 }
 
 static const struct kernel_symbol *
@@ -2296,16 +2282,19 @@ static void free_module(struct module *mod)
 
 void *__symbol_get(const char *symbol)
 {
-	struct module *owner;
-	const struct kernel_symbol *sym;
+	struct find_symbol_arg fsa = {
+		.name	= symbol,
+		.gplok	= true,
+		.warn	= true,
+	};
 
 	preempt_disable();
-	sym = find_symbol(symbol, &owner, NULL, NULL, true, true);
-	if (sym && strong_try_module_get(owner))
-		sym = NULL;
+	if (!find_symbol(&fsa) || !strong_try_module_get(fsa.owner)) {
+		preempt_enable();
+		return NULL;
+	}
 	preempt_enable();
-
-	return sym ? (void *)kernel_symbol_value(sym) : NULL;
+	return (void *)kernel_symbol_value(fsa.sym);
 }
 EXPORT_SYMBOL_GPL(__symbol_get);
 
@@ -2318,7 +2307,6 @@ EXPORT_SYMBOL_GPL(__symbol_get);
 static int verify_exported_symbols(struct module *mod)
 {
 	unsigned int i;
-	struct module *owner;
 	const struct kernel_symbol *s;
 	struct {
 		const struct kernel_symbol *sym;
@@ -2335,12 +2323,15 @@ static int verify_exported_symbols(struct module *mod)
 
 	for (i = 0; i < ARRAY_SIZE(arr); i++) {
 		for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) {
-			if (find_symbol(kernel_symbol_name(s), &owner, NULL,
-					NULL, true, false)) {
+			struct find_symbol_arg fsa = {
+				.name	= kernel_symbol_name(s),
+				.gplok	= true,
+			};
+			if (find_symbol(&fsa)) {
 				pr_err("%s: exports duplicate symbol %s"
 				       " (owned by %s)\n",
 				       mod->name, kernel_symbol_name(s),
-				       module_name(owner));
+				       module_name(fsa.owner));
 				return -ENOEXEC;
 			}
 		}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 12/13] module: remove EXPORT_SYMBOL_GPL_FUTURE
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (10 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 11/13] module: pass struct find_symbol_args to find_symbol Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-21  7:49 ` [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL* Christoph Hellwig
  12 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

As far as I can tell this has never been used at all, and certainly
not any time recently.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/x86/tools/relocs.c           |  4 ++--
 include/asm-generic/vmlinux.lds.h | 14 --------------
 include/linux/export.h            |  1 -
 include/linux/module.h            |  5 -----
 kernel/module.c                   | 17 -----------------
 scripts/mod/modpost.c             | 13 +------------
 scripts/mod/modpost.h             |  1 -
 scripts/module.lds.S              |  2 --
 tools/include/linux/export.h      |  1 -
 9 files changed, 3 insertions(+), 55 deletions(-)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae58a..0d210d0e83e241 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|_gpl_future)|"
-	"__(start|stop)___kcrctab(|_gpl|_unused|_unused_gpl|_gpl_future)|"
+	"__(start|stop)___ksymtab(|_gpl|_unused|_unused_gpl)|"
+	"__(start|stop)___kcrctab(|_gpl|_unused|_unused_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 b2b3d81b1535a5..83243506e68b00 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -495,13 +495,6 @@
 		__stop___ksymtab_unused_gpl = .;			\
 	}								\
 									\
-	/* Kernel symbol table: GPL-future-only symbols */		\
-	__ksymtab_gpl_future : AT(ADDR(__ksymtab_gpl_future) - LOAD_OFFSET) { \
-		__start___ksymtab_gpl_future = .;			\
-		KEEP(*(SORT(___ksymtab_gpl_future+*)))			\
-		__stop___ksymtab_gpl_future = .;			\
-	}								\
-									\
 	/* Kernel symbol table: Normal symbols */			\
 	__kcrctab         : AT(ADDR(__kcrctab) - LOAD_OFFSET) {		\
 		__start___kcrctab = .;					\
@@ -530,13 +523,6 @@
 		__stop___kcrctab_unused_gpl = .;			\
 	}								\
 									\
-	/* Kernel symbol table: GPL-future-only symbols */		\
-	__kcrctab_gpl_future : AT(ADDR(__kcrctab_gpl_future) - LOAD_OFFSET) { \
-		__start___kcrctab_gpl_future = .;			\
-		KEEP(*(SORT(___kcrctab_gpl_future+*)))			\
-		__stop___kcrctab_gpl_future = .;			\
-	}								\
-									\
 	/* 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 fceb5e85571711..362b64f8d4a7c2 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -157,7 +157,6 @@ struct kernel_symbol {
 
 #define EXPORT_SYMBOL(sym)		_EXPORT_SYMBOL(sym, "")
 #define EXPORT_SYMBOL_GPL(sym)		_EXPORT_SYMBOL(sym, "_gpl")
-#define EXPORT_SYMBOL_GPL_FUTURE(sym)	_EXPORT_SYMBOL(sym, "_gpl_future")
 #define EXPORT_SYMBOL_NS(sym, ns)	__EXPORT_SYMBOL(sym, "", #ns)
 #define EXPORT_SYMBOL_NS_GPL(sym, ns)	__EXPORT_SYMBOL(sym, "_gpl", #ns)
 
diff --git a/include/linux/module.h b/include/linux/module.h
index c92c30a285144f..8f4d577d4707c2 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -411,11 +411,6 @@ struct module {
 
 	bool async_probe_requested;
 
-	/* symbols that will be GPL-only in the near future. */
-	const struct kernel_symbol *gpl_future_syms;
-	const s32 *gpl_future_crcs;
-	unsigned int num_gpl_future_syms;
-
 	/* Exception table */
 	unsigned int num_exentries;
 	struct exception_table_entry *extable;
diff --git a/kernel/module.c b/kernel/module.c
index 7a88b71736ff5c..917fd1b5c95a42 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -413,11 +413,8 @@ extern const struct kernel_symbol __start___ksymtab[];
 extern const struct kernel_symbol __stop___ksymtab[];
 extern const struct kernel_symbol __start___ksymtab_gpl[];
 extern const struct kernel_symbol __stop___ksymtab_gpl[];
-extern const struct kernel_symbol __start___ksymtab_gpl_future[];
-extern const struct kernel_symbol __stop___ksymtab_gpl_future[];
 extern const s32 __start___kcrctab[];
 extern const s32 __start___kcrctab_gpl[];
-extern const s32 __start___kcrctab_gpl_future[];
 #ifdef CONFIG_UNUSED_SYMBOLS
 extern const struct kernel_symbol __start___ksymtab_unused[];
 extern const struct kernel_symbol __stop___ksymtab_unused[];
@@ -544,9 +541,6 @@ static bool find_symbol(struct find_symbol_arg *fsa)
 		{ __start___ksymtab_gpl, __stop___ksymtab_gpl,
 		  __start___kcrctab_gpl,
 		  GPL_ONLY, false },
-		{ __start___ksymtab_gpl_future, __stop___ksymtab_gpl_future,
-		  __start___kcrctab_gpl_future,
-		  WILL_BE_GPL_ONLY, false },
 #ifdef CONFIG_UNUSED_SYMBOLS
 		{ __start___ksymtab_unused, __stop___ksymtab_unused,
 		  __start___kcrctab_unused,
@@ -573,10 +567,6 @@ static bool find_symbol(struct find_symbol_arg *fsa)
 			{ mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
 			  mod->gpl_crcs,
 			  GPL_ONLY, false },
-			{ mod->gpl_future_syms,
-			  mod->gpl_future_syms + mod->num_gpl_future_syms,
-			  mod->gpl_future_crcs,
-			  WILL_BE_GPL_ONLY, false },
 #ifdef CONFIG_UNUSED_SYMBOLS
 			{ mod->unused_syms,
 			  mod->unused_syms + mod->num_unused_syms,
@@ -2314,7 +2304,6 @@ static int verify_exported_symbols(struct module *mod)
 	} arr[] = {
 		{ mod->syms, mod->num_syms },
 		{ mod->gpl_syms, mod->num_gpl_syms },
-		{ mod->gpl_future_syms, mod->num_gpl_future_syms },
 #ifdef CONFIG_UNUSED_SYMBOLS
 		{ mod->unused_syms, mod->num_unused_syms },
 		{ mod->unused_gpl_syms, mod->num_unused_gpl_syms },
@@ -3232,11 +3221,6 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 				     sizeof(*mod->gpl_syms),
 				     &mod->num_gpl_syms);
 	mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
-	mod->gpl_future_syms = section_objs(info,
-					    "__ksymtab_gpl_future",
-					    sizeof(*mod->gpl_future_syms),
-					    &mod->num_gpl_future_syms);
-	mod->gpl_future_crcs = section_addr(info, "__kcrctab_gpl_future");
 
 #ifdef CONFIG_UNUSED_SYMBOLS
 	mod->unused_syms = section_objs(info, "__ksymtab_unused",
@@ -3430,7 +3414,6 @@ static int check_module_license_and_versions(struct module *mod)
 #ifdef CONFIG_MODVERSIONS
 	if ((mod->num_syms && !mod->crcs)
 	    || (mod->num_gpl_syms && !mod->gpl_crcs)
-	    || (mod->num_gpl_future_syms && !mod->gpl_future_crcs)
 #ifdef CONFIG_UNUSED_SYMBOLS
 	    || (mod->num_unused_syms && !mod->unused_crcs)
 	    || (mod->num_unused_gpl_syms && !mod->unused_gpl_crcs)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index d6c81657d69550..25c1446055d16b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -44,7 +44,7 @@ static bool error_occurred;
 
 enum export {
 	export_plain,      export_unused,     export_gpl,
-	export_unused_gpl, export_gpl_future, export_unknown
+	export_unused_gpl, export_unknown
 };
 
 /* In kernel, this size is defined in linux/module.h;
@@ -304,7 +304,6 @@ static const struct {
 	{ .str = "EXPORT_UNUSED_SYMBOL",     .export = export_unused },
 	{ .str = "EXPORT_SYMBOL_GPL",        .export = export_gpl },
 	{ .str = "EXPORT_UNUSED_SYMBOL_GPL", .export = export_unused_gpl },
-	{ .str = "EXPORT_SYMBOL_GPL_FUTURE", .export = export_gpl_future },
 	{ .str = "(unknown)",                .export = export_unknown },
 };
 
@@ -369,8 +368,6 @@ static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
 		return export_gpl;
 	else if (strstarts(secname, "___ksymtab_unused_gpl+"))
 		return export_unused_gpl;
-	else if (strstarts(secname, "___ksymtab_gpl_future+"))
-		return export_gpl_future;
 	else
 		return export_unknown;
 }
@@ -385,8 +382,6 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
 		return export_gpl;
 	else if (sec == elf->export_unused_gpl_sec)
 		return export_unused_gpl;
-	else if (sec == elf->export_gpl_future_sec)
-		return export_gpl_future;
 	else
 		return export_unknown;
 }
@@ -596,8 +591,6 @@ static int parse_elf(struct elf_info *info, const char *filename)
 			info->export_gpl_sec = i;
 		else if (strcmp(secname, "__ksymtab_unused_gpl") == 0)
 			info->export_unused_gpl_sec = i;
-		else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
-			info->export_gpl_future_sec = i;
 
 		if (sechdrs[i].sh_type == SHT_SYMTAB) {
 			unsigned int sh_link_idx;
@@ -2152,10 +2145,6 @@ static void check_for_gpl_usage(enum export exp, const char *m, const char *s)
 		error("GPL-incompatible module %s.ko uses GPL-only symbol marked UNUSED '%s'\n",
 		      m, s);
 		break;
-	case export_gpl_future:
-		warn("GPL-incompatible module %s.ko uses future GPL-only symbol '%s'\n",
-		     m, s);
-		break;
 	case export_plain:
 	case export_unused:
 	case export_unknown:
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index e6f46eee0af02f..834220de002bd1 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -142,7 +142,6 @@ struct elf_info {
 	Elf_Section  export_unused_sec;
 	Elf_Section  export_gpl_sec;
 	Elf_Section  export_unused_gpl_sec;
-	Elf_Section  export_gpl_future_sec;
 	char         *strtab;
 	char	     *modinfo;
 	unsigned int modinfo_len;
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 69b9b71a6a4731..d82b452e8a7168 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -13,12 +13,10 @@ SECTIONS {
 	__ksymtab_gpl		0 : { *(SORT(___ksymtab_gpl+*)) }
 	__ksymtab_unused	0 : { *(SORT(___ksymtab_unused+*)) }
 	__ksymtab_unused_gpl	0 : { *(SORT(___ksymtab_unused_gpl+*)) }
-	__ksymtab_gpl_future	0 : { *(SORT(___ksymtab_gpl_future+*)) }
 	__kcrctab		0 : { *(SORT(___kcrctab+*)) }
 	__kcrctab_gpl		0 : { *(SORT(___kcrctab_gpl+*)) }
 	__kcrctab_unused	0 : { *(SORT(___kcrctab_unused+*)) }
 	__kcrctab_unused_gpl	0 : { *(SORT(___kcrctab_unused_gpl+*)) }
-	__kcrctab_gpl_future	0 : { *(SORT(___kcrctab_gpl_future+*)) }
 
 	.init_array		0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
 
diff --git a/tools/include/linux/export.h b/tools/include/linux/export.h
index d07e586b9ba0ec..9f61349a8944e1 100644
--- a/tools/include/linux/export.h
+++ b/tools/include/linux/export.h
@@ -3,7 +3,6 @@
 
 #define EXPORT_SYMBOL(sym)
 #define EXPORT_SYMBOL_GPL(sym)
-#define EXPORT_SYMBOL_GPL_FUTURE(sym)
 #define EXPORT_UNUSED_SYMBOL(sym)
 #define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL*
  2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
                   ` (11 preceding siblings ...)
  2021-01-21  7:49 ` [PATCH 12/13] module: remove EXPORT_SYMBOL_GPL_FUTURE Christoph Hellwig
@ 2021-01-21  7:49 ` Christoph Hellwig
  2021-01-27 13:49   ` Jessica Yu
  12 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  7:49 UTC (permalink / raw)
  To: 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

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;
 };
 
 /* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
diff --git a/init/Kconfig b/init/Kconfig
index b77c60f8b963d4..11b803b45c1995 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2262,25 +2262,8 @@ config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
 
 	  If unsure, say N.
 
-config UNUSED_SYMBOLS
-	bool "Enable unused/obsolete exported symbols"
-	default y if X86
-	help
-	  Unused but exported symbols make the kernel needlessly bigger.  For
-	  that reason most of these unused exports will soon be removed.  This
-	  option is provided temporarily to provide a transition period in case
-	  some external kernel module needs one of these symbols anyway. If you
-	  encounter such a case in your module, consider if you are actually
-	  using the right API.  (rationale: since nobody in the kernel is using
-	  this in a module, there is a pretty good chance it's actually the
-	  wrong interface to use).  If you really need the symbol, please send a
-	  mail to the linux kernel mailing list mentioning the symbol and why
-	  you really need it, and what the merge plan to the mainline kernel for
-	  your module is.
-
 config TRIM_UNUSED_KSYMS
 	bool "Trim unused exported kernel symbols"
-	depends on !UNUSED_SYMBOLS
 	help
 	  The kernel and some modules make many symbols available for
 	  other modules to use via EXPORT_SYMBOL() and variants. Depending
diff --git a/kernel/module.c b/kernel/module.c
index 917fd1b5c95a42..f725ff99b64f54 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -415,14 +415,6 @@ extern const struct kernel_symbol __start___ksymtab_gpl[];
 extern const struct kernel_symbol __stop___ksymtab_gpl[];
 extern const s32 __start___kcrctab[];
 extern const s32 __start___kcrctab_gpl[];
-#ifdef CONFIG_UNUSED_SYMBOLS
-extern const struct kernel_symbol __start___ksymtab_unused[];
-extern const struct kernel_symbol __stop___ksymtab_unused[];
-extern const struct kernel_symbol __start___ksymtab_unused_gpl[];
-extern const struct kernel_symbol __stop___ksymtab_unused_gpl[];
-extern const s32 __start___kcrctab_unused[];
-extern const s32 __start___kcrctab_unused_gpl[];
-#endif
 
 #ifndef CONFIG_MODVERSIONS
 #define symversion(base, idx) NULL
@@ -459,18 +451,6 @@ static bool check_exported_symbol(const struct symsearch *syms,
 		}
 	}
 
-#ifdef CONFIG_UNUSED_SYMBOLS
-	if (syms->unused && fsa->warn) {
-		pr_warn("Symbol %s is marked as UNUSED, however this module is "
-			"using it.\n", fsa->name);
-		pr_warn("This symbol will go away in the future.\n");
-		pr_warn("Please evaluate if this is the right api to use and "
-			"if it really is, submit a report to the linux kernel "
-			"mailing list together with submitting your code for "
-			"inclusion.\n");
-	}
-#endif
-
 	fsa->owner = owner;
 	fsa->crc = symversion(syms->crcs, symnum);
 	fsa->sym = &syms->start[symnum];
@@ -537,18 +517,10 @@ static bool find_symbol(struct find_symbol_arg *fsa)
 {
 	static const struct symsearch arr[] = {
 		{ __start___ksymtab, __stop___ksymtab, __start___kcrctab,
-		  NOT_GPL_ONLY, false },
+		  NOT_GPL_ONLY },
 		{ __start___ksymtab_gpl, __stop___ksymtab_gpl,
 		  __start___kcrctab_gpl,
-		  GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
-		{ __start___ksymtab_unused, __stop___ksymtab_unused,
-		  __start___kcrctab_unused,
-		  NOT_GPL_ONLY, true },
-		{ __start___ksymtab_unused_gpl, __stop___ksymtab_unused_gpl,
-		  __start___kcrctab_unused_gpl,
-		  GPL_ONLY, true },
-#endif
+		  GPL_ONLY },
 	};
 	struct module *mod;
 	unsigned int i;
@@ -563,20 +535,10 @@ static bool find_symbol(struct find_symbol_arg *fsa)
 				lockdep_is_held(&module_mutex)) {
 		struct symsearch arr[] = {
 			{ mod->syms, mod->syms + mod->num_syms, mod->crcs,
-			  NOT_GPL_ONLY, false },
+			  NOT_GPL_ONLY },
 			{ mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
 			  mod->gpl_crcs,
-			  GPL_ONLY, false },
-#ifdef CONFIG_UNUSED_SYMBOLS
-			{ mod->unused_syms,
-			  mod->unused_syms + mod->num_unused_syms,
-			  mod->unused_crcs,
-			  NOT_GPL_ONLY, true },
-			{ mod->unused_gpl_syms,
-			  mod->unused_gpl_syms + mod->num_unused_gpl_syms,
-			  mod->unused_gpl_crcs,
-			  GPL_ONLY, true },
-#endif
+			  GPL_ONLY },
 		};
 
 		if (mod->state == MODULE_STATE_UNFORMED)
@@ -2304,10 +2266,6 @@ static int verify_exported_symbols(struct module *mod)
 	} arr[] = {
 		{ mod->syms, mod->num_syms },
 		{ mod->gpl_syms, mod->num_gpl_syms },
-#ifdef CONFIG_UNUSED_SYMBOLS
-		{ mod->unused_syms, mod->num_unused_syms },
-		{ mod->unused_gpl_syms, mod->num_unused_gpl_syms },
-#endif
 	};
 
 	for (i = 0; i < ARRAY_SIZE(arr); i++) {
@@ -3222,16 +3180,6 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 				     &mod->num_gpl_syms);
 	mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
 
-#ifdef CONFIG_UNUSED_SYMBOLS
-	mod->unused_syms = section_objs(info, "__ksymtab_unused",
-					sizeof(*mod->unused_syms),
-					&mod->num_unused_syms);
-	mod->unused_crcs = section_addr(info, "__kcrctab_unused");
-	mod->unused_gpl_syms = section_objs(info, "__ksymtab_unused_gpl",
-					    sizeof(*mod->unused_gpl_syms),
-					    &mod->num_unused_gpl_syms);
-	mod->unused_gpl_crcs = section_addr(info, "__kcrctab_unused_gpl");
-#endif
 #ifdef CONFIG_CONSTRUCTORS
 	mod->ctors = section_objs(info, ".ctors",
 				  sizeof(*mod->ctors), &mod->num_ctors);
@@ -3412,13 +3360,8 @@ static int check_module_license_and_versions(struct module *mod)
 		pr_warn("%s: module license taints kernel.\n", mod->name);
 
 #ifdef CONFIG_MODVERSIONS
-	if ((mod->num_syms && !mod->crcs)
-	    || (mod->num_gpl_syms && !mod->gpl_crcs)
-#ifdef CONFIG_UNUSED_SYMBOLS
-	    || (mod->num_unused_syms && !mod->unused_crcs)
-	    || (mod->num_unused_gpl_syms && !mod->unused_gpl_crcs)
-#endif
-		) {
+	if ((mod->num_syms && !mod->crcs) ||
+	    (mod->num_gpl_syms && !mod->gpl_crcs)) {
 		return try_to_force_load(mod,
 					 "no versions for exported symbols");
 	}
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 92e888ed939f98..eabd2d5467b156 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4290,8 +4290,7 @@ sub process {
 		if (defined $realline_next &&
 		    exists $lines[$realline_next - 1] &&
 		    !defined $suppress_export{$realline_next} &&
-		    ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
-		     $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
+		    ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/)) {
 			# Handle definitions which produce identifiers with
 			# a prefix:
 			#   XXX(foo);
@@ -4318,8 +4317,7 @@ sub process {
 		}
 		if (!defined $suppress_export{$linenr} &&
 		    $prevline =~ /^.\s*$/ &&
-		    ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
-		     $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
+		    ($line =~ /EXPORT_SYMBOL.*\((.*)\)/)) {
 #print "FOO B <$lines[$linenr - 1]>\n";
 			$suppress_export{$linenr} = 2;
 		}
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 25c1446055d16b..20fc57837881ab 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -43,8 +43,9 @@ static int allow_missing_ns_imports;
 static bool error_occurred;
 
 enum export {
-	export_plain,      export_unused,     export_gpl,
-	export_unused_gpl, export_unknown
+	export_plain,
+	export_gpl,
+	export_unknown
 };
 
 /* In kernel, this size is defined in linux/module.h;
@@ -301,9 +302,7 @@ static const struct {
 	enum export export;
 } export_list[] = {
 	{ .str = "EXPORT_SYMBOL",            .export = export_plain },
-	{ .str = "EXPORT_UNUSED_SYMBOL",     .export = export_unused },
 	{ .str = "EXPORT_SYMBOL_GPL",        .export = export_gpl },
-	{ .str = "EXPORT_UNUSED_SYMBOL_GPL", .export = export_unused_gpl },
 	{ .str = "(unknown)",                .export = export_unknown },
 };
 
@@ -362,12 +361,8 @@ static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
 
 	if (strstarts(secname, "___ksymtab+"))
 		return export_plain;
-	else if (strstarts(secname, "___ksymtab_unused+"))
-		return export_unused;
 	else if (strstarts(secname, "___ksymtab_gpl+"))
 		return export_gpl;
-	else if (strstarts(secname, "___ksymtab_unused_gpl+"))
-		return export_unused_gpl;
 	else
 		return export_unknown;
 }
@@ -376,12 +371,8 @@ static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
 {
 	if (sec == elf->export_sec)
 		return export_plain;
-	else if (sec == elf->export_unused_sec)
-		return export_unused;
 	else if (sec == elf->export_gpl_sec)
 		return export_gpl;
-	else if (sec == elf->export_unused_gpl_sec)
-		return export_unused_gpl;
 	else
 		return export_unknown;
 }
@@ -585,12 +576,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
 			info->modinfo_len = sechdrs[i].sh_size;
 		} else if (strcmp(secname, "__ksymtab") == 0)
 			info->export_sec = i;
-		else if (strcmp(secname, "__ksymtab_unused") == 0)
-			info->export_unused_sec = i;
 		else if (strcmp(secname, "__ksymtab_gpl") == 0)
 			info->export_gpl_sec = i;
-		else if (strcmp(secname, "__ksymtab_unused_gpl") == 0)
-			info->export_unused_gpl_sec = i;
 
 		if (sechdrs[i].sh_type == SHT_SYMTAB) {
 			unsigned int sh_link_idx;
@@ -2141,32 +2128,13 @@ static void check_for_gpl_usage(enum export exp, const char *m, const char *s)
 		error("GPL-incompatible module %s.ko uses GPL-only symbol '%s'\n",
 		      m, s);
 		break;
-	case export_unused_gpl:
-		error("GPL-incompatible module %s.ko uses GPL-only symbol marked UNUSED '%s'\n",
-		      m, s);
-		break;
 	case export_plain:
-	case export_unused:
 	case export_unknown:
 		/* ignore */
 		break;
 	}
 }
 
-static void check_for_unused(enum export exp, const char *m, const char *s)
-{
-	switch (exp) {
-	case export_unused:
-	case export_unused_gpl:
-		warn("module %s.ko uses symbol '%s' marked UNUSED\n",
-		     m, s);
-		break;
-	default:
-		/* ignore */
-		break;
-	}
-}
-
 static void check_exports(struct module *mod)
 {
 	struct symbol *s, *exp;
@@ -2197,7 +2165,6 @@ static void check_exports(struct module *mod)
 
 		if (!mod->gpl_compatible)
 			check_for_gpl_usage(exp->export, basename, exp->name);
-		check_for_unused(exp->export, basename, exp->name);
 	}
 }
 
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 834220de002bd1..0c47ff95c0e227 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -139,9 +139,7 @@ struct elf_info {
 	Elf_Sym      *symtab_start;
 	Elf_Sym      *symtab_stop;
 	Elf_Section  export_sec;
-	Elf_Section  export_unused_sec;
 	Elf_Section  export_gpl_sec;
-	Elf_Section  export_unused_gpl_sec;
 	char         *strtab;
 	char	     *modinfo;
 	unsigned int modinfo_len;
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index d82b452e8a7168..24e8af579ce378 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -11,12 +11,8 @@ SECTIONS {
 
 	__ksymtab		0 : { *(SORT(___ksymtab+*)) }
 	__ksymtab_gpl		0 : { *(SORT(___ksymtab_gpl+*)) }
-	__ksymtab_unused	0 : { *(SORT(___ksymtab_unused+*)) }
-	__ksymtab_unused_gpl	0 : { *(SORT(___ksymtab_unused_gpl+*)) }
 	__kcrctab		0 : { *(SORT(___kcrctab+*)) }
 	__kcrctab_gpl		0 : { *(SORT(___kcrctab_gpl+*)) }
-	__kcrctab_unused	0 : { *(SORT(___kcrctab_unused+*)) }
-	__kcrctab_unused_gpl	0 : { *(SORT(___kcrctab_unused_gpl+*)) }
 
 	.init_array		0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
 
diff --git a/tools/include/linux/export.h b/tools/include/linux/export.h
index 9f61349a8944e1..acb6f4daa2f0b4 100644
--- a/tools/include/linux/export.h
+++ b/tools/include/linux/export.h
@@ -3,7 +3,5 @@
 
 #define EXPORT_SYMBOL(sym)
 #define EXPORT_SYMBOL_GPL(sym)
-#define EXPORT_UNUSED_SYMBOL(sym)
-#define EXPORT_UNUSED_SYMBOL_GPL(sym)
 
 #endif
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
  2021-01-21  7:49 ` [PATCH 08/13] drm: remove drm_fb_helper_modinit Christoph Hellwig
@ 2021-01-21  8:25   ` Daniel Vetter
  2021-01-21  8:28     ` Christoph Hellwig
  0 siblings, 1 reply; 29+ messages in thread
From: Daniel Vetter @ 2021-01-21  8:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Jessica Yu,
	Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Masahiro Yamada, Michal Marek,
	Linux Kernel Mailing List, linuxppc-dev, dri-devel,
	live-patching, linux-kbuild

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
  2021-01-21  8:25   ` Daniel Vetter
@ 2021-01-21  8:28     ` Christoph Hellwig
  2021-01-21  8:39       ` Daniel Vetter
  0 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21  8:28 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Christoph Hellwig, Frederic Barrat, Andrew Donnellan,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Jessica Yu, Josh Poimboeuf, Jiri Kosina,
	Miroslav Benes, Petr Mladek, Joe Lawrence, Masahiro Yamada,
	Michal Marek, Linux Kernel Mailing List, linuxppc-dev, dri-devel,
	live-patching, linux-kbuild

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.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
  2021-01-21  8:28     ` Christoph Hellwig
@ 2021-01-21  8:39       ` Daniel Vetter
  0 siblings, 0 replies; 29+ messages in thread
From: Daniel Vetter @ 2021-01-21  8:39 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Jessica Yu,
	Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Masahiro Yamada, Michal Marek,
	Linux Kernel Mailing List, linuxppc-dev, dri-devel,
	live-patching, linux-kbuild

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 01/13] powerpc/powernv: remove get_cxl_module
  2021-01-21  7:49 ` [PATCH 01/13] powerpc/powernv: remove get_cxl_module Christoph Hellwig
@ 2021-01-21  9:09   ` Andrew Donnellan
  0 siblings, 0 replies; 29+ 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: Masahiro Yamada, Michal Marek, linux-kernel, linuxppc-dev,
	dri-devel, live-patching, linux-kbuild

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 02/13] module: add a module_loaded helper
  2021-01-21  7:49 ` [PATCH 02/13] module: add a module_loaded helper Christoph Hellwig
@ 2021-01-21 10:00   ` Christophe Leroy
  2021-01-21 17:11     ` Christoph Hellwig
  0 siblings, 1 reply; 29+ 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.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 02/13] module: add a module_loaded helper
  2021-01-21 10:00   ` Christophe Leroy
@ 2021-01-21 17:11     ` Christoph Hellwig
  2021-01-21 17:44       ` David Laight
  0 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-21 17:11 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: 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,
	Michal Marek, linux-kbuild, Masahiro Yamada, linux-kernel,
	dri-devel, live-patching, 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.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* RE: [PATCH 02/13] module: add a module_loaded helper
  2021-01-21 17:11     ` Christoph Hellwig
@ 2021-01-21 17:44       ` David Laight
  0 siblings, 0 replies; 29+ messages in thread
From: David Laight @ 2021-01-21 17:44 UTC (permalink / raw)
  To: 'Christoph Hellwig', Christophe Leroy
  Cc: Petr Mladek, Jiri Kosina, Andrew Donnellan, linux-kbuild,
	David Airlie, Masahiro Yamada, Josh Poimboeuf, Maarten Lankhorst,
	linux-kernel, Maxime Ripard, live-patching, Michal Marek,
	Joe Lawrence, dri-devel, Thomas Zimmermann, Jessica Yu,
	Frederic Barrat, Daniel Vetter, 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)


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c
  2021-01-21  7:49 ` [PATCH 04/13] livepatch: move klp_find_object_module to module.c Christoph Hellwig
@ 2021-01-21 21:45   ` Josh Poimboeuf
  2021-01-26 14:25   ` Jessica Yu
  1 sibling, 0 replies; 29+ messages in thread
From: Josh Poimboeuf @ 2021-01-21 21:45 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Jessica Yu, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Masahiro Yamada, Michal Marek, linux-kernel,
	linuxppc-dev, dri-devel, live-patching, linux-kbuild

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c
  2021-01-21  7:49 ` [PATCH 04/13] livepatch: move klp_find_object_module to module.c Christoph Hellwig
  2021-01-21 21:45   ` Josh Poimboeuf
@ 2021-01-26 14:25   ` Jessica Yu
  2021-01-27 11:55     ` Petr Mladek
  1 sibling, 1 reply; 29+ messages in thread
From: Jessica Yu @ 2021-01-26 14:25 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Masahiro Yamada, Michal Marek, linux-kernel,
	linuxppc-dev, dri-devel, live-patching, linux-kbuild

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

^ permalink raw reply	[flat|nested] 29+ 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; 29+ messages in thread
From: Petr Mladek @ 2021-01-27 11:55 UTC (permalink / raw)
  To: Jessica Yu
  Cc: Christoph Hellwig, Frederic Barrat, Andrew Donnellan,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Josh Poimboeuf, Jiri Kosina,
	Miroslav Benes, Joe Lawrence, Masahiro Yamada, Michal Marek,
	linux-kernel, linuxppc-dev, dri-devel, live-patching,
	linux-kbuild

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/13] livepatch: refactor klp_init_object
  2021-01-21  7:49 ` [PATCH 03/13] livepatch: refactor klp_init_object Christoph Hellwig
@ 2021-01-27 12:58   ` Petr Mladek
  2021-01-28 16:22     ` Christoph Hellwig
  0 siblings, 1 reply; 29+ messages in thread
From: Petr Mladek @ 2021-01-27 12:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Jessica Yu, Josh Poimboeuf, Jiri Kosina, Miroslav Benes,
	Joe Lawrence, Masahiro Yamada, Michal Marek, linux-kernel,
	linuxppc-dev, dri-devel, live-patching, linux-kbuild

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

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL*
  2021-01-21  7:49 ` [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL* Christoph Hellwig
@ 2021-01-27 13:49   ` Jessica Yu
  2021-01-28 16:09     ` Christoph Hellwig
  0 siblings, 1 reply; 29+ messages in thread
From: Jessica Yu @ 2021-01-27 13:49 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Frederic Barrat, Andrew Donnellan, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
	Joe Lawrence, Masahiro Yamada, Michal Marek, linux-kernel,
	linuxppc-dev, dri-devel, live-patching, linux-kbuild

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


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL*
  2021-01-27 13:49   ` Jessica Yu
@ 2021-01-28 16:09     ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-28 16:09 UTC (permalink / raw)
  To: Jessica Yu
  Cc: Christoph Hellwig, Frederic Barrat, Andrew Donnellan,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Josh Poimboeuf, Jiri Kosina,
	Miroslav Benes, Petr Mladek, Joe Lawrence, Masahiro Yamada,
	Michal Marek, linux-kernel, linuxppc-dev, dri-devel,
	live-patching, linux-kbuild

On Wed, Jan 27, 2021 at 02:49:38PM +0100, Jessica Yu wrote:
>> #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.

I've added a patch to just move it directly into module.c.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/13] livepatch: refactor klp_init_object
  2021-01-27 12:58   ` Petr Mladek
@ 2021-01-28 16:22     ` Christoph Hellwig
  2021-01-28 16:24       ` Christoph Hellwig
  0 siblings, 1 reply; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-28 16:22 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Christoph Hellwig, Frederic Barrat, Andrew Donnellan,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jessica Yu, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Joe Lawrence, Masahiro Yamada,
	Michal Marek, linux-kernel, linuxppc-dev, dri-devel,
	live-patching, linux-kbuild

On Wed, Jan 27, 2021 at 01:58:21PM +0100, Petr Mladek wrote:
> > --- 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.

Well, the check is in the caller now where we have a conditional for
it.  So I'd be tempted to either update the comment, or just drop the
patch.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH 03/13] livepatch: refactor klp_init_object
  2021-01-28 16:22     ` Christoph Hellwig
@ 2021-01-28 16:24       ` Christoph Hellwig
  0 siblings, 0 replies; 29+ messages in thread
From: Christoph Hellwig @ 2021-01-28 16:24 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Christoph Hellwig, Frederic Barrat, Andrew Donnellan,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Jessica Yu, Josh Poimboeuf,
	Jiri Kosina, Miroslav Benes, Joe Lawrence, Masahiro Yamada,
	Michal Marek, linux-kernel, linuxppc-dev, dri-devel,
	live-patching, linux-kbuild

On Thu, Jan 28, 2021 at 05:22:40PM +0100, Christoph Hellwig wrote:
> > 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.
> 
> Well, the check is in the caller now where we have a conditional for
> it.  So I'd be tempted to either update the comment, or just drop the
> patch.

Also even without the check I think it will do the right thing when
called for vmlinux given that it simplify won't find a module called
vmlinux..

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2021-01-28 16:25 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21  7:49 module loader dead code removal and cleanusp Christoph Hellwig
2021-01-21  7:49 ` [PATCH 01/13] powerpc/powernv: remove get_cxl_module Christoph Hellwig
2021-01-21  9:09   ` Andrew Donnellan
2021-01-21  7:49 ` [PATCH 02/13] module: add a module_loaded helper Christoph Hellwig
2021-01-21 10:00   ` Christophe Leroy
2021-01-21 17:11     ` Christoph Hellwig
2021-01-21 17:44       ` David Laight
2021-01-21  7:49 ` [PATCH 03/13] livepatch: refactor klp_init_object Christoph Hellwig
2021-01-27 12:58   ` Petr Mladek
2021-01-28 16:22     ` Christoph Hellwig
2021-01-28 16:24       ` Christoph Hellwig
2021-01-21  7:49 ` [PATCH 04/13] livepatch: move klp_find_object_module to module.c Christoph Hellwig
2021-01-21 21:45   ` Josh Poimboeuf
2021-01-26 14:25   ` Jessica Yu
2021-01-27 11:55     ` Petr Mladek
2021-01-21  7:49 ` [PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol Christoph Hellwig
2021-01-21  7:49 ` [PATCH 06/13] kallsyms: only build {,module_}kallsyms_on_each_symbol when required Christoph Hellwig
2021-01-21  7:49 ` [PATCH 07/13] module: mark module_mutex static Christoph Hellwig
2021-01-21  7:49 ` [PATCH 08/13] drm: remove drm_fb_helper_modinit Christoph Hellwig
2021-01-21  8:25   ` Daniel Vetter
2021-01-21  8:28     ` Christoph Hellwig
2021-01-21  8:39       ` Daniel Vetter
2021-01-21  7:49 ` [PATCH 09/13] module: remove each_symbol_in_section Christoph Hellwig
2021-01-21  7:49 ` [PATCH 10/13] module: merge each_symbol_section into find_symbol Christoph Hellwig
2021-01-21  7:49 ` [PATCH 11/13] module: pass struct find_symbol_args to find_symbol Christoph Hellwig
2021-01-21  7:49 ` [PATCH 12/13] module: remove EXPORT_SYMBOL_GPL_FUTURE Christoph Hellwig
2021-01-21  7:49 ` [PATCH 13/13] module: remove EXPORY_UNUSED_SYMBOL* Christoph Hellwig
2021-01-27 13:49   ` Jessica Yu
2021-01-28 16:09     ` Christoph Hellwig

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