Simplify the error handling branch code in cpufreq_boost_trigger_state().
Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
drivers/cpufreq/cpufreq.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index a4399e5490da..a725747572c9 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2824,18 +2824,13 @@ static int cpufreq_boost_trigger_state(int state)
ret = policy_set_boost(policy, state);
if (ret)
- goto err_reset_state;
+ break;
}
- if (ret)
- goto err_reset_state;
-
cpus_read_unlock();
- return 0;
-
-err_reset_state:
- cpus_read_unlock();
+ if (!ret)
+ return 0;
write_lock_irqsave(&cpufreq_driver_lock, flags);
cpufreq_driver->boost_enabled = !state;
--
2.33.0
On 28-11-25, 17:13, Lifeng Zheng wrote: > Simplify the error handling branch code in cpufreq_boost_trigger_state(). > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > --- > drivers/cpufreq/cpufreq.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index a4399e5490da..a725747572c9 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -2824,18 +2824,13 @@ static int cpufreq_boost_trigger_state(int state) > > ret = policy_set_boost(policy, state); > if (ret) > - goto err_reset_state; > + break; > } > > - if (ret) > - goto err_reset_state; > - > cpus_read_unlock(); > > - return 0; > - > -err_reset_state: > - cpus_read_unlock(); > + if (!ret) Maybe we can make this `if (likely(!ret))` > + return 0; > > write_lock_irqsave(&cpufreq_driver_lock, flags); > cpufreq_driver->boost_enabled = !state; Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh
On 2025/12/1 11:42, Viresh Kumar wrote: > On 28-11-25, 17:13, Lifeng Zheng wrote: >> Simplify the error handling branch code in cpufreq_boost_trigger_state(). >> >> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> >> --- >> drivers/cpufreq/cpufreq.c | 11 +++-------- >> 1 file changed, 3 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >> index a4399e5490da..a725747572c9 100644 >> --- a/drivers/cpufreq/cpufreq.c >> +++ b/drivers/cpufreq/cpufreq.c >> @@ -2824,18 +2824,13 @@ static int cpufreq_boost_trigger_state(int state) >> >> ret = policy_set_boost(policy, state); >> if (ret) >> - goto err_reset_state; >> + break; >> } >> >> - if (ret) >> - goto err_reset_state; >> - >> cpus_read_unlock(); >> >> - return 0; >> - >> -err_reset_state: >> - cpus_read_unlock(); >> + if (!ret) > > Maybe we can make this `if (likely(!ret))` For the platforms which are not boost supported, this will never be matched. Is `likely` OK in this situation? > >> + return 0; >> >> write_lock_irqsave(&cpufreq_driver_lock, flags); >> cpufreq_driver->boost_enabled = !state; > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> >
On 02-12-25, 09:32, zhenglifeng (A) wrote: > On 2025/12/1 11:42, Viresh Kumar wrote: > > On 28-11-25, 17:13, Lifeng Zheng wrote: > >> Simplify the error handling branch code in cpufreq_boost_trigger_state(). > >> > >> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > >> --- > >> drivers/cpufreq/cpufreq.c | 11 +++-------- > >> 1 file changed, 3 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > >> index a4399e5490da..a725747572c9 100644 > >> --- a/drivers/cpufreq/cpufreq.c > >> +++ b/drivers/cpufreq/cpufreq.c > >> @@ -2824,18 +2824,13 @@ static int cpufreq_boost_trigger_state(int state) > >> > >> ret = policy_set_boost(policy, state); > >> if (ret) > >> - goto err_reset_state; > >> + break; > >> } > >> > >> - if (ret) > >> - goto err_reset_state; > >> - > >> cpus_read_unlock(); > >> > >> - return 0; > >> - > >> -err_reset_state: > >> - cpus_read_unlock(); > >> + if (!ret) > > > > Maybe we can make this `if (likely(!ret))` > > For the platforms which are not boost supported, this will never be > matched. Is `likely` OK in this situation? Ideally they won't have a `boost` file in sysfs, and if they have it, we don't really need to optimize the failure case. -- viresh
On 2025/12/2 12:58, Viresh Kumar wrote: > On 02-12-25, 09:32, zhenglifeng (A) wrote: >> On 2025/12/1 11:42, Viresh Kumar wrote: >>> On 28-11-25, 17:13, Lifeng Zheng wrote: >>>> Simplify the error handling branch code in cpufreq_boost_trigger_state(). >>>> >>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> >>>> --- >>>> drivers/cpufreq/cpufreq.c | 11 +++-------- >>>> 1 file changed, 3 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >>>> index a4399e5490da..a725747572c9 100644 >>>> --- a/drivers/cpufreq/cpufreq.c >>>> +++ b/drivers/cpufreq/cpufreq.c >>>> @@ -2824,18 +2824,13 @@ static int cpufreq_boost_trigger_state(int state) >>>> >>>> ret = policy_set_boost(policy, state); >>>> if (ret) >>>> - goto err_reset_state; >>>> + break; >>>> } >>>> >>>> - if (ret) >>>> - goto err_reset_state; >>>> - >>>> cpus_read_unlock(); >>>> >>>> - return 0; >>>> - >>>> -err_reset_state: >>>> - cpus_read_unlock(); >>>> + if (!ret) >>> >>> Maybe we can make this `if (likely(!ret))` >> >> For the platforms which are not boost supported, this will never be >> matched. Is `likely` OK in this situation? > > Ideally they won't have a `boost` file in sysfs, and if they have it, we don't > really need to optimize the failure case. I see. Then I think the `if (ret)` in the loop should be `if (unlikely(ret))` too.
On 02-12-25, 14:24, zhenglifeng (A) wrote: > On 2025/12/2 12:58, Viresh Kumar wrote: > > On 02-12-25, 09:32, zhenglifeng (A) wrote: > >> On 2025/12/1 11:42, Viresh Kumar wrote: > >>> On 28-11-25, 17:13, Lifeng Zheng wrote: > >>>> Simplify the error handling branch code in cpufreq_boost_trigger_state(). > >>>> > >>>> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > >>>> --- > >>>> drivers/cpufreq/cpufreq.c | 11 +++-------- > >>>> 1 file changed, 3 insertions(+), 8 deletions(-) > >>>> > >>>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > >>>> index a4399e5490da..a725747572c9 100644 > >>>> --- a/drivers/cpufreq/cpufreq.c > >>>> +++ b/drivers/cpufreq/cpufreq.c > >>>> @@ -2824,18 +2824,13 @@ static int cpufreq_boost_trigger_state(int state) > >>>> > >>>> ret = policy_set_boost(policy, state); > >>>> if (ret) > >>>> - goto err_reset_state; > >>>> + break; > >>>> } > >>>> > >>>> - if (ret) > >>>> - goto err_reset_state; > >>>> - > >>>> cpus_read_unlock(); > >>>> > >>>> - return 0; > >>>> - > >>>> -err_reset_state: > >>>> - cpus_read_unlock(); > >>>> + if (!ret) > >>> > >>> Maybe we can make this `if (likely(!ret))` > >> > >> For the platforms which are not boost supported, this will never be > >> matched. Is `likely` OK in this situation? > > > > Ideally they won't have a `boost` file in sysfs, and if they have it, we don't > > really need to optimize the failure case. > > I see. Then I think the `if (ret)` in the loop should be > `if (unlikely(ret))` too. That can be done too. -- viresh
© 2016 - 2026 Red Hat, Inc.