| Message ID | dcd19eb862a0de24b441f4d43473a01ae90e20e9.1480908064.git.viresh.kumar@linaro.org |
|---|---|
| State | Accepted |
| Commit | e37d35082e75982ef714f9e25bfa43a061d0c5e6 |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;