The OPP structures are abused to the best here, without understanding how the OPP core and RCU locks work.
In short, the OPP pointer saved in 'rk3399_dmcfreq' can become invalid under your nose, as the OPP core may free it.
Fix various abuses around OPP structures and calls.
Compile tested only.
Reviewed-by: Chanwoo Choi cw00.choi@samsung.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org --- V1->V2: - Added Reviewed-by from Chanwoo. - Fixed an issue reported by buildbot.
drivers/devfreq/rk3399_dmc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 5063ac1a5939..75333050cdea 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -80,7 +80,6 @@ struct rk3399_dmcfreq { struct regulator *vdd_center; unsigned long rate, target_rate; unsigned long volt, target_volt; - struct dev_pm_opp *curr_opp; };
static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, @@ -102,9 +101,6 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, target_rate = dev_pm_opp_get_freq(opp); target_volt = dev_pm_opp_get_voltage(opp);
- dmcfreq->rate = dev_pm_opp_get_freq(dmcfreq->curr_opp); - dmcfreq->volt = dev_pm_opp_get_voltage(dmcfreq->curr_opp); - rcu_read_unlock();
if (dmcfreq->rate == target_rate) @@ -165,7 +161,9 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, if (err) dev_err(dev, "Cannot to set vol %lu uV\n", target_volt);
- dmcfreq->curr_opp = opp; + dmcfreq->rate = target_rate; + dmcfreq->volt = target_volt; + out: mutex_unlock(&dmcfreq->lock); return err; @@ -431,8 +429,9 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) rcu_read_unlock(); return PTR_ERR(opp); } + data->rate = dev_pm_opp_get_freq(opp); + data->volt = dev_pm_opp_get_voltage(opp); rcu_read_unlock(); - data->curr_opp = opp;
rk3399_devfreq_dmc_profile.initial_freq = data->rate;
Hi MyungJoo,
On Mon, Dec 5, 2016 at 4:23 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
The OPP structures are abused to the best here, without understanding how the OPP core and RCU locks work.
In short, the OPP pointer saved in 'rk3399_dmcfreq' can become invalid under your nose, as the OPP core may free it.
Fix various abuses around OPP structures and calls.
Compile tested only.
Reviewed-by: Chanwoo Choi cw00.choi@samsung.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
V1->V2:
- Added Reviewed-by from Chanwoo.
- Fixed an issue reported by buildbot.
I think this along with https://patchwork.kernel.org/patch/9455789/ and https://patchwork.kernel.org/patch/9455757/ should go into 4.10-rc1.
Are you going to send a pull request for this before the merge window or do you want me to apply them directly?
drivers/devfreq/rk3399_dmc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 5063ac1a5939..75333050cdea 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -80,7 +80,6 @@ struct rk3399_dmcfreq { struct regulator *vdd_center; unsigned long rate, target_rate; unsigned long volt, target_volt;
struct dev_pm_opp *curr_opp;
};
static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, @@ -102,9 +101,6 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, target_rate = dev_pm_opp_get_freq(opp); target_volt = dev_pm_opp_get_voltage(opp);
dmcfreq->rate = dev_pm_opp_get_freq(dmcfreq->curr_opp);
dmcfreq->volt = dev_pm_opp_get_voltage(dmcfreq->curr_opp);
rcu_read_unlock(); if (dmcfreq->rate == target_rate)
@@ -165,7 +161,9 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, if (err) dev_err(dev, "Cannot to set vol %lu uV\n", target_volt);
dmcfreq->curr_opp = opp;
dmcfreq->rate = target_rate;
dmcfreq->volt = target_volt;
out: mutex_unlock(&dmcfreq->lock); return err; @@ -431,8 +429,9 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) rcu_read_unlock(); return PTR_ERR(opp); }
data->rate = dev_pm_opp_get_freq(opp);
data->volt = dev_pm_opp_get_voltage(opp); rcu_read_unlock();
data->curr_opp = opp; rk3399_devfreq_dmc_profile.initial_freq = data->rate;
--
Thanks, Rafael
On Tuesday, December 06, 2016 11:40:18 PM Rafael J. Wysocki wrote:
Hi MyungJoo,
On Mon, Dec 5, 2016 at 4:23 AM, Viresh Kumar viresh.kumar@linaro.org wrote:
The OPP structures are abused to the best here, without understanding how the OPP core and RCU locks work.
In short, the OPP pointer saved in 'rk3399_dmcfreq' can become invalid under your nose, as the OPP core may free it.
Fix various abuses around OPP structures and calls.
Compile tested only.
Reviewed-by: Chanwoo Choi cw00.choi@samsung.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
V1->V2:
- Added Reviewed-by from Chanwoo.
- Fixed an issue reported by buildbot.
I think this along with https://patchwork.kernel.org/patch/9455789/ and https://patchwork.kernel.org/patch/9455757/ should go into 4.10-rc1.
Are you going to send a pull request for this before the merge window or do you want me to apply them directly?
Due to the lack of response, I'm going to apply these three patches directly.
Thanks, Rafael
linaro-kernel@lists.linaro.org