On Mon, Feb 11, 2013 at 08:36:17PM +0100, Rafael J. Wysocki wrote: > On Monday, February 11, 2013 05:13:30 PM Nathan Zimmer wrote: > > There are some spots that I need to give a much deeper review, cpufreq_register_driver for example. > > > > But I believe > > > @@ -196,7 +195,7 @@ static void __cpufreq_cpu_put(struct cpufreq_policy *data, bool sysfs) > > > { > > > if (!sysfs) > > > kobject_put(&data->kobj); > > > - module_put(cpufreq_driver->owner); > > > + module_put(rcu_dereference(cpufreq_driver)->owner); > > > } > > would be ok. In the documentation whatisRCU.txt they give a very similar example. > > Well, the very same document states the following: > > Note that the value returned by rcu_dereference() is valid > only within the enclosing RCU read-side critical section. Ah, there is a code sample in that document showing a bug. I added comments to the code sample making it clear even to someone skimming the document that the code is buggy. Thanx, Paul ------------------------------------------------------------------------ rcu: Make bugginess of code sample more evident One of the code samples in whatisRCU.txt shows a bug, but someone scanning the document quickly might mistake it for a valid use of RCU. Add some screaming comments to help keep speed-readers on track. Reported-by: Nathan Zimmer <nzimmer@xxxxxxx> Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> diff --git a/Documentation/RCU/whatisRCU.txt b/Documentation/RCU/whatisRCU.txt index 0cc7820..10df0b8 100644 --- a/Documentation/RCU/whatisRCU.txt +++ b/Documentation/RCU/whatisRCU.txt @@ -265,9 +265,9 @@ rcu_dereference() rcu_read_lock(); p = rcu_dereference(head.next); rcu_read_unlock(); - x = p->address; + x = p->address; /* BUG!!! */ rcu_read_lock(); - y = p->data; + y = p->data; /* BUG!!! */ rcu_read_unlock(); Holding a reference from one RCU read-side critical section -- To unsubscribe from this list: send the line "unsubscribe cpufreq" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html