| Message ID | 20220510080136.11950-1-wanjiabing@vivo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cpufreq: mediatek: Fix potential deadlock problem in mtk_cpufreq_set_target | expand |
On Tue, 2022-05-10 at 16:01 +0800, Wan Jiabing wrote: > Fix following coccichek error: > ./drivers/cpufreq/mediatek-cpufreq.c:199:2-8: preceding lock on line > ./drivers/cpufreq/mediatek-cpufreq.c:208:2-8: preceding lock on line > > mutex_lock is acquired but not released before return. > Use 'goto out' to help releasing the mutex_lock. > > Fixes: c210063b40ac ("cupful: mediatek: Add opp notification > support") > Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> > --- > drivers/cpufreq/mediatek-cpufreq.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c > b/drivers/cpufreq/mediatek-cpufreq.c > index 75bf21ddf61f..4c6d53c99d79 100644 > --- a/drivers/cpufreq/mediatek-cpufreq.c > +++ b/drivers/cpufreq/mediatek-cpufreq.c > @@ -196,7 +196,8 @@ static int mtk_cpufreq_set_target(struct > cpufreq_policy *policy, > > if (pre_vproc < 0) { > dev_err(cpu_dev, "invalid Vproc value: %d\n", > pre_vproc); > - return pre_vproc; > + ret = pre_vproc; > + goto out; > } > > freq_hz = freq_table[index].frequency * 1000; > @@ -205,7 +206,8 @@ static int mtk_cpufreq_set_target(struct > cpufreq_policy *policy, > if (IS_ERR(opp)) { > dev_err(cpu_dev, "cpu%d: failed to find OPP for %ld\n", > policy->cpu, freq_hz); > - return PTR_ERR(opp); > + ret = PTR_ERR(opp); > + goto out; > } > vproc = dev_pm_opp_get_voltage(opp); > dev_pm_opp_put(opp); Hello Wan, Thanks for checking this issue. I did not notice this issue when I am asked to move the mutex lock here in my patch. Reviewed-by: Rex-BC Chen <rex-bc.chen@mediatek.com> BRs, Rex
On 10-05-22, 16:01, Wan Jiabing wrote: > Fix following coccichek error: > ./drivers/cpufreq/mediatek-cpufreq.c:199:2-8: preceding lock on line > ./drivers/cpufreq/mediatek-cpufreq.c:208:2-8: preceding lock on line > > mutex_lock is acquired but not released before return. > Use 'goto out' to help releasing the mutex_lock. > > Fixes: c210063b40ac ("cupful: mediatek: Add opp notification support") cupful ?? > Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> > --- > drivers/cpufreq/mediatek-cpufreq.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c > index 75bf21ddf61f..4c6d53c99d79 100644 > --- a/drivers/cpufreq/mediatek-cpufreq.c > +++ b/drivers/cpufreq/mediatek-cpufreq.c > @@ -196,7 +196,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > > if (pre_vproc < 0) { > dev_err(cpu_dev, "invalid Vproc value: %d\n", pre_vproc); > - return pre_vproc; > + ret = pre_vproc; > + goto out; > } > > freq_hz = freq_table[index].frequency * 1000; > @@ -205,7 +206,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > if (IS_ERR(opp)) { > dev_err(cpu_dev, "cpu%d: failed to find OPP for %ld\n", > policy->cpu, freq_hz); > - return PTR_ERR(opp); > + ret = PTR_ERR(opp); > + goto out; > } > vproc = dev_pm_opp_get_voltage(opp); > dev_pm_opp_put(opp); > -- > 2.35.1
Hi, Viresh Kumar On 2022/5/10 16:12, Viresh Kumar wrote: > On 10-05-22, 16:01, Wan Jiabing wrote: >> Fix following coccichek error: >> ./drivers/cpufreq/mediatek-cpufreq.c:199:2-8: preceding lock on line >> ./drivers/cpufreq/mediatek-cpufreq.c:208:2-8: preceding lock on line >> >> mutex_lock is acquired but not released before return. >> Use 'goto out' to help releasing the mutex_lock. >> >> Fixes: c210063b40ac ("cupful: mediatek: Add opp notification support") > cupful ?? Sorry for the typo... By the way, is this patch correct? I am not sure whether it should return with the mutex_lock. But IMO, it should release the lock before return. If correct, I'll fix this typo in v2. Thanks, Wan Jiabing >> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> >> --- >> drivers/cpufreq/mediatek-cpufreq.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c >> index 75bf21ddf61f..4c6d53c99d79 100644 >> --- a/drivers/cpufreq/mediatek-cpufreq.c >> +++ b/drivers/cpufreq/mediatek-cpufreq.c >> @@ -196,7 +196,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, >> >> if (pre_vproc < 0) { >> dev_err(cpu_dev, "invalid Vproc value: %d\n", pre_vproc); >> - return pre_vproc; >> + ret = pre_vproc; >> + goto out; >> } >> >> freq_hz = freq_table[index].frequency * 1000; >> @@ -205,7 +206,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, >> if (IS_ERR(opp)) { >> dev_err(cpu_dev, "cpu%d: failed to find OPP for %ld\n", >> policy->cpu, freq_hz); >> - return PTR_ERR(opp); >> + ret = PTR_ERR(opp); >> + goto out; >> } >> vproc = dev_pm_opp_get_voltage(opp); >> dev_pm_opp_put(opp); >> -- >> 2.35.1
On 10-05-22, 16:26, Jiabing Wan wrote: > Hi, Viresh Kumar > > On 2022/5/10 16:12, Viresh Kumar wrote: > > On 10-05-22, 16:01, Wan Jiabing wrote: > > > Fix following coccichek error: > > > ./drivers/cpufreq/mediatek-cpufreq.c:199:2-8: preceding lock on line > > > ./drivers/cpufreq/mediatek-cpufreq.c:208:2-8: preceding lock on line > > > > > > mutex_lock is acquired but not released before return. > > > Use 'goto out' to help releasing the mutex_lock. > > > > > > Fixes: c210063b40ac ("cupful: mediatek: Add opp notification support") > > cupful ?? > > Sorry for the typo... Did you write this line by hand or generate it with Git ? There shouldn't be a typo here. > By the way, is this patch correct? > > I am not sure whether it should return with the mutex_lock. > But IMO, it should release the lock before return. Yes, we need to release the lock.
On 2022/5/10 16:50, Viresh Kumar wrote: > On 10-05-22, 16:26, Jiabing Wan wrote: >> Hi, Viresh Kumar >> >> On 2022/5/10 16:12, Viresh Kumar wrote: >>> On 10-05-22, 16:01, Wan Jiabing wrote: >>>> Fix following coccichek error: >>>> ./drivers/cpufreq/mediatek-cpufreq.c:199:2-8: preceding lock on line >>>> ./drivers/cpufreq/mediatek-cpufreq.c:208:2-8: preceding lock on line >>>> >>>> mutex_lock is acquired but not released before return. >>>> Use 'goto out' to help releasing the mutex_lock. >>>> >>>> Fixes: c210063b40ac ("cupful: mediatek: Add opp notification support") >>> cupful ?? >> Sorry for the typo... > Did you write this line by hand or generate it with Git ? There shouldn't be a > typo here. Yes, I write this line by hand. There must be something wrong when copying the subject line. Maybe I should find some tools in git to avoid it in the future. >> By the way, is this patch correct? >> >> I am not sure whether it should return with the mutex_lock. >> But IMO, it should release the lock before return. > Yes, we need to release the lock. OK, I'll fix it in v2. Thanks, Wan Jiabing
On 10-05-22, 16:58, Jiabing Wan wrote: > Yes, I write this line by hand. > There must be something wrong when copying the subject line. > > Maybe I should find some tools in git to avoid it in the future. Add this to ~/.gitconfig [pretty] fixes = Fixes: %h (\"%s\") And then just do: $ git log --pretty=fixes c210063b40ac It shall give you: Fixes: c210063b40ac ("cpufreq: mediatek: Add opp notification support") Fixes: 6a17b3876bc8 ("cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking()") Fixes: ead858bd128d ("cpufreq: mediatek: Move voltage limits to platform data") Fixes: f126fbadce92 ("cpufreq: mediatek: Unregister platform device on exit") Fixes: a3b8d1b12c6b ("cpufreq: mediatek: Fix NULL pointer dereference in mediatek-cpufreq") Fixes: ffa7bdf7f344 ("cpufreq: mediatek: Make sram regulator optional") and other patches as we did git log. OR Else you could do something like this to just see one commit: $ git show -s --pretty=oneline --pretty=fixes c210063b40ac
diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c index 75bf21ddf61f..4c6d53c99d79 100644 --- a/drivers/cpufreq/mediatek-cpufreq.c +++ b/drivers/cpufreq/mediatek-cpufreq.c @@ -196,7 +196,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, if (pre_vproc < 0) { dev_err(cpu_dev, "invalid Vproc value: %d\n", pre_vproc); - return pre_vproc; + ret = pre_vproc; + goto out; } freq_hz = freq_table[index].frequency * 1000; @@ -205,7 +206,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, if (IS_ERR(opp)) { dev_err(cpu_dev, "cpu%d: failed to find OPP for %ld\n", policy->cpu, freq_hz); - return PTR_ERR(opp); + ret = PTR_ERR(opp); + goto out; } vproc = dev_pm_opp_get_voltage(opp); dev_pm_opp_put(opp);
Fix following coccichek error: ./drivers/cpufreq/mediatek-cpufreq.c:199:2-8: preceding lock on line ./drivers/cpufreq/mediatek-cpufreq.c:208:2-8: preceding lock on line mutex_lock is acquired but not released before return. Use 'goto out' to help releasing the mutex_lock. Fixes: c210063b40ac ("cupful: mediatek: Add opp notification support") Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> --- drivers/cpufreq/mediatek-cpufreq.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)