lkml.org 
[lkml]   [2020]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: RFC: Handle hard module dependencies that are not symbol-based (r8169 + realtek)
On Wed, Apr 01, 2020 at 11:20:20PM +0200, Heiner Kallweit wrote:
>Currently we have no way to express a hard dependency that is not
>a symbol-based dependency (symbol defined in module A is used in
>module B). Use case:
>Network driver ND uses callbacks in the dedicated PHY driver DP
>for the integrated PHY (namely read_page() and write_page() in
>struct phy_driver). If DP can't be loaded (e.g. because ND is in
>initramfs but DP is not), then phylib will use the generic
>PHY driver GP. GP doesn't implement certain callbacks that are
>needed by ND, therefore ND's probe has to bail out with an error
>once it detects that DP is not loaded.
>We have this problem with driver r8169 having such a dependency
>on PHY driver realtek. Some distributions have tools for
>configuring initramfs that consider hard dependencies based on
>depmod output. Means so far somebody can add r8169.ko to initramfs,
>and neither human being nor machine will have an idea that
>realtek.ko needs to be added too.

Could you expand on why softdep doesn't solve this problem
with MODULE_SOFTDEP()

initramfs tools can already read it and modules can already expose them
(they end up in /lib/modules/$(uname -r)/modules.softdep and modprobe
makes use of them)

Lucas De Marchi

>
>Attached patch set (two patches for kmod, one for the kernel)
>allows to express this hard dependency of ND from DP. depmod will
>read this dependency information and treat it like a symbol-based
>dependency. As a result tools e.g. populating initramfs can
>consider the dependency and place DP in initramfs if ND is in
>initramfs. On my system the patch set does the trick when
>adding following line to r8169_main.c:
>MODULE_HARDDEP("realtek");
>
>I'm interested in your opinion on the patches, and whether you
>maybe have a better idea how to solve the problem.
>
>Heiner

>From 290e7dee9f6043d677f08dc06e612e13ee0d2d83 Mon Sep 17 00:00:00 2001
>From: Heiner Kallweit <hkallweit1@gmail.com>
>Date: Tue, 31 Mar 2020 23:02:47 +0200
>Subject: [PATCH 1/2] depmod: add helper mod_add_dep_unique
>
>Create new helper mod_add_dep_unique(), next patch in this series will
>also make use of it.
>
>Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>---
> tools/depmod.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
>diff --git a/tools/depmod.c b/tools/depmod.c
>index 875e314..5419d4d 100644
>--- a/tools/depmod.c
>+++ b/tools/depmod.c
>@@ -907,23 +907,35 @@ static void mod_free(struct mod *mod)
> free(mod);
> }
>
>-static int mod_add_dependency(struct mod *mod, struct symbol *sym)
>+static int mod_add_dep_unique(struct mod *mod, struct mod *dep)
> {
> int err;
>
>- DBG("%s depends on %s %s\n", mod->path, sym->name,
>- sym->owner != NULL ? sym->owner->path : "(unknown)");
>-
>- if (sym->owner == NULL)
>+ if (dep == NULL)
> return 0;
>
>- err = array_append_unique(&mod->deps, sym->owner);
>+ err = array_append_unique(&mod->deps, dep);
> if (err == -EEXIST)
> return 0;
> if (err < 0)
> return err;
>
>- sym->owner->users++;
>+ dep->users++;
>+
>+ return 1;
>+}
>+
>+static int mod_add_dependency(struct mod *mod, struct symbol *sym)
>+{
>+ int err;
>+
>+ DBG("%s depends on %s %s\n", mod->path, sym->name,
>+ sym->owner != NULL ? sym->owner->path : "(unknown)");
>+
>+ err = mod_add_dep_unique(mod, sym->owner);
>+ if (err <= 0)
>+ return err;
>+
> SHOW("%s needs \"%s\": %s\n", mod->path, sym->name, sym->owner->path);
> return 0;
> }
>--
>2.26.0
>

>From b12fa0d85b21d84cdf4509c5048c67e17914eb28 Mon Sep 17 00:00:00 2001
>From: Heiner Kallweit <hkallweit1@gmail.com>
>Date: Mon, 30 Mar 2020 17:12:44 +0200
>Subject: [PATCH] module: add MODULE_HARDDEP
>
>Currently we have no way to express a hard dependency that is not a
>symbol-based dependency (symbol defined in module A is used in
>module B). Use case:
>Network driver ND uses callbacks in the dedicated PHY driver DP
>for the integrated PHY. If DP can't be loaded (e.g. because ND
>is in initramfs but DP is not), then phylib will load the generic
>PHY driver GP. GP doesn't implement certain callbacks that are
>used by ND, therefore ND's probe has to bail out with an error
>once it detects that DP is not loaded.
>This patch allows to express this hard dependency of ND from DP.
>depmod will read this dependency information and treat it like
>a symbol-based dependency. As a result tools e.g. populating
>initramfs can consider the dependency and place DP in initramfs
>if ND is in initramfs.
>
>Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>---
> include/linux/module.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/include/linux/module.h b/include/linux/module.h
>index 1ad393e62..f38d4107f 100644
>--- a/include/linux/module.h
>+++ b/include/linux/module.h
>@@ -169,6 +169,11 @@ extern void cleanup_module(void);
> */
> #define MODULE_SOFTDEP(_softdep) MODULE_INFO(softdep, _softdep)
>
>+/* Hard module dependencies that are not code dependencies
>+ * Example: MODULE_HARDDEP("module-foo module-bar")
>+ */
>+#define MODULE_HARDDEP(_harddep) MODULE_INFO(harddep, _harddep)
>+
> /*
> * MODULE_FILE is used for generating modules.builtin
> * So, make it no-op when this is being built as a module
>--
>2.26.0
>

>From af3a25833a160e029441eaf5a93f7c8625544296 Mon Sep 17 00:00:00 2001
>From: Heiner Kallweit <hkallweit1@gmail.com>
>Date: Wed, 1 Apr 2020 22:42:55 +0200
>Subject: [PATCH 2/2] depmod: add depmod_load_harddeps
>
>Load explicitly declared hard dependency information from modules and
>add it to the symbol-derived dependencies. This will allow
>depmod-based tools to consider hard dependencies that are not code
>dependencies.
>
>Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>---
> tools/depmod.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
>diff --git a/tools/depmod.c b/tools/depmod.c
>index 5419d4d..5771dc9 100644
>--- a/tools/depmod.c
>+++ b/tools/depmod.c
>@@ -1522,6 +1522,41 @@ static struct symbol *depmod_symbol_find(const struct depmod *depmod,
> return hash_find(depmod->symbols, name);
> }
>
>+static void depmod_load_harddeps(struct depmod *depmod, struct mod *mod)
>+{
>+
>+ struct kmod_list *l;
>+
>+ kmod_list_foreach(l, mod->info_list) {
>+ const char *key = kmod_module_info_get_key(l);
>+ const char *dep_name;
>+ struct mod *dep;
>+ char *value;
>+
>+ if (!streq(key, "harddep"))
>+ continue;
>+
>+ value = strdup(kmod_module_info_get_value(l));
>+ if (value == NULL)
>+ return;
>+
>+ dep_name = strtok(value, " \t");
>+
>+ while (dep_name) {
>+ dep = hash_find(depmod->modules_by_name, dep_name);
>+ if (dep)
>+ mod_add_dep_unique(mod, dep);
>+ else
>+ WRN("harddep: %s: unknown dependency %s\n",
>+ mod->modname, dep_name);
>+
>+ dep_name = strtok(NULL, " \t");
>+ }
>+
>+ free(value);
>+ }
>+}
>+
> static int depmod_load_modules(struct depmod *depmod)
> {
> struct mod **itr, **itr_end;
>@@ -1569,6 +1604,9 @@ static int depmod_load_module_dependencies(struct depmod *depmod, struct mod *mo
> struct kmod_list *l;
>
> DBG("do dependencies of %s\n", mod->path);
>+
>+ depmod_load_harddeps(depmod, mod);
>+
> kmod_list_foreach(l, mod->dep_sym_list) {
> const char *name = kmod_module_dependency_symbol_get_symbol(l);
> uint64_t crc = kmod_module_dependency_symbol_get_crc(l);
>--
>2.26.0
>

\
 
 \ /
  Last update: 2020-04-09 02:03    [W:0.068 / U:3.832 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site