On Mon, Apr 21, 2014 at 06:47:34PM -0400, Pranith Kumar wrote: > > On Mon, Apr 21, 2014 at 6:03 PM, Paul E. McKenney > <paul...@linux.vnet.ibm.com> wrote: > >> do most of the uses of rcu_dereference_raw() need to be changed to use > >> other > >> dereference functions or are there cases where its usage is valid? > > > > The second call from iwl_op_mode_dvm_stop() -might- be valid. For it > > to be valid, there must be a grace period between the time that the > > field was made inaccessible to readers and the time that iwl_uninit_drv() > > was called. Usually something like synchronize_rcu() waits for the > > needed grace period. > > So there are valid use cases of the rcu_dereference_raw() in scenarios where > we can verify that a grace period has passed. > > Thank you for the info. Mind adding it as a comment as in the patch below? > > add comment for rcu_dereference_raw > > Signed-off-by: Pranith Kumar <bobby.pr...@gmail.com>
If we are going to do that, we should probably have a complete list: 1. The pointer has not yet been made available to readers, for example, the access happens during initialization. 2. A grace period has elapsed since the last time that this pointer was available to readers, for example, this access happens during clean-up processing within an RCU callback function. 3. Code that is shared among multiple flavors of RCU cannot say which flavor of RCU to specify. Given SRCU, there might be an unbounded number to choose from. This is the reason for use of rcu_dereference_raw() in list_for_each_entry_rcu() and similar RCU list-traversal macros. 4. Code that uses a combination of RCU and per-data-structure locking, but where a given lock guards not just the data structure containing it, but a number of subordinate linked structures as well. Code operating on one of the subordinate structures might not know which lock is protecting it. There probably are others, but that is what comes to mind at the moment. Ah, and we should ask that uses of rcu_dereference_raw() be commented. Thanx, Paul > --- > include/linux/rcupdate.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > index 00a7fd6..af40a86 100644 > --- a/include/linux/rcupdate.h > +++ b/include/linux/rcupdate.h > @@ -662,7 +662,13 @@ static inline void rcu_preempt_sleep_check(void) > __rcu_dereference_check((p), rcu_read_lock_sched_held() || (c), \ > __rcu) > > -#define rcu_dereference_raw(p) rcu_dereference_check(p, 1) /*@@@ needed? > @@@*/ > +/* rcu_dereference_raw() - rcu_dereference with no checking > + * @p: The pointer to read, prior to dereferencing > + * > + * Use this to dereference a rcu pointer if you are sure that there exists a > + * grace period between the time this pointer was made inaccessible to > readers > + */ > +#define rcu_dereference_raw(p) rcu_dereference_check(p, 1) > > /* > * The tracing infrastructure traces RCU (we want that), but unfortunately > -- > 1.7.9.5 > > > -- > Pranith > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/