lkml.org 
[lkml]   [2022]   [May]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 1/2] cpufreq: fix race on cpufreq online
On 13-05-22, 14:06, Schspa Shi wrote:
> Viresh Kumar <viresh.kumar@linaro.org> writes:
> > On 12-05-22, 21:52, Schspa Shi wrote:
> >> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> >> index 80f535cc8a75..35dffd738580 100644
> >> --- a/drivers/cpufreq/cpufreq.c
> >> +++ b/drivers/cpufreq/cpufreq.c
> >> @@ -953,7 +953,10 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
> >> return -EIO;
> >>
> >> down_read(&policy->rwsem);
> >> - ret = fattr->show(policy, buf);
> >> + if (unlikely(policy_is_inactive(policy)))
> >> + ret = -EBUSY;
> >> + else
> >> + ret = fattr->show(policy, buf);
> >
> > I like it the way I have done earlier, initialize ret to -EBUSY and
> > get rid of the else part and call show/store in if itself. Same for
> > below.
> >
>
> I add a unlikely here, to avoid branch prediction failed.

I am not asking you to drop it, I also added the unlikely within the
implementation of policy_is_inactive() then. It can be written as:

if (likely(!policy_is_inactive(policy)))
ret = fattr->show(policy, buf);

> And move the
> to the fail path to avoid a register assignment to -EBUSY.

We don't care about such assignments for performance to be honest.
This makes the code smaller by few lines, that's enough.

--
viresh

\
 
 \ /
  Last update: 2022-05-13 08:15    [W:0.045 / U:0.668 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site