[PATCH] drm/lima: Mark simple_ondemand governor as softdep

[PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Dragan Simic]

Lima DRM driver uses devfreq to perform DVFS, while using simple_ondemand
devfreq governor by default.  This causes driver initialization to fail on
boot when simple_ondemand governor isn't built into the kernel statically,
as a result of the missing module dependency and, consequently, the required
governor module not being included in the initial ramdisk.  Thus, let's mark
simple_ondemand governor as a softdep for Lima, to have its kernel module
included in the initial ramdisk.

This is a rather longstanding issue that has forced distributions to build
devfreq governors statically into their kernels, [1][2] or may have forced
some users to introduce unnecessary workarounds.

Having simple_ondemand marked as a softdep for Lima may not resolve this
issue for all Linux distributions.  In particular, it will remain unresolved
for the distributions whose utilities for the initial ramdisk generation do
not handle the available softdep information [3] properly yet.  However, some
Linux distributions already handle softdeps properly while generating their
initial ramdisks, [4] and this is a prerequisite step in the right direction
for the distributions that don't handle them properly yet.

[1] https://gitlab.manjaro.org/manjaro-arm/packages/core/linux-pinephone/-/blob/6.7-megi/config?ref_type=heads#L5749
[2] https://gitlab.com/postmarketOS/pmaports/-/blob/7f64e287e7732c9eaa029653e73ca3d4ba1c8598/main/linux-postmarketos-allwinner/config-postmarketos-allwinner.aarch64#L4654
[3] https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=49d8e0b59052999de577ab732b719cfbeb89504d
[4] https://github.com/archlinux/mkinitcpio/commit/97ac4d37aae084a050be512f6d8f4489054668ad

Cc: Philip Muller <philm@manjaro.org>
Cc: Oliver Smith <ollieparanoid@postmarketos.org>
Cc: Daniel Smith <danct12@disroot.org>
Cc: stable@vger.kernel.org
Fixes: 1996970773a3 ("drm/lima: Add optional devfreq and cooling device support")
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
 drivers/gpu/drm/lima/lima_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c
index 739c865b556f..10bce18b7c31 100644
--- a/drivers/gpu/drm/lima/lima_drv.c
+++ b/drivers/gpu/drm/lima/lima_drv.c
@@ -501,3 +501,4 @@ module_platform_driver(lima_platform_driver);
 MODULE_AUTHOR("Lima Project Developers");
 MODULE_DESCRIPTION("Lima DRM Driver");
 MODULE_LICENSE("GPL v2");
+MODULE_SOFTDEP("pre: governor_simpleondemand");

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Qiang Yu]

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()?

On Tue, Jun 18, 2024 at 4:22 AM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Lima DRM driver uses devfreq to perform DVFS, while using simple_ondemand
> devfreq governor by default.  This causes driver initialization to fail on
> boot when simple_ondemand governor isn't built into the kernel statically,
> as a result of the missing module dependency and, consequently, the required
> governor module not being included in the initial ramdisk.  Thus, let's mark
> simple_ondemand governor as a softdep for Lima, to have its kernel module
> included in the initial ramdisk.
>
> This is a rather longstanding issue that has forced distributions to build
> devfreq governors statically into their kernels, [1][2] or may have forced
> some users to introduce unnecessary workarounds.
>
> Having simple_ondemand marked as a softdep for Lima may not resolve this
> issue for all Linux distributions.  In particular, it will remain unresolved
> for the distributions whose utilities for the initial ramdisk generation do
> not handle the available softdep information [3] properly yet.  However, some
> Linux distributions already handle softdeps properly while generating their
> initial ramdisks, [4] and this is a prerequisite step in the right direction
> for the distributions that don't handle them properly yet.
>
> [1] https://gitlab.manjaro.org/manjaro-arm/packages/core/linux-pinephone/-/blob/6.7-megi/config?ref_type=heads#L5749
> [2] https://gitlab.com/postmarketOS/pmaports/-/blob/7f64e287e7732c9eaa029653e73ca3d4ba1c8598/main/linux-postmarketos-allwinner/config-postmarketos-allwinner.aarch64#L4654
> [3] https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=49d8e0b59052999de577ab732b719cfbeb89504d
> [4] https://github.com/archlinux/mkinitcpio/commit/97ac4d37aae084a050be512f6d8f4489054668ad
>
> Cc: Philip Muller <philm@manjaro.org>
> Cc: Oliver Smith <ollieparanoid@postmarketos.org>
> Cc: Daniel Smith <danct12@disroot.org>
> Cc: stable@vger.kernel.org
> Fixes: 1996970773a3 ("drm/lima: Add optional devfreq and cooling device support")
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  drivers/gpu/drm/lima/lima_drv.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c
> index 739c865b556f..10bce18b7c31 100644
> --- a/drivers/gpu/drm/lima/lima_drv.c
> +++ b/drivers/gpu/drm/lima/lima_drv.c
> @@ -501,3 +501,4 @@ module_platform_driver(lima_platform_driver);
>  MODULE_AUTHOR("Lima Project Developers");
>  MODULE_DESCRIPTION("Lima DRM Driver");
>  MODULE_LICENSE("GPL v2");
> +MODULE_SOFTDEP("pre: governor_simpleondemand");

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Qiang Yu]

On Tue, Jun 18, 2024 at 12:33 PM Qiang Yu <yuq825@gmail.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()?
>
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.

> On Tue, Jun 18, 2024 at 4:22 AM Dragan Simic <dsimic@manjaro.org> wrote:
> >
> > Lima DRM driver uses devfreq to perform DVFS, while using simple_ondemand
> > devfreq governor by default.  This causes driver initialization to fail on
> > boot when simple_ondemand governor isn't built into the kernel statically,
> > as a result of the missing module dependency and, consequently, the required
> > governor module not being included in the initial ramdisk.  Thus, let's mark
> > simple_ondemand governor as a softdep for Lima, to have its kernel module
> > included in the initial ramdisk.
> >
> > This is a rather longstanding issue that has forced distributions to build
> > devfreq governors statically into their kernels, [1][2] or may have forced
> > some users to introduce unnecessary workarounds.
> >
> > Having simple_ondemand marked as a softdep for Lima may not resolve this
> > issue for all Linux distributions.  In particular, it will remain unresolved
> > for the distributions whose utilities for the initial ramdisk generation do
> > not handle the available softdep information [3] properly yet.  However, some
> > Linux distributions already handle softdeps properly while generating their
> > initial ramdisks, [4] and this is a prerequisite step in the right direction
> > for the distributions that don't handle them properly yet.
> >
> > [1] https://gitlab.manjaro.org/manjaro-arm/packages/core/linux-pinephone/-/blob/6.7-megi/config?ref_type=heads#L5749
> > [2] https://gitlab.com/postmarketOS/pmaports/-/blob/7f64e287e7732c9eaa029653e73ca3d4ba1c8598/main/linux-postmarketos-allwinner/config-postmarketos-allwinner.aarch64#L4654
> > [3] https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/commit/?id=49d8e0b59052999de577ab732b719cfbeb89504d
> > [4] https://github.com/archlinux/mkinitcpio/commit/97ac4d37aae084a050be512f6d8f4489054668ad
> >
> > Cc: Philip Muller <philm@manjaro.org>
> > Cc: Oliver Smith <ollieparanoid@postmarketos.org>
> > Cc: Daniel Smith <danct12@disroot.org>
> > Cc: stable@vger.kernel.org
> > Fixes: 1996970773a3 ("drm/lima: Add optional devfreq and cooling device support")
> > Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> > ---
> >  drivers/gpu/drm/lima/lima_drv.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c
> > index 739c865b556f..10bce18b7c31 100644
> > --- a/drivers/gpu/drm/lima/lima_drv.c
> > +++ b/drivers/gpu/drm/lima/lima_drv.c
> > @@ -501,3 +501,4 @@ module_platform_driver(lima_platform_driver);
> >  MODULE_AUTHOR("Lima Project Developers");
> >  MODULE_DESCRIPTION("Lima DRM Driver");
> >  MODULE_LICENSE("GPL v2");
> > +MODULE_SOFTDEP("pre: governor_simpleondemand");

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

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@gmail.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()?
> >
> 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.

It's still super fragile, and depends on the user not changing the
policy. It should be solved in some other, more robust way.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Dragan Simic]

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@gmail.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.

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Dragan Simic]

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@gmail.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

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Dragan Simic]

Hello everyone,

Just checking, any further thoughts about this patch?

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@gmail.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

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Qiang Yu]

On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@manjaro.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@gmail.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

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Dragan Simic]

Hello Qiang,

On 2024-06-26 03:11, Qiang Yu wrote:
> On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@manjaro.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.

Thanks.  Just as you described it, it's far from perfect, but it's still
fine until there's a better solution, such as harddeps.  I'll continue 
my
research about the possibility for adding harddeps, which would 
hopefully
replace quite a few instances of the softdep (ab)use.

>> 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@gmail.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

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Qiang Yu]

Applied to drm-misc-next.

On Wed, Jun 26, 2024 at 2:49 PM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Hello Qiang,
>
> On 2024-06-26 03:11, Qiang Yu wrote:
> > On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@manjaro.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.
>
> Thanks.  Just as you described it, it's far from perfect, but it's still
> fine until there's a better solution, such as harddeps.  I'll continue
> my
> research about the possibility for adding harddeps, which would
> hopefully
> replace quite a few instances of the softdep (ab)use.
>
> >> 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@gmail.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

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Dragan Simic]

Hello Qiang,

On 2024-06-26 08:49, Dragan Simic wrote:
> On 2024-06-26 03:11, Qiang Yu wrote:
>> On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@manjaro.org> 
>> wrote:
>>> 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.
> 
> Thanks.  Just as you described it, it's far from perfect, but it's 
> still
> fine until there's a better solution, such as harddeps.  I'll continue 
> my
> research about the possibility for adding harddeps, which would 
> hopefully
> replace quite a few instances of the softdep (ab)use.

Another option has become available for expressing additional module
dependencies, weakdeps. [1][2]  Long story short, weakdeps are similar
to softdeps, in the sense of telling the initial ramdisk utilities to
include additional kernel modules, but weakdeps result in no module
loading being performed by userspace.

Maybe "weak" isn't the best possible word choice (arguably, "soft" also
wasn't the best word choice), but weakdeps should be a better choice for
use with Lima and governor_simpleondemand, because weakdeps provide the
required information to the utilities used to generate initial ramdisk,
while the actual module loading is left to the kernel.

The recent addition of weakdeps renders the previously mentioned 
harddeps
obsolete, because weakdeps actually do what we need.  Obviously, "weak"
doesn't go along very well with the actual nature of the dependency 
between
Lima and governor_simpleondemand, but it's pretty much just the somewhat
unfortunate word choice.

The support for weakdeps has been already added to the kmod [3][4] and
Dracut [5] userspace utilities.  I'll hopefully add support for weakdeps
to mkinitcpio [6] rather soon.

Maybe we could actually add MODULE_HARDDEP() as some kind of syntactic
sugar, which would currently be an alias for MODULE_WEAKDEP(), so the
actual hard module dependencies could be expressed properly, and 
possibly
handled differently in the future, with no need to go back and track all
such instances of hard module dependencies.

With all this in mind, here's what I'm going to do:

1) Submit a patch that adds MODULE_HARDDEP() as syntactic sugar
2) Implement support for weakdeps in Arch Linux's mkinitcpio [6]
3) Depending on what kind of feedback the MODULE_HARDDEP() patch 
receives,
    I'll submit follow-up patches for Lima and Panfrost, which will swap
    uses of MODULE_SOFTDEP() with MODULE_HARDDEP() or MODULE_WEAKDEP()

Looking forward to your thoughts.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/module.h?id=61842868de13aa7fd7391c626e889f4d6f1450bf
[2] 
https://lore.kernel.org/linux-kernel/20240724102349.430078-1-jtornosm@redhat.com/T/#u
[3] 
https://github.com/kmod-project/kmod/commit/05828b4a6e9327a63ef94df544a042b5e9ce4fe7
[4] 
https://github.com/kmod-project/kmod/commit/d06712b51404061eef92cb275b8303814fca86ec
[5] 
https://github.com/dracut-ng/dracut-ng/commit/8517a6be5e20f4a6d87e55fce35ee3e29e2a1150
[6] https://gitlab.archlinux.org/archlinux/mkinitcpio/mkinitcpio


>>> 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@gmail.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

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Qiang Yu]

Yeah, I agree weakdep is a better choice here. It solves the confusion
of softdep
which the depend module is optional.

But I prefer using weakdep directly instead of creating an aliasing of
it which has
no actual difference.


On Thu, Jul 25, 2024 at 4:21 PM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Hello Qiang,
>
> On 2024-06-26 08:49, Dragan Simic wrote:
> > On 2024-06-26 03:11, Qiang Yu wrote:
> >> On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@manjaro.org>
> >> wrote:
> >>> 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.
> >
> > Thanks.  Just as you described it, it's far from perfect, but it's
> > still
> > fine until there's a better solution, such as harddeps.  I'll continue
> > my
> > research about the possibility for adding harddeps, which would
> > hopefully
> > replace quite a few instances of the softdep (ab)use.
>
> Another option has become available for expressing additional module
> dependencies, weakdeps. [1][2]  Long story short, weakdeps are similar
> to softdeps, in the sense of telling the initial ramdisk utilities to
> include additional kernel modules, but weakdeps result in no module
> loading being performed by userspace.
>
> Maybe "weak" isn't the best possible word choice (arguably, "soft" also
> wasn't the best word choice), but weakdeps should be a better choice for
> use with Lima and governor_simpleondemand, because weakdeps provide the
> required information to the utilities used to generate initial ramdisk,
> while the actual module loading is left to the kernel.
>
> The recent addition of weakdeps renders the previously mentioned
> harddeps
> obsolete, because weakdeps actually do what we need.  Obviously, "weak"
> doesn't go along very well with the actual nature of the dependency
> between
> Lima and governor_simpleondemand, but it's pretty much just the somewhat
> unfortunate word choice.
>
> The support for weakdeps has been already added to the kmod [3][4] and
> Dracut [5] userspace utilities.  I'll hopefully add support for weakdeps
> to mkinitcpio [6] rather soon.
>
> Maybe we could actually add MODULE_HARDDEP() as some kind of syntactic
> sugar, which would currently be an alias for MODULE_WEAKDEP(), so the
> actual hard module dependencies could be expressed properly, and
> possibly
> handled differently in the future, with no need to go back and track all
> such instances of hard module dependencies.
>
> With all this in mind, here's what I'm going to do:
>
> 1) Submit a patch that adds MODULE_HARDDEP() as syntactic sugar
> 2) Implement support for weakdeps in Arch Linux's mkinitcpio [6]
> 3) Depending on what kind of feedback the MODULE_HARDDEP() patch
> receives,
>     I'll submit follow-up patches for Lima and Panfrost, which will swap
>     uses of MODULE_SOFTDEP() with MODULE_HARDDEP() or MODULE_WEAKDEP()
>
> Looking forward to your thoughts.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/module.h?id=61842868de13aa7fd7391c626e889f4d6f1450bf
> [2]
> https://lore.kernel.org/linux-kernel/20240724102349.430078-1-jtornosm@redhat.com/T/#u
> [3]
> https://github.com/kmod-project/kmod/commit/05828b4a6e9327a63ef94df544a042b5e9ce4fe7
> [4]
> https://github.com/kmod-project/kmod/commit/d06712b51404061eef92cb275b8303814fca86ec
> [5]
> https://github.com/dracut-ng/dracut-ng/commit/8517a6be5e20f4a6d87e55fce35ee3e29e2a1150
> [6] https://gitlab.archlinux.org/archlinux/mkinitcpio/mkinitcpio
>
>
> >>> 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@gmail.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

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Dragan Simic]

Hello Qiang Yu,

On 2024-07-26 08:07, Qiang Yu wrote:
> Yeah, I agree weakdep is a better choice here. It solves the confusion
> of softdep which the depend module is optional.

Thanks, I'm glad that you agree.

> But I prefer using weakdep directly instead of creating an aliasing of
> it which has no actual difference.

Just checking, did you have a chance to read what I wrote in my earlier
response on the linux-modules mailing list, [7] which includes a rather
elaborate explanation of the intent behind MODULE_HARDDEP being 
currently
just a proposed alias for MODULE_WEAKDEP?  It also describes why using
this alias might save use some time and effort in the future.

[7] 
https://lore.kernel.org/linux-modules/0720a516416a92a8f683053d37ee9481@manjaro.org/

> On Thu, Jul 25, 2024 at 4:21 PM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> 
>> Hello Qiang,
>> 
>> On 2024-06-26 08:49, Dragan Simic wrote:
>> > On 2024-06-26 03:11, Qiang Yu wrote:
>> >> On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@manjaro.org>
>> >> wrote:
>> >>> 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.
>> >
>> > Thanks.  Just as you described it, it's far from perfect, but it's
>> > still
>> > fine until there's a better solution, such as harddeps.  I'll continue
>> > my
>> > research about the possibility for adding harddeps, which would
>> > hopefully
>> > replace quite a few instances of the softdep (ab)use.
>> 
>> Another option has become available for expressing additional module
>> dependencies, weakdeps. [1][2]  Long story short, weakdeps are similar
>> to softdeps, in the sense of telling the initial ramdisk utilities to
>> include additional kernel modules, but weakdeps result in no module
>> loading being performed by userspace.
>> 
>> Maybe "weak" isn't the best possible word choice (arguably, "soft" 
>> also
>> wasn't the best word choice), but weakdeps should be a better choice 
>> for
>> use with Lima and governor_simpleondemand, because weakdeps provide 
>> the
>> required information to the utilities used to generate initial 
>> ramdisk,
>> while the actual module loading is left to the kernel.
>> 
>> The recent addition of weakdeps renders the previously mentioned
>> harddeps
>> obsolete, because weakdeps actually do what we need.  Obviously, 
>> "weak"
>> doesn't go along very well with the actual nature of the dependency
>> between
>> Lima and governor_simpleondemand, but it's pretty much just the 
>> somewhat
>> unfortunate word choice.
>> 
>> The support for weakdeps has been already added to the kmod [3][4] and
>> Dracut [5] userspace utilities.  I'll hopefully add support for 
>> weakdeps
>> to mkinitcpio [6] rather soon.
>> 
>> Maybe we could actually add MODULE_HARDDEP() as some kind of syntactic
>> sugar, which would currently be an alias for MODULE_WEAKDEP(), so the
>> actual hard module dependencies could be expressed properly, and
>> possibly
>> handled differently in the future, with no need to go back and track 
>> all
>> such instances of hard module dependencies.
>> 
>> With all this in mind, here's what I'm going to do:
>> 
>> 1) Submit a patch that adds MODULE_HARDDEP() as syntactic sugar
>> 2) Implement support for weakdeps in Arch Linux's mkinitcpio [6]
>> 3) Depending on what kind of feedback the MODULE_HARDDEP() patch
>> receives,
>>     I'll submit follow-up patches for Lima and Panfrost, which will 
>> swap
>>     uses of MODULE_SOFTDEP() with MODULE_HARDDEP() or MODULE_WEAKDEP()
>> 
>> Looking forward to your thoughts.
>> 
>> [1] 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/module.h?id=61842868de13aa7fd7391c626e889f4d6f1450bf
>> [2] 
>> https://lore.kernel.org/linux-kernel/20240724102349.430078-1-jtornosm@redhat.com/T/#u
>> [3] 
>> https://github.com/kmod-project/kmod/commit/05828b4a6e9327a63ef94df544a042b5e9ce4fe7
>> [4] 
>> https://github.com/kmod-project/kmod/commit/d06712b51404061eef92cb275b8303814fca86ec
>> [5] 
>> https://github.com/dracut-ng/dracut-ng/commit/8517a6be5e20f4a6d87e55fce35ee3e29e2a1150
>> [6] https://gitlab.archlinux.org/archlinux/mkinitcpio/mkinitcpio

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Qiang Yu]

On Fri, Jul 26, 2024 at 4:03 PM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Hello Qiang Yu,
>
> On 2024-07-26 08:07, Qiang Yu wrote:
> > Yeah, I agree weakdep is a better choice here. It solves the confusion
> > of softdep which the depend module is optional.
>
> Thanks, I'm glad that you agree.
>
> > But I prefer using weakdep directly instead of creating an aliasing of
> > it which has no actual difference.
>
> Just checking, did you have a chance to read what I wrote in my earlier
> response on the linux-modules mailing list, [7] which includes a rather
> elaborate explanation of the intent behind MODULE_HARDDEP being
> currently
> just a proposed alias for MODULE_WEAKDEP?  It also describes why using
> this alias might save use some time and effort in the future.
>
> [7]
> https://lore.kernel.org/linux-modules/0720a516416a92a8f683053d37ee9481@manjaro.org/
>
Yeah, I've seen that mail. But I haven't seen clearly how weakdep will change
in the future which could break our usage here. As an interface exposed to other
users, I expect it should be stable.

> > On Thu, Jul 25, 2024 at 4:21 PM Dragan Simic <dsimic@manjaro.org>
> > wrote:
> >>
> >> Hello Qiang,
> >>
> >> On 2024-06-26 08:49, Dragan Simic wrote:
> >> > On 2024-06-26 03:11, Qiang Yu wrote:
> >> >> On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@manjaro.org>
> >> >> wrote:
> >> >>> 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.
> >> >
> >> > Thanks.  Just as you described it, it's far from perfect, but it's
> >> > still
> >> > fine until there's a better solution, such as harddeps.  I'll continue
> >> > my
> >> > research about the possibility for adding harddeps, which would
> >> > hopefully
> >> > replace quite a few instances of the softdep (ab)use.
> >>
> >> Another option has become available for expressing additional module
> >> dependencies, weakdeps. [1][2]  Long story short, weakdeps are similar
> >> to softdeps, in the sense of telling the initial ramdisk utilities to
> >> include additional kernel modules, but weakdeps result in no module
> >> loading being performed by userspace.
> >>
> >> Maybe "weak" isn't the best possible word choice (arguably, "soft"
> >> also
> >> wasn't the best word choice), but weakdeps should be a better choice
> >> for
> >> use with Lima and governor_simpleondemand, because weakdeps provide
> >> the
> >> required information to the utilities used to generate initial
> >> ramdisk,
> >> while the actual module loading is left to the kernel.
> >>
> >> The recent addition of weakdeps renders the previously mentioned
> >> harddeps
> >> obsolete, because weakdeps actually do what we need.  Obviously,
> >> "weak"
> >> doesn't go along very well with the actual nature of the dependency
> >> between
> >> Lima and governor_simpleondemand, but it's pretty much just the
> >> somewhat
> >> unfortunate word choice.
> >>
> >> The support for weakdeps has been already added to the kmod [3][4] and
> >> Dracut [5] userspace utilities.  I'll hopefully add support for
> >> weakdeps
> >> to mkinitcpio [6] rather soon.
> >>
> >> Maybe we could actually add MODULE_HARDDEP() as some kind of syntactic
> >> sugar, which would currently be an alias for MODULE_WEAKDEP(), so the
> >> actual hard module dependencies could be expressed properly, and
> >> possibly
> >> handled differently in the future, with no need to go back and track
> >> all
> >> such instances of hard module dependencies.
> >>
> >> With all this in mind, here's what I'm going to do:
> >>
> >> 1) Submit a patch that adds MODULE_HARDDEP() as syntactic sugar
> >> 2) Implement support for weakdeps in Arch Linux's mkinitcpio [6]
> >> 3) Depending on what kind of feedback the MODULE_HARDDEP() patch
> >> receives,
> >>     I'll submit follow-up patches for Lima and Panfrost, which will
> >> swap
> >>     uses of MODULE_SOFTDEP() with MODULE_HARDDEP() or MODULE_WEAKDEP()
> >>
> >> Looking forward to your thoughts.
> >>
> >> [1]
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/module.h?id=61842868de13aa7fd7391c626e889f4d6f1450bf
> >> [2]
> >> https://lore.kernel.org/linux-kernel/20240724102349.430078-1-jtornosm@redhat.com/T/#u
> >> [3]
> >> https://github.com/kmod-project/kmod/commit/05828b4a6e9327a63ef94df544a042b5e9ce4fe7
> >> [4]
> >> https://github.com/kmod-project/kmod/commit/d06712b51404061eef92cb275b8303814fca86ec
> >> [5]
> >> https://github.com/dracut-ng/dracut-ng/commit/8517a6be5e20f4a6d87e55fce35ee3e29e2a1150
> >> [6] https://gitlab.archlinux.org/archlinux/mkinitcpio/mkinitcpio

Re: [PATCH] drm/lima: Mark simple_ondemand governor as softdep

[Dragan Simic]

On 2024-07-26 10:54, Qiang Yu wrote:
> On Fri, Jul 26, 2024 at 4:03 PM Dragan Simic <dsimic@manjaro.org> 
> wrote:
>> On 2024-07-26 08:07, Qiang Yu wrote:
>> > Yeah, I agree weakdep is a better choice here. It solves the confusion
>> > of softdep which the depend module is optional.
>> 
>> Thanks, I'm glad that you agree.
>> 
>> > But I prefer using weakdep directly instead of creating an aliasing of
>> > it which has no actual difference.
>> 
>> Just checking, did you have a chance to read what I wrote in my 
>> earlier
>> response on the linux-modules mailing list, [7] which includes a 
>> rather
>> elaborate explanation of the intent behind MODULE_HARDDEP being
>> currently
>> just a proposed alias for MODULE_WEAKDEP?  It also describes why using
>> this alias might save use some time and effort in the future.
>> 
>> [7] 
>> https://lore.kernel.org/linux-modules/0720a516416a92a8f683053d37ee9481@manjaro.org/
>> 
> Yeah, I've seen that mail. But I haven't seen clearly how weakdep will 
> change
> in the future which could break our usage here. As an interface exposed 
> to other
> users, I expect it should be stable.

Let me clarify, please.

The intent isn't to prevent breakage, but to future-proof our weakdeps
that are actually harddeps under the hood.  Of course, weakdeps aren't
expected to become unsuitable for our needs in the future, but we might
actually need to treat our uses of weakdeps as harddeps at some point,
so marking them as (currently aliased) harddeps leaves clear "earmarks"
for us in the future.

The Btrfs example, which I used in my earlier response on linux-modules,
shows how such "earmarks" can be useful after some time passes.

>> > On Thu, Jul 25, 2024 at 4:21 PM Dragan Simic <dsimic@manjaro.org>
>> > wrote:
>> >>
>> >> Hello Qiang,
>> >>
>> >> On 2024-06-26 08:49, Dragan Simic wrote:
>> >> > On 2024-06-26 03:11, Qiang Yu wrote:
>> >> >> On Wed, Jun 26, 2024 at 2:15 AM Dragan Simic <dsimic@manjaro.org>
>> >> >> wrote:
>> >> >>> 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.
>> >> >
>> >> > Thanks.  Just as you described it, it's far from perfect, but it's
>> >> > still
>> >> > fine until there's a better solution, such as harddeps.  I'll continue
>> >> > my
>> >> > research about the possibility for adding harddeps, which would
>> >> > hopefully
>> >> > replace quite a few instances of the softdep (ab)use.
>> >>
>> >> Another option has become available for expressing additional module
>> >> dependencies, weakdeps. [1][2]  Long story short, weakdeps are similar
>> >> to softdeps, in the sense of telling the initial ramdisk utilities to
>> >> include additional kernel modules, but weakdeps result in no module
>> >> loading being performed by userspace.
>> >>
>> >> Maybe "weak" isn't the best possible word choice (arguably, "soft"
>> >> also
>> >> wasn't the best word choice), but weakdeps should be a better choice
>> >> for
>> >> use with Lima and governor_simpleondemand, because weakdeps provide
>> >> the
>> >> required information to the utilities used to generate initial
>> >> ramdisk,
>> >> while the actual module loading is left to the kernel.
>> >>
>> >> The recent addition of weakdeps renders the previously mentioned
>> >> harddeps
>> >> obsolete, because weakdeps actually do what we need.  Obviously,
>> >> "weak"
>> >> doesn't go along very well with the actual nature of the dependency
>> >> between
>> >> Lima and governor_simpleondemand, but it's pretty much just the
>> >> somewhat
>> >> unfortunate word choice.
>> >>
>> >> The support for weakdeps has been already added to the kmod [3][4] and
>> >> Dracut [5] userspace utilities.  I'll hopefully add support for
>> >> weakdeps
>> >> to mkinitcpio [6] rather soon.
>> >>
>> >> Maybe we could actually add MODULE_HARDDEP() as some kind of syntactic
>> >> sugar, which would currently be an alias for MODULE_WEAKDEP(), so the
>> >> actual hard module dependencies could be expressed properly, and
>> >> possibly
>> >> handled differently in the future, with no need to go back and track
>> >> all
>> >> such instances of hard module dependencies.
>> >>
>> >> With all this in mind, here's what I'm going to do:
>> >>
>> >> 1) Submit a patch that adds MODULE_HARDDEP() as syntactic sugar
>> >> 2) Implement support for weakdeps in Arch Linux's mkinitcpio [6]
>> >> 3) Depending on what kind of feedback the MODULE_HARDDEP() patch
>> >> receives,
>> >>     I'll submit follow-up patches for Lima and Panfrost, which will
>> >> swap
>> >>     uses of MODULE_SOFTDEP() with MODULE_HARDDEP() or MODULE_WEAKDEP()
>> >>
>> >> Looking forward to your thoughts.
>> >>
>> >> [1]
>> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/linux/module.h?id=61842868de13aa7fd7391c626e889f4d6f1450bf
>> >> [2]
>> >> https://lore.kernel.org/linux-kernel/20240724102349.430078-1-jtornosm@redhat.com/T/#u
>> >> [3]
>> >> https://github.com/kmod-project/kmod/commit/05828b4a6e9327a63ef94df544a042b5e9ce4fe7
>> >> [4]
>> >> https://github.com/kmod-project/kmod/commit/d06712b51404061eef92cb275b8303814fca86ec
>> >> [5]
>> >> https://github.com/dracut-ng/dracut-ng/commit/8517a6be5e20f4a6d87e55fce35ee3e29e2a1150
>> >> [6] https://gitlab.archlinux.org/archlinux/mkinitcpio/mkinitcpio