RFC: Handle hard module dependencies that are not symbol-based (r8169 + realtek)

[Heiner Kallweit]

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.

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