On Tue, Jan 23, 2018 at 10:29:06PM +0000, Bart Van Assche wrote:
> On Tue, 2018-01-23 at 14:03 -0800, Paul E. McKenney wrote:
> > On Tue, Jan 23, 2018 at 04:04:17PM +0000, Bart Van Assche wrote:
> > > It would be great if Paul could provide feedback. The comment in
> > > include/linux/rcupdate.h seems to contradict the following paragraph from
> > > Documentation/RCU/Design/Requirements/Requirements.html for statically
> > > allocated objects:
> > >
> > > The corresponding <tt>rcu_head</tt> structures that are
> > > dynamically allocated are automatically tracked, but
> > > <tt>rcu_head</tt> structures allocated on the stack
> > > must be initialized with <tt>init_rcu_head_on_stack()</tt>
> > > and cleaned up with <tt>destroy_rcu_head_on_stack()</tt>.
> > > Similarly, statically allocated non-stack <tt>rcu_head</tt>
> > > structures must be initialized with <tt>init_rcu_head()</tt>
> > > and cleaned up with <tt>destroy_rcu_head()</tt>.
> >
> > And this is the intent. What breaks when you do that?
>
> Hello Paul,
>
> I inserted a init_rcu_head() call in the ib_srpt driver for a dynamically
> allocated object and that triggers a linker error with
> CONFIG_DEBUG_OBJECTS_RCU_HEAD=y. I just verified that the ib_srpt driver still
> works fine if the init_rcu_head() calls are removed from that driver. However,
> the following part of the comment in include/linux/rcupdate.h above
> init_rcu_head() seems confusing to me: "rcu_head structures allocated
> dynamically in the heap or defined statically don't need any initialization."
> Should that comment perhaps be updated such that it matches
> Documentation/RCU/Design/Requirements/Requirements.html?
Good catch!!! How about the patch below?
Thanx, Paul
------------------------------------------------------------------------
commit 2ccdd2a8f0ba33b71dab24da408fda79ed2ee063
Author: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
Date: Tue Jan 23 14:48:33 2018 -0800
rcu: Fix init_rcu_head() comment.
The current (and implicit) comment header for init_rcu_head() and
destroy_rcu_head() incorrectly says that they are not needed for
statically allocated rcu_head structures. This commit therefore
fixes this comment.
Reported-by: Bart Van Assche <Bart.VanAssche@xxxxxxx>
Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 043d04784675..36360d07f25b 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -214,10 +214,12 @@ do { \
#endif
/*
- * init_rcu_head_on_stack()/destroy_rcu_head_on_stack() are needed for dynamic
- * initialization and destruction of rcu_head on the stack. rcu_head structures
- * allocated dynamically in the heap or defined statically don't need any
- * initialization.
+ * The init_rcu_head_on_stack() and destroy_rcu_head_on_stack() calls
+ * are needed for dynamic initialization and destruction of rcu_head
+ * on the stack, and init_rcu_head()/destroy_rcu_head() are needed for
+ * dynamic initialization and destruction of statically allocated rcu_head
+ * structures. However, rcu_head structures allocated dynamically in the
+ * heap don't need any initialization.
*/
#ifdef CONFIG_DEBUG_OBJECTS_RCU_HEAD
void init_rcu_head(struct rcu_head *head);
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
