On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@xxxxxxxxxxx> wrote: > > Hello everyone, > > Just checking, any further thoughts about this patch? > I'm OK with this as a temp workaround because it's simple and do no harm even it's not perfect. If no other better suggestion for short term, I'll submit this at weekend. > On 2024-06-18 21:22, Dragan Simic wrote: > > On 2024-06-18 12:33, Dragan Simic wrote: > >> 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
