cpufreq cleanups - .30 vs .31

[Thomas Renninger]

Hi Dave,

this is about Venki's and Mathieu's recently sent cleanups.
I'd like to summarize this to help finding a solution:

IMO Venki's approach (making .governor() always be called with
rwsem held) is the cleaner one and this should be the way to
go for .31 and future. This better separates locking responsibilities
between cpufreq core and governors and brings back "design" into this.

One could argue that for .30 Mathieu's is better, because less
intrusive. It's up to Dave in the end, but:
[patch 2.6.30 1/4] remove rwsem lock from CPUFREQ_GOV_STOP call (second call 
site)
should not be the way to go for .31 and I'd vote for Venki's
approach concerning locking .governor() against multiple calls (done by
rwsem) and governor() vs do_dbs_timer calls (governor's job with a governor
specific sem).

So if not find too intrusive, I'd say:
Venkatesh's whole series of:
[patch 0/4] Take care of cpufreq lockdep issues (take 2)
should be seen in .31.

Depending on how intrusive this is seen, Venki's first patch:
[patch 1/4] cpufreq: Eliminate the recent lockdep warnings in cpufreq
should then go to .30 (after still waiting a bit?)
or Mathieu's approach (I'd vote for Venki's to be consistent for .30 and .31).

The one patch from Mathieu:
[patch 2.6.30 2/4] CPUFREQ: fix (utter) cpufreq_add_dev mess
is a separate, general cleanup which should show up in .31.



I still have two patch specific questions:
about Mathieu's (it's a minor issue in the error path):
[patch 2.6.30 2/4] CPUFREQ: fix (utter) cpufreq_add_dev mess

+                       if (lock_policy_rwsem_write(cpu) < 0) {
+                               /* Should not go through policy unlock path */
+                               if (cpufreq_driver->exit)
+                                       cpufreq_driver->exit(policy);
+                               ret = -EBUSY;
+                               cpufreq_cpu_put(managed_policy);
Shouldn't:
 cpufreq_cpu_put(managed_policy);
be called before:
 cpufreq_driver->exit(policy);
Just in case the driver itself wants to grab the policy of the
managed cpu?


about Venki's:
[patch 3/4] cpufreq: Cleanup locking in ondemand governor
Isn't it possible to use only one mutex(timer_mutex) to protect do_dbs_timer 
against governor start, stop, limit?
Then dbs_mutex would only be used to protect against concurrent sysfs access
and can be thrown away as soon as ondemand only provides global sysfs files,
not per cpu ones.

Hmm, maybe this should just go in? It eases up things, but it's still hard
to follow up each detail. Fixes/enhancements can still be put on top
for .31.

  Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/