On 2024-06-18 12:33, Dragan Simic wrote:
Hello Qiang and Maxime,
On 2024-06-18 10:13, Maxime Ripard wrote:
On Tue, Jun 18, 2024 at 04:01:26PM GMT, Qiang Yu wrote:
On Tue, Jun 18, 2024 at 12:33 PM Qiang Yu <yuq825@xxxxxxxxx> wrote:
>
> I see the problem that initramfs need to build a module dependency chain,
> but lima does not call any symbol from simpleondemand governor module.
> softdep module seems to be optional while our dependency is hard one,
> can we just add MODULE_INFO(depends, _depends), or create a new
> macro called MODULE_DEPENDS()?
I had the same thoughts, because softdeps are for optional module
dependencies, while in this case it's a hard dependency. Though,
I went with adding a softdep, simply because I saw no better option
available.
This doesn't work on my side because depmod generates modules.dep
by symbol lookup instead of modinfo section. So softdep may be our
only
choice to add module dependency manually. I can accept the softdep
first, then make PM optional later.
I also thought about making devfreq optional in the Lima driver,
which would make this additional softdep much more appropriate.
Though, I'm not really sure that's a good approach, because not
having working devfreq for Lima might actually cause issues on
some devices, such as increased power consumption.
In other words, it might be better to have Lima probing fail if
devfreq can't be initialized, rather than having probing succeed
with no working devfreq. Basically, failed probing is obvious,
while a warning in the kernel log about no devfreq might easily
be overlooked, causing regressions on some devices.
It's still super fragile, and depends on the user not changing the
policy. It should be solved in some other, more robust way.
I see, but I'm not really sure how to make it more robust? In
the end, some user can blacklist the simple_ondemand governor
module, and we can't do much about it.
Introducing harddeps alongside softdeps would make sense from
the design standpoint, but the amount of required changes wouldn't
be trivial at all, on various levels.
After further investigation, it seems that the softdeps have
already seen a fair amount of abuse for what they actually aren't
intended, i.e. resolving hard dependencies. For example, have
a look at the commit d5178578bcd4 (btrfs: directly call into
crypto framework for checksumming) [1] and the lines containing
MODULE_SOFTDEP() at the very end of fs/btrfs/super.c. [2]
If a filesystem driver can rely on the abuse of softdeps, which
admittedly are a bit fragile, I think we can follow the same
approach, at least for now.
With all that in mind, I think that accepting this patch, as well
as the related Panfrost patch, [3] should be warranted. I'd keep
investigating the possibility of introducing harddeps in form
of MODULE_HARDDEP() and the related support in kmod project,
similar to the already existing softdep support, [4] but that
will inevitably take a lot of time, both for implementing it
and for reaching various Linux distributions, which is another
reason why accepting these patches seems reasonable.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d5178578bcd4
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/btrfs/super.c#n2593
[3]
https://lore.kernel.org/dri-devel/4e1e00422a14db4e2a80870afb704405da16fd1b.1718655077.git.dsimic@xxxxxxxxxxx/
[4]
https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=49d8e0b59052999de577ab732b719cfbeb89504d
