On Mon, Jul 23, 2018 at 9:27 PM, Waiman Long <long...@redhat.com> wrote: > On 07/23/2018 03:16 PM, Peter Zijlstra wrote: >> On Mon, Jul 23, 2018 at 01:49:39PM -0400, Waiman Long wrote: >>> drivers/cpufreq/cpufreq.c | 16 +++++++++++++++- >>> 1 file changed, 15 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c >>> index b0dfd32..9cf02d7 100644 >>> --- a/drivers/cpufreq/cpufreq.c >>> +++ b/drivers/cpufreq/cpufreq.c >>> @@ -922,8 +922,22 @@ static ssize_t store(struct kobject *kobj, struct >>> attribute *attr, >>> struct cpufreq_policy *policy = to_policy(kobj); >>> struct freq_attr *fattr = to_attr(attr); >>> ssize_t ret = -EINVAL; >>> + int retries = 3; >>> >>> - cpus_read_lock(); >>> + /* >>> + * cpus_read_trylock() is used here to work around a circular lock >>> + * dependency problem with respect to the cpufreq_register_driver(). >>> + * With a simple retry loop, the chance of not able to get the >>> + * read lock is extremely small. >>> + */ >>> + while (!cpus_read_trylock()) { >>> + if (retries-- <= 0) >>> + return -EBUSY; >>> + /* >>> + * Sleep for about 50ms and retry again. >>> + */ >>> + msleep(50); >>> + } >> That's atrocious. >> >> > I had thought about just returning an error if the trylock fails as CPU > hotplug rarely happened. I can revert to that simple case if others have > no objection.
Yes, you can return -EBUSY or -EAGAIN right away from here if the cpus_read_trylock() is not successful. There is not much reason for the sysfs operation to continue in that case.