Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Wed, 19 Oct 2022 13:47:39 +0200 | Subject | Re: [PATCH 1/1] cpufreq: Fix show()/store() issue for hotplugging offline CPU |
| |
On Wed, Oct 19, 2022 at 10:40 AM Guanjun <guanjun@linux.alibaba.com> wrote: > > From: Zelin Deng <zelin.deng@linux.alibaba.com> > > After brought one CPU offline, lscpu returned failure: > > lscpu: cannot read /sys/devices/system/cpu/cpu64/cpufreq/cpuinfo_max_freq: Device or resource busy > > which had blocked all outputs of lscpu.
OK, so the policy->cpus mask is empty and -EBUSY is returned.
What's wrong?
> This is not the case mentioned in commit d4627a287e251, as the policy > had been created successfully but is inactive due to CPU gets offline.
Yes, that's when policy_is_inactive(policy) returns "true" IIUC.
> To fix this issue, just add an addtional check whether CPU is online or > not.
Which is racy.
Please explain the problem in the first place.
> Fixes: d4627a287e251 ("cpufreq: Abort show()/store() for half-initialized policies") > Signed-off-by: Zelin Deng <zelin.deng@linux.alibaba.com> > Signed-off-by: Guanjun <guanjun@linux.alibaba.com> > --- > drivers/cpufreq/cpufreq.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 69b3d61852ac..aa238ba7d2fe 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -956,8 +956,12 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf) > return -EIO; > > down_read(&policy->rwsem); > - if (likely(!policy_is_inactive(policy))) > - ret = fattr->show(policy, buf); > + if (unlikely(policy_is_inactive(policy) && cpu_online(policy->cpu))) > + goto err; > + > + ret = fattr->show(policy, buf); > + > +err: > up_read(&policy->rwsem); > > return ret; > @@ -974,8 +978,12 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr, > return -EIO; > > down_write(&policy->rwsem); > - if (likely(!policy_is_inactive(policy))) > - ret = fattr->store(policy, buf, count); > + if (unlikely(policy_is_inactive(policy) && cpu_online(policy->cpu))) > + goto err; > + > + ret = fattr->store(policy, buf, count); > + > +err: > up_write(&policy->rwsem); > > return ret; > -- > 2.32.0.GIT >
| |