Re: [PATCH] module: Add hard dependencies as syntactic sugar

[Steven Price]

On 25/07/2024 12:37, 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
> 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@xxxxxxxxxxx/T/#u
> [2] https://lore.kernel.org/dri-devel/fdaf2e41bb6a0c5118ff9cc21f4f62583208d885.1718655070.git.dsimic@xxxxxxxxxxx/T/#u
> 
> Cc: Steven Price <steven.price@xxxxxxx>
> Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> Cc: Qiang Yu <yuq825@xxxxxxxxx>
> Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx>

Thanks Dragan, while there's obviously a bunch more work to hook this up
appropriately, this at least lets drivers signal the actual requirement.

Reviewed-by: Steven Price <steven.price@xxxxxxx>

Steve

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