linux-kernel - Re: [PATCH] drm/lima: Mark simple

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKGbVbsGm7emEPzGuf0Xn5k22Pbjfg9J9ykJHtvDF3SacfDg6A@mail.gmail.com>
Date: Wed, 26 Jun 2024 09:11:07 +0800
From: Qiang Yu <yuq825@...il.com>
To: Dragan Simic <dsimic@...jaro.org>
Cc: Maxime Ripard <mripard@...nel.org>, dri-devel@...ts.freedesktop.org, 
	lima@...ts.freedesktop.org, maarten.lankhorst@...ux.intel.com, 
	tzimmermann@...e.de, airlied@...il.com, daniel@...ll.ch, 
	linux-kernel@...r.kernel.org, Philip Muller <philm@...jaro.org>, 
	Oliver Smith <ollieparanoid@...tmarketos.org>, Daniel Smith <danct12@...root.org>, 
	stable@...r.kernel.org
Subject: Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@...jaro.org> 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@...il.com> 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@manjaro.org/
> > [4]
> > https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=49d8e0b59052999de577ab732b719cfbeb89504d