Messages in this thread | | | From | Schspa Shi <> | Date | Fri, 13 May 2022 19:16:01 +0800 | Subject | Re: [PATCH v4 1/2] cpufreq: fix race on cpufreq online |
| |
Viresh Kumar <viresh.kumar@linaro.org> writes:
> 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.
OK, I have uploaded a v5 patch for this. Please review it.
-- Schspa Shi BRs
| |