From: Thomas Renninger <trenn-l3A5Bk7waGM@public.gmane.org>
To: Dave Jones <davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
cpufreq-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
kernel-testers-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>,
"Rafael J. Wysocki" <rjw-KKrjLPT3xs0@public.gmane.org>,
Dave Young
<hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
Pekka Enberg <penberg-bbCR+/B0CizivPeTLB3BmA@public.gmane.org>,
Mathieu Desnoyers
<mathieu.desnoyers-scC8bbJcJLCw5LPnMra/2Q@public.gmane.org>,
Thomas Renninger <trenn-l3A5Bk7waGM@public.gmane.org>,
Venkatesh Pallipadi
<venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: cpufreq cleanups - .30 vs .31
Date: Mon, 6 Jul 2009 13:18:18 +0200 [thread overview]
Message-ID: <200907061318.20839.trenn@suse.de> (raw)
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
next reply other threads:[~2009-07-06 11:18 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-06 11:18 Thomas Renninger [this message]
[not found] ` <200907061318.20839.trenn-l3A5Bk7waGM@public.gmane.org>
2009-07-06 13:45 ` cpufreq cleanups - .30 vs .31 Mathieu Desnoyers
2009-07-06 17:20 ` Pallipadi, Venkatesh
2009-07-07 1:51 ` Dave Jones
[not found] ` <20090707015115.GB5310-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2009-07-27 14:25 ` Mathieu Desnoyers
2009-07-27 16:31 ` Pallipadi, Venkatesh
[not found] ` <1248712266.11545.8824.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2009-07-27 16:37 ` Mathieu Desnoyers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200907061318.20839.trenn@suse.de \
--to=trenn-l3a5bk7wagm@public.gmane.org \
--cc=cpufreq-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=kernel-testers-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mathieu.desnoyers-scC8bbJcJLCw5LPnMra/2Q@public.gmane.org \
--cc=mingo-X9Un+BFzKDI@public.gmane.org \
--cc=penberg-bbCR+/B0CizivPeTLB3BmA@public.gmane.org \
--cc=rjw-KKrjLPT3xs0@public.gmane.org \
--cc=venkatesh.pallipadi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).