linux-kernel - Re: [PATCH] PM / OPP: Allow inactive opp

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: <20161129035531.GC3288@vireshk-i7>
Date:   Tue, 29 Nov 2016 09:25:31 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Stephen Boyd <sboyd@...eaurora.org>
Cc:     Rafael Wysocki <rjw@...ysocki.net>, jy0922.shim@...sung.com,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        linaro-kernel@...ts.linaro.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] PM / OPP: Allow inactive opp_device to be present in dev
 list

On 28-11-16, 18:46, Stephen Boyd wrote:
> That's a lot of lines for something that we want to backport to
> stable kernels!

Hmm, I agree.

> The whole dev_list design seems fairly broken to me. Another
> solution would be to iterate the cpumask in reverse, but there
> doesn't seem to be a construct for that and adding one is
> probably not worth the effort.
> 
> Adding yet another member to the structure and doing accounting
> in different places seems to be papering over the problem as
> well. Now we want to have "inactive" devices in the list? That
> seems like a problem for cpufreq to solve. It can decide to not
> call OPP APIs when the cpu device isn't actually physically
> removed if it wants to.
> 
> It also exposes the OPP API's strong reliance on struct device
> for everything. Really we shouldn't be storing device pointers in
> the OPP core at all because we're not treating them like the
> reference counted objects they are. The dev_list should go
> probably go away and be replaced with some sort of counter. It
> would also be nice if struct device had a pointer to the OPP
> table(s) for a device so the lookup is direct.

If the struct device gets a pointer to the opp-table, then yes we can kill the
dev-list completely. I will work on cleaning up OPP core a bit later on.

> BTW, _dev_pm_opp_remove_table() calls _find_opp_dev() twice, once
> to find the opp_table for a device and then to find the
> opp_device inside the table that was used to match up the table
> in the first place. Madness!
> 
> Anyway, rant over, how about handing out the opp table pointer to
> the caller so they can pass it back in when they call the put
> side? That should fix the same problem if I understand correctly.

Yes, that can be a solution for the time being.

> We should think about changing the API further so that callers
> have to "get" the OPP table cookie for their device and then pass
> that pointer to the dev_pm_*_set() APIs instead of passing a
> struct device pointer. That would save lots of cycles searching
> for something we already had.

Hmm, we need to do some cleanup soon I believe. Also note that we want to kill
the RCU thing :)

> -static inline void dev_pm_opp_put_regulator(struct device *dev) {}
> +static inline void dev_pm_opp_put_regulator(struct opp_table *opp_table) {}

We need to modify few more things as well. I will send a patch for that soon.

-- 
viresh