linux-kernel - Re: [PATCH v3 1/3] PM: domains: Make set_performance

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1e54674e-000d-d3a1-6123-715f8f445726@gmail.com>
Date:   Mon, 18 Jan 2021 21:44:43 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>,
        Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Peter Geis <pgwipeout@...il.com>,
        Nicolas Chauvet <kwizart@...il.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Kevin Hilman <khilman@...nel.org>,
        Peter De Schrijver <pdeschrijver@...dia.com>,
        Viresh Kumar <vireshk@...nel.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Matt Merhar <mattmerhar@...tonmail.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-tegra <linux-tegra@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] PM: domains: Make set_performance_state() callback
 optional

18.01.2021 13:59, Ulf Hansson пишет:
> On Mon, 18 Jan 2021 at 08:28, Viresh Kumar <viresh.kumar@...aro.org> wrote:
>>
>> On 18-01-21, 04:13, Dmitry Osipenko wrote:
>>> Make set_performance_state() callback optional in order to remove the
>>> need from power domain drivers to implement a dummy callback. If callback
>>> isn't implemented by a GENPD driver, then the performance state is passed
>>> to the parent domain.
>>>
>>> Tested-by: Peter Geis <pgwipeout@...il.com>
>>> Tested-by: Nicolas Chauvet <kwizart@...il.com>
>>> Tested-by: Matt Merhar <mattmerhar@...tonmail.com>
>>> Suggested-by: Ulf Hansson <ulf.hansson@...aro.org>
>>> Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
>>> Signed-off-by: Dmitry Osipenko <digetx@...il.com>
>>> ---
>>>  drivers/base/power/domain.c | 11 +++++------
>>>  1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>> index 9a14eedacb92..a3e1bfc233d4 100644
>>> --- a/drivers/base/power/domain.c
>>> +++ b/drivers/base/power/domain.c
>>> @@ -339,9 +339,11 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
>>>                       goto err;
>>>       }
>>>
>>> -     ret = genpd->set_performance_state(genpd, state);
>>> -     if (ret)
>>> -             goto err;
>>> +     if (genpd->set_performance_state) {
>>> +             ret = genpd->set_performance_state(genpd, state);
>>> +             if (ret)
>>> +                     goto err;
>>> +     }
>>
>> Earlier in this routine we also have this:
>>
>> if (!parent->set_performance_state)
>>         continue;
>>
>> Should we change that too ?
> 
> Good point! I certainly overlooked that when reviewing. We need to
> reevaluate the new state when propagating to the parent(s).
> 
> To me, it looks like when doing the propagation we must check if the
> parent has the ->set_performance_state() callback assigned. If so, we
> should call dev_pm_opp_xlate_performance_state(), but otherwise just
> use the value of "state", when doing the reevaluation.
> 
> Does it make sense?

I played a tad with the power domains by creating a couple dummy domains
and putting them into a parent->child chain. Yours suggestion works well
for the case where intermediate domain doesn't implement the
set_performance_state() callback, i.e. the performance state is
propagated through the whole chain instead of stopping on the domain
that doesn't implement the callback.

I'll make a v4, thanks.