On 08/05/2014 06:06 PM, Saravana Kannan wrote:

quoted

On 08/05/2014 03:53 AM, Viresh Kumar wrote:

quoted

On 5 August 2014 16:17, Prarit Bhargava [off-list ref] wrote:

quoted

Nope, not a stupid question.  After reproducing (finally!) yesterday I've been
wondering the same thing.

Good to know that :)

quoted

I've been looking into *exactly* this.  On any platform where
cpu_weight(affected_cpus) == 1 for a particular cpu this lockdep trace should
happen.

quoted

That's what I'm wondering too.  I'm going to instrument the code to find out
this morning.  I'm wondering if this comes down to a lockdep class issue
(perhaps lockdep puts globally defined locks like cpufreq_global_kobject in a
different class?).

Maybe, I tried this Hack to make this somewhat similar to the other case
on my platform with just two CPUs:

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6f02485..6b4abac 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c

@@ -98,7 +98,7 @@ static DEFINE_MUTEX(cpufreq_governor_mutex);

   bool have_governor_per_policy(void)
   {
-       return !!(cpufreq_driver->flags & CPUFREQ_HAVE_GOVERNOR_PER_POLICY);
+       return !(cpufreq_driver->flags & CPUFREQ_HAVE_GOVERNOR_PER_POLICY);
   }
   EXPORT_SYMBOL_GPL(have_governor_per_policy);

This should result in something similar to setting that per-policy-governor
flag (Actually I could have done that too :)), and I couldn't see that crash :(

That needs more investigation now, probably we can get some champ of
sysfs stuff like Tejun/Greg into discussion now..

Stephen and I looked into this. This is not a sysfs framework difference. The
reason we don't have this issue when we use global tunables is because we add
the attribute group to the cpufreq_global_kobject and that kobject doesn't have
a kobj_type ops similar to the per policy kobject. So, read/write to those
attributes do NOT go through the generic show/store ops that wrap every other
cpufreq framework attribute read/writes.

So, none of those read/write do any kind of locking. They don't race with
POLICY_EXIT (because we remove the sysfs group first thing in POLICY_EXIT) but
might still race with START/STOPs (not sure, haven't looked closely yet).

For example, writing to sampling_rate of ondemand governor might cause a race in
update_sampling_rate(). It could race and happen between a STOP and POLICY_EXIT
(triggered by hotplug, gov change, etc).

So, this might be a completely separate bug that needs fixing when we don't use
per policy govs.

Yeah, the show_one & store_one macros don't have any locking in them :/.

Okay ... at least that isn't the issue.  I spent 1/2 the day trying to figure
out why

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index fa11a7d..6297c76 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c

@@ -745,12 +745,14 @@ static struct attribute *default_attrs[] = {
  #define to_policy(k) container_of(k, struct cpufreq_policy, kobj)
  #define to_attr(a) container_of(a, struct freq_attr, attr)

+/* PRARIT - in the CPUFREQ_HAVE_GOVERNOR_PER_POLICY, this is used */
  static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf)
  {
         struct cpufreq_policy *policy = to_policy(kobj);
         struct freq_attr *fattr = to_attr(attr);
         ssize_t ret;

+       printk("%s: kobject %p\n", __FUNCTION__, kobj);
         if (!down_read_trylock(&cpufreq_rwsem))
                 return -EINVAL;

wasn't printing the kobject line when acpi-cpufreq didn't have the
CPUFREQ_HAVE_GOVERNOR_PER_POLICY flag.  And I agree ... it is a bug.