Message-ID: <gcykzencr7rmeiy3rmclxrbbvuryo2uyb6plqqovee3bsme42b@g6pwzbgitgka>
Date: Thu, 25 Jul 2024 09:29:51 -0500
From: Lucas De Marchi <lucas.demarchi@...el.com>
To: Dragan Simic <dsimic@...jaro.org>
CC: <linux-modules@...r.kernel.org>, <mcgrof@...nel.org>,
	<linux-kernel@...r.kernel.org>, <didi.debian@...ow.org>, Steven Price
	<steven.price@....com>, Boris Brezillon <boris.brezillon@...labora.com>,
	Qiang Yu <yuq825@...il.com>
Subject: Re: [PATCH] module: Add hard dependencies as syntactic sugar

On Thu, Jul 25, 2024 at 01:37:46PM GMT, Dragan Simic wrote:
>Panfrost and Lima DRM drivers use devfreq to perform DVFS, which is supported
>on the associated platforms, while using simple_ondemand devfreq governor by
>default.  This makes the simple_ondemand module a hard dependency for both
>Panfrost and Lima, because the presence of the simple_ondemand module in an
>initial ramdisk allows the initialization of Panfrost or Lima to succeed.
>This is currently expressed using MODULE_SOFTDEP. [1][2]  Please see commits
>80f4e62730a9 ("drm/panfrost: Mark simple_ondemand governor as softdep") and
>0c94f58cef31 ("drm/lima: Mark simple_ondemand governor as softdep") for
>additional background information.
>
>With the addition of MODULE_WEAKDEP in commit 61842868de13 ("module: create
>weak dependecies"), the dependency between Panfrost/Lima and simple_ondemand
>can be expressed in a much better way as a weakdep, because that provides
>the required dependency information to the utilities that generate initial
>ramdisks, but leaves the actual loading of the required kernel module(s) to
>the kernel.  However, being able to actually express this as a hard module
>dependency would still be beneficial.
>
>With all this in mind, let's add MODULE_HARDDEP as some kind of syntactic

Sorry, but NACK from me. This only adds to the confusion.

hard/normal dependency:
	It's a symbol dependency. If you want it in your module, you
	have to use a symbol. Example:

	$ modinfo ksmbd | grep depends
	depends:        ib_core,rdma_cm,nls_ucs2_utils,cifs_arc4


soft dependency:
	A dependency you declare in configuration or in the module
	info added by the kernel. A "pre" softdep means libkmod/modprobe
	will try to load that dep before the actual module. Example:

	$ modinfo ksmbd | grep softdep
	softdep:        pre: crc32
	softdep:        pre: gcm
	softdep:        pre: ccm
	softdep:        pre: aead2
	softdep:        pre: sha512
	softdep:        pre: sha256
	softdep:        pre: cmac
	softdep:        pre: aes
	softdep:        pre: nls
	softdep:        pre: md5
	softdep:        pre: hmac
	softdep:        pre: ecb

weak dependency:
	A dependency you declare in configuration or in the module
	info added by the kernel. libkmod/modprobe will not change the
	way it loads the module and it will only used by tools that need
	to make sure the module is there when the kernel does a
	request_module() or somehow tries to load that module.

So if you want a hard dependency, just use a symbol from the module. If
you want to emulate a hard dependency without calling a symbol, you use
a pre softdep, not a weakdep.  You use a weakdep if the kernel itself,
somehow may load module in runtime.

The problem described in 80f4e62730a9 ("drm/panfrost: Mark simple_ondemand governor as softdep")
could indeed be solved with a weakdep, so I'm not sure why you'd want to
alias it as a "hard dep".

Lucas De Marchi

>sugar, currently implemented as an alias for MODULE_WEAKDEP, so the actual
>hard module dependencies can be expressed properly, and possibly handled
>differently in the future, avoiding the need to go back, track and churn
>all such instances of hard module dependencies.  The first consumers of
>MODULE_HARDDEP will be the Panfrost and Lima DRM drivers, but the list of
>consumers may also grow a bit in the future.
>
>For example, allowing reduction of the initial ramdisk size is a possible
>future difference between handling the MODULE_WEAKDEP and MODULE_HARDDEP
>dependencies.  When the size of the initial ramdisk is limited, the utilities
>that generate initial ramdisks can use the distinction between the weakdeps
>and the harddeps to safely omit some of the weakdep modules from the created
>initial ramdisks, and to keep all harddep modules.
>
>Due to the nature of MODULE_WEAKDEP, the above-described example will also
>require some additional device-specific information to be made available to
>the utilities that create initial ramdisks, so they can actually know which
>weakdep modules can be safely pruned for a particular device, but the
>distinction between the harddeps and the weakdeps opens up a path towards
>using such additional "pruning information" in a more robust way, by ensuring
>that the absolutely required harddep modules aren't pruned away.
>
>[1] https://lore.kernel.org/dri-devel/4e1e00422a14db4e2a80870afb704405da16fd1b.1718655077.git.dsimic@manjaro.org/T/#u
>[2] https://lore.kernel.org/dri-devel/fdaf2e41bb6a0c5118ff9cc21f4f62583208d885.1718655070.git.dsimic@manjaro.org/T/#u
>
>Cc: Steven Price <steven.price@....com>
>Cc: Boris Brezillon <boris.brezillon@...labora.com>
>Cc: Qiang Yu <yuq825@...il.com>
>Signed-off-by: Dragan Simic <dsimic@...jaro.org>
>---
> include/linux/module.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/include/linux/module.h b/include/linux/module.h
>index 88ecc5e9f523..40e5762847a9 100644
>--- a/include/linux/module.h
>+++ b/include/linux/module.h
>@@ -179,6 +179,14 @@ extern void cleanup_module(void);
>  */
> #define MODULE_WEAKDEP(_weakdep) MODULE_INFO(weakdep, _weakdep)
>
>+/*
>+ * Hard module dependencies. Currently handled the same as weak
>+ * module dependencies, but intended to mark hard dependencies
>+ * as such for possible different handling in the future.
>+ * Example: MODULE_HARDDEP("module-foo")
>+ */
>+#define MODULE_HARDDEP(_harddep) MODULE_WEAKDEP(_harddep)
>+
> /*
>  * MODULE_FILE is used for generating modules.builtin
>  * So, make it no-op when this is being built as a module

Powered by blists - more mailing lists