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

[Lucas De Marchi]

On Thu, Jul 25, 2024 at 04:39:40PM GMT, Steven Price wrote:
> On 25/07/2024 15:29, Lucas De Marchi wrote:
> > 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".
>
> The simple_ondemand dependency sadly isn't visible as a symbol. It's
> currently 'fixed' by using a softdep, but that has drawbacks and doesn't
> actually express the requirement. A "weakdep" works, but has the
> drawback that it implies that the dependency is optional. This patch at
> least means that the driver can express the dependency, even if
> currently that just gets output as the same weakdep.
>
> I'm not sure what the logic was behind the name "weak" - what we

borrowed terminology from linker and weak symbols

> (currently at least) have in panfrost is not a weak dependency by the
> normal definition of the term - the driver will fail to load if the
> ondemand governor is unavailable.

there are 2 options:

1) use a softdep and let the module loading logic always load the
simple_ondemand module before panfrost
2) use a weakdep and if/when needed, do a request_module()

In both cases the tools creating the initramfs should add all
dependencies to initramfs: weakdep, softdep and dep.

> This patch doesn't solve the confusion, but at least means panfrost
> doesn't need another round of churn in the future. The alternative
> presumably is don't merge this and panfrost will have to wait until a
> proper "hard dependency" mechanism is available.

hard dependency == symbol dependency. I think the mix of terms isn't
helping. soft doesn't necessarily means "optional". AFAICT this hard dep
thing is trying to introduce a "mandatory softdep", with a name that is
already used to denote symbol dependency. And it currently does anything
other than turning it into a weakdep.

Lucas De Marchi