Messages in this thread |  | | Date | Thu, 16 Nov 2006 18:33:16 -0800 | From | "Paul E. McKenney" <> | Subject | Re: [patch] cpufreq: mark cpufreq_tsc() as core_initcall_sync |
| |
On Thu, Nov 16, 2006 at 01:47:48PM -0800, Linus Torvalds wrote: > > > On Thu, 16 Nov 2006, Thomas Gleixner wrote: > > > > Here is the i386/sparc fixup > > Gag me with a volvo.
No can do -- my wife drives a Ford and my car is a bicycle.
> This is disgusting, but I would actually prefer the following version over > the patches I've seen, because > > - it doesn't end up having any architecture-specific parts > > - it doesn't use the new "xxx_sync()" thing that I'm not even sure we > should be using. > > - it makes it clear that this should be fixed, preferably by just having > some way to initialize SRCU structs staticalyl. If we get that, the fix > is to just replace the horrible "initialize by hand" with a static > initializer once and for all. > > Hmm? > > Totally untested, but it compiles and it _looks_ sane. The overhead of the > function call should be minimal, once things are initialized. > > Paul, it would be _really_ nice to have some way to just initialize that > SRCU thing statically. This kind of crud is just crazy.
Static initialization is a bit of a tarpit for SRCU. Before this week, I would have protested bitterly over the overhead of a dynamic runtime check, but Jens is running into another issue that looks to require a bit more read-side overhead as well (synchronize_srcu() is too expensive for his situation). Now if I can get one of the local weak-memory model torture-chamber boxes to deal with a recent kernel...
Hardware whines aside, shouldn't be too hard. Will put something together...
Thanx, Paul
> Comments? > > Linus > > ---- > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 86e69b7..02326b2 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -52,14 +52,39 @@ static void handle_update(void *data); > * The mutex locks both lists. > */ > static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list); > -static struct srcu_notifier_head cpufreq_transition_notifier_list; > > -static int __init init_cpufreq_transition_notifier_list(void) > +/* > + * This is horribly horribly ugly. > + * > + * We really want to initialize the transition notifier list > + * statically and just once, but there is no static way to > + * initialize a srcu lock, so we instead make up all this nasty > + * infrastructure to make sure it's initialized when we use it. > + * > + * Bleaargh. > + */ > +static struct srcu_notifier_head *cpufreq_transition_notifier_list(void) > { > - srcu_init_notifier_head(&cpufreq_transition_notifier_list); > - return 0; > + static struct srcu_notifier_head *initialized; > + struct srcu_notifier_head *ret; > + > + ret = initialized; > + if (!ret) { > + static DEFINE_MUTEX(init_lock); > + > + mutex_lock(&init_lock); > + ret = initialized; > + if (!ret) { > + static struct srcu_notifier_head list_head; > + ret = &list_head; > + srcu_init_notifier_head(ret); > + smp_wmb(); > + initialized = ret; > + } > + mutex_unlock(&init_lock); > + } > + return ret; > } > -core_initcall(init_cpufreq_transition_notifier_list); > > static LIST_HEAD(cpufreq_governor_list); > static DEFINE_MUTEX (cpufreq_governor_mutex); > @@ -268,14 +293,14 @@ void cpufreq_notify_transition(struct cp > freqs->old = policy->cur; > } > } > - srcu_notifier_call_chain(&cpufreq_transition_notifier_list, > + srcu_notifier_call_chain(cpufreq_transition_notifier_list(), > CPUFREQ_PRECHANGE, freqs); > adjust_jiffies(CPUFREQ_PRECHANGE, freqs); > break; > > case CPUFREQ_POSTCHANGE: > adjust_jiffies(CPUFREQ_POSTCHANGE, freqs); > - srcu_notifier_call_chain(&cpufreq_transition_notifier_list, > + srcu_notifier_call_chain(cpufreq_transition_notifier_list(), > CPUFREQ_POSTCHANGE, freqs); > if (likely(policy) && likely(policy->cpu == freqs->cpu)) > policy->cur = freqs->new; > @@ -1055,7 +1080,7 @@ static int cpufreq_suspend(struct sys_de > freqs.old = cpu_policy->cur; > freqs.new = cur_freq; > > - srcu_notifier_call_chain(&cpufreq_transition_notifier_list, > + srcu_notifier_call_chain(cpufreq_transition_notifier_list(), > CPUFREQ_SUSPENDCHANGE, &freqs); > adjust_jiffies(CPUFREQ_SUSPENDCHANGE, &freqs); > > @@ -1137,7 +1162,7 @@ static int cpufreq_resume(struct sys_dev > freqs.new = cur_freq; > > srcu_notifier_call_chain( > - &cpufreq_transition_notifier_list, > + cpufreq_transition_notifier_list(), > CPUFREQ_RESUMECHANGE, &freqs); > adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs); > > @@ -1183,7 +1208,7 @@ int cpufreq_register_notifier(struct not > switch (list) { > case CPUFREQ_TRANSITION_NOTIFIER: > ret = srcu_notifier_chain_register( > - &cpufreq_transition_notifier_list, nb); > + cpufreq_transition_notifier_list(), nb); > break; > case CPUFREQ_POLICY_NOTIFIER: > ret = blocking_notifier_chain_register( > @@ -1215,7 +1240,7 @@ int cpufreq_unregister_notifier(struct n > switch (list) { > case CPUFREQ_TRANSITION_NOTIFIER: > ret = srcu_notifier_chain_unregister( > - &cpufreq_transition_notifier_list, nb); > + cpufreq_transition_notifier_list(), nb); > break; > case CPUFREQ_POLICY_NOTIFIER: > ret = blocking_notifier_chain_unregister( - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |