Messages in this thread | | | Date | Fri, 13 May 2022 11:43:43 +0530 | From | Viresh Kumar <> | Subject | Re: [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
| |