public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
@ 2018-01-23 11:50 Leon Romanovsky
       [not found] ` <20180123115013.12213-1-leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2018-01-23 11:50 UTC (permalink / raw)
  To: Bart Van Assche, Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, linux-rdma-u79uwXL29TY76Z2rM5mHXA

Combination of CONFIG_DEBUG_OBJECTS_RCU_HEAD=y and
CONFIG_INFINIBAND_SRPT=m produces the following build error.

ERROR: "init_rcu_head" [drivers/infiniband/ulp/srpt/ib_srpt.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
make: *** [Makefile:1216: modules] Error 2

The reason to it that init_rcu_head() is not exported and not supposed
to be used in modules. The correct function to use is init_rcu_head_on_stack().

Fixes: 795bc112cd5a ("IB/srpt: Make it safe to use RCU for srpt_device.rch_list")
Fixes: a11253142e6d ("IB/srpt: Rework multi-channel support")
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index bf37816a1b12..65bbc3dbf0fd 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1950,7 +1950,7 @@ static struct srpt_nexus *srpt_get_nexus(struct srpt_port *sport,
 			nexus = ERR_PTR(-ENOMEM);
 			break;
 		}
-		init_rcu_head(&tmp_nexus->rcu);
+		init_rcu_head_on_stack(&tmp_nexus->rcu);
 		INIT_LIST_HEAD(&tmp_nexus->ch_list);
 		memcpy(tmp_nexus->i_port_id, i_port_id, 16);
 		memcpy(tmp_nexus->t_port_id, t_port_id, 16);
@@ -2110,7 +2110,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
 		goto reject;
 	}

-	init_rcu_head(&ch->rcu);
+	init_rcu_head_on_stack(&ch->rcu);
 	kref_init(&ch->kref);
 	ch->pkey = be16_to_cpu(pkey);
 	ch->nexus = nexus;
--
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found] ` <20180123115013.12213-1-leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
@ 2018-01-23 15:21   ` Jason Gunthorpe
       [not found]     ` <20180123152142.GB30619-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Jason Gunthorpe @ 2018-01-23 15:21 UTC (permalink / raw)
  To: Leon Romanovsky, Paul E. McKenney
  Cc: Bart Van Assche, Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Tue, Jan 23, 2018 at 01:50:13PM +0200, Leon Romanovsky wrote:
> Combination of CONFIG_DEBUG_OBJECTS_RCU_HEAD=y and
> CONFIG_INFINIBAND_SRPT=m produces the following build error.
> 
> ERROR: "init_rcu_head" [drivers/infiniband/ulp/srpt/ib_srpt.ko] undefined!
> make[1]: *** [scripts/Makefile.modpost:92: __modpost] Error 1
> make: *** [Makefile:1216: modules] Error 2
> 
> The reason to it that init_rcu_head() is not exported and not supposed
> to be used in modules. The correct function to use is init_rcu_head_on_stack().
> 
> Fixes: 795bc112cd5a ("IB/srpt: Make it safe to use RCU for srpt_device.rch_list")
> Fixes: a11253142e6d ("IB/srpt: Rework multi-channel support")
> Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>  drivers/infiniband/ulp/srpt/ib_srpt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index bf37816a1b12..65bbc3dbf0fd 100644
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -1950,7 +1950,7 @@ static struct srpt_nexus *srpt_get_nexus(struct srpt_port *sport,
>  			nexus = ERR_PTR(-ENOMEM);
>  			break;
>  		}
> -		init_rcu_head(&tmp_nexus->rcu);
> +		init_rcu_head_on_stack(&tmp_nexus->rcu);

The comments for init_rcu_head_on_stack say it should only be used for
stack allocations and tmp_nexus is not a stack allocation.

Removing the init_rcu_head entirely seems like the right thing to do,
and maybe that function is poorly named?

/*
 * 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 commit that introduced init_rcu_head is
546a9d8519ed137b2804a3f5a3659003039dd49c which suggests it exists only
to help the slab allocators, although curiously the allocators never
call it these days..

Maybe Paul can confirm?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]     ` <20180123152142.GB30619-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
@ 2018-01-23 16:04       ` Bart Van Assche
       [not found]         ` <1516723456.3339.3.camel-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Van Assche @ 2018-01-23 16:04 UTC (permalink / raw)
  To: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1429 bytes --]

On Tue, 2018-01-23 at 08:21 -0700, Jason Gunthorpe wrote:
> /*
>  * 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 commit that introduced init_rcu_head is
> 546a9d8519ed137b2804a3f5a3659003039dd49c which suggests it exists only
> to help the slab allocators, although curiously the allocators never
> call it these days..
> 
> Maybe Paul can confirm?

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>.

Bart.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]         ` <1516723456.3339.3.camel-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-23 18:57           ` Leon Romanovsky
       [not found]             ` <20180123185732.GA1393-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
  2018-01-23 22:03           ` Paul E. McKenney
  1 sibling, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2018-01-23 18:57 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

[-- Attachment #1: Type: text/plain, Size: 1636 bytes --]

On Tue, Jan 23, 2018 at 04:04:17PM +0000, Bart Van Assche wrote:
> On Tue, 2018-01-23 at 08:21 -0700, Jason Gunthorpe wrote:
> > /*
> >  * 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 commit that introduced init_rcu_head is
> > 546a9d8519ed137b2804a3f5a3659003039dd49c which suggests it exists only
> > to help the slab allocators, although curiously the allocators never
> > call it these days..
> >
> > Maybe Paul can confirm?
>
> 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>.

Right, I missed it.

So how are we going to solve that rdma/for-next doesn't compile?

Thanks

>
> Bart.N?????r??y????b?X??ǧv?^?)޺{.n?+????{??ٚ?{ay?\x1dʇڙ?,j\a??f???h???z?\x1e?w???\f???j:+v???w?j?m????\a????zZ+?????ݢj"??!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]         ` <1516723456.3339.3.camel-Sjgp3cTcYWE@public.gmane.org>
  2018-01-23 18:57           ` Leon Romanovsky
@ 2018-01-23 22:03           ` Paul E. McKenney
       [not found]             ` <20180123220337.GE3741-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  1 sibling, 1 reply; 15+ messages in thread
From: Paul E. McKenney @ 2018-01-23 22:03 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

On Tue, Jan 23, 2018 at 04:04:17PM +0000, Bart Van Assche wrote:
> On Tue, 2018-01-23 at 08:21 -0700, Jason Gunthorpe wrote:
> > /*
> >  * 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 commit that introduced init_rcu_head is
> > 546a9d8519ed137b2804a3f5a3659003039dd49c which suggests it exists only
> > to help the slab allocators, although curiously the allocators never
> > call it these days..
> > 
> > Maybe Paul can confirm?
> 
> 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?

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]             ` <20180123220337.GE3741-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
@ 2018-01-23 22:29               ` Bart Van Assche
       [not found]                 ` <1516746545.3339.46.camel-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Van Assche @ 2018-01-23 22:29 UTC (permalink / raw)
  To: paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1738 bytes --]

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?

Thanks,

Bart.N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]             ` <20180123185732.GA1393-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2018-01-23 22:30               ` Bart Van Assche
       [not found]                 ` <1516746607.3339.47.camel-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Van Assche @ 2018-01-23 22:30 UTC (permalink / raw)
  To: leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

On Tue, 2018-01-23 at 20:57 +0200, Leon Romanovsky wrote:
> So how are we going to solve that rdma/for-next doesn't compile?

How about the following?

[PATCH] IB/srpt: Fix CONFIG_DEBUG_OBJECTS_RCU_HEAD=y build

Avoid that the kernel build fails as follows if RCU debugging is
enabled:

ERROR: "init_rcu_head" [drivers/infiniband/ulp/srpt/ib_srpt.ko] undefined!

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 5ccc75c389e2..a78a79791950 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1977,7 +1977,6 @@ static struct srpt_nexus *srpt_get_nexus(struct srpt_port *sport,
 			nexus = ERR_PTR(-ENOMEM);
 			break;
 		}
-		init_rcu_head(&tmp_nexus->rcu);
 		INIT_LIST_HEAD(&tmp_nexus->ch_list);
 		memcpy(tmp_nexus->i_port_id, i_port_id, 16);
 		memcpy(tmp_nexus->t_port_id, t_port_id, 16);
@@ -2147,7 +2146,6 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,
 		goto reject;
 	}
 
-	init_rcu_head(&ch->rcu);
 	kref_init(&ch->kref);
 	ch->pkey = be16_to_cpu(pkey);
 	ch->nexus = nexus;

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]                 ` <1516746545.3339.46.camel-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-23 22:50                   ` Paul E. McKenney
       [not found]                     ` <20180123225054.GK3741-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Paul E. McKenney @ 2018-01-23 22:50 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

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-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
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-Sjgp3cTcYWE@public.gmane.org>
    Signed-off-by: Paul E. McKenney <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]                     ` <20180123225054.GK3741-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
@ 2018-01-23 22:59                       ` Bart Van Assche
       [not found]                         ` <1516748369.3339.59.camel-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Van Assche @ 2018-01-23 22:59 UTC (permalink / raw)
  To: paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

On Tue, 2018-01-23 at 14:50 -0800, Paul E. McKenney wrote:
> 
>     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@wdc.com>
>     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> 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);

Thanks!

Feel free to add:

Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]                         ` <1516748369.3339.59.camel-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-24  6:43                           ` Leon Romanovsky
       [not found]                             ` <20180124064337.GB1393-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Leon Romanovsky @ 2018-01-24  6:43 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
	jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

On Tue, Jan 23, 2018 at 10:59:30PM +0000, Bart Van Assche wrote:
> On Tue, 2018-01-23 at 14:50 -0800, Paul E. McKenney wrote:
> >
> >     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-Sjgp3cTcYWE@public.gmane.org>
> >     Signed-off-by: Paul E. McKenney <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> >
> > 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);
>
> Thanks!
>
> Feel free to add:
>
> Reviewed-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
>
>

Paul,

Are you going to send it through your tree?

Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]                 ` <1516746607.3339.47.camel-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-24  6:44                   ` Leon Romanovsky
  2018-01-24 22:36                   ` Doug Ledford
  1 sibling, 0 replies; 15+ messages in thread
From: Leon Romanovsky @ 2018-01-24  6:44 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]

On Tue, Jan 23, 2018 at 10:30:08PM +0000, Bart Van Assche wrote:
> On Tue, 2018-01-23 at 20:57 +0200, Leon Romanovsky wrote:
> > So how are we going to solve that rdma/for-next doesn't compile?
>
> How about the following?
>
> [PATCH] IB/srpt: Fix CONFIG_DEBUG_OBJECTS_RCU_HEAD=y build
>
> Avoid that the kernel build fails as follows if RCU debugging is
> enabled:
>
> ERROR: "init_rcu_head" [drivers/infiniband/ulp/srpt/ib_srpt.ko] undefined!
>
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index 5ccc75c389e2..a78a79791950 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -1977,7 +1977,6 @@ static struct srpt_nexus *srpt_get_nexus(struct srpt_port *sport,
>  			nexus = ERR_PTR(-ENOMEM);
>  			break;
>  		}
> -		init_rcu_head(&tmp_nexus->rcu);
>  		INIT_LIST_HEAD(&tmp_nexus->ch_list);
>  		memcpy(tmp_nexus->i_port_id, i_port_id, 16);
>  		memcpy(tmp_nexus->t_port_id, t_port_id, 16);
> @@ -2147,7 +2146,6 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,
>  		goto reject;
>  	}
>
> -	init_rcu_head(&ch->rcu);
>  	kref_init(&ch->kref);
>  	ch->pkey = be16_to_cpu(pkey);
>  	ch->nexus = nexus;

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]                             ` <20180124064337.GB1393-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
@ 2018-01-24 21:51                               ` Paul E. McKenney
  0 siblings, 0 replies; 15+ messages in thread
From: Paul E. McKenney @ 2018-01-24 21:51 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Bart Van Assche, jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org

On Wed, Jan 24, 2018 at 08:43:37AM +0200, Leon Romanovsky wrote:
> On Tue, Jan 23, 2018 at 10:59:30PM +0000, Bart Van Assche wrote:
> > On Tue, 2018-01-23 at 14:50 -0800, Paul E. McKenney wrote:
> > >
> > >     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-Sjgp3cTcYWE@public.gmane.org>
> > >     Signed-off-by: Paul E. McKenney <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > >
> > > 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);
> >
> > Thanks!
> >
> > Feel free to add:
> >
> > Reviewed-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
> 
> Paul,
> 
> Are you going to send it through your tree?

It is currently queued for v4.17 -- didn't seem horribly urgent.

> Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Applied, thank you!

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]                 ` <1516746607.3339.47.camel-Sjgp3cTcYWE@public.gmane.org>
  2018-01-24  6:44                   ` Leon Romanovsky
@ 2018-01-24 22:36                   ` Doug Ledford
       [not found]                     ` <1516833393.27592.108.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  1 sibling, 1 reply; 15+ messages in thread
From: Doug Ledford @ 2018-01-24 22:36 UTC (permalink / raw)
  To: Bart Van Assche, leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

On Tue, 2018-01-23 at 22:30 +0000, Bart Van Assche wrote:
> On Tue, 2018-01-23 at 20:57 +0200, Leon Romanovsky wrote:
> > So how are we going to solve that rdma/for-next doesn't compile?
> 
> How about the following?
> 
> [PATCH] IB/srpt: Fix CONFIG_DEBUG_OBJECTS_RCU_HEAD=y build
> 
> Avoid that the kernel build fails as follows if RCU debugging is
> enabled:
> 
> ERROR: "init_rcu_head" [drivers/infiniband/ulp/srpt/ib_srpt.ko] undefined!
> 
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index 5ccc75c389e2..a78a79791950 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -1977,7 +1977,6 @@ static struct srpt_nexus *srpt_get_nexus(struct srpt_port *sport,
>  			nexus = ERR_PTR(-ENOMEM);
>  			break;
>  		}
> -		init_rcu_head(&tmp_nexus->rcu);
>  		INIT_LIST_HEAD(&tmp_nexus->ch_list);
>  		memcpy(tmp_nexus->i_port_id, i_port_id, 16);
>  		memcpy(tmp_nexus->t_port_id, t_port_id, 16);
> @@ -2147,7 +2146,6 @@ static int srpt_cm_req_recv(struct srpt_device *const sdev,
>  		goto reject;
>  	}
>  
> -	init_rcu_head(&ch->rcu);
>  	kref_init(&ch->kref);
>  	ch->pkey = be16_to_cpu(pkey);
>  	ch->nexus = nexus;

Hi Bart,

Can you please resend this as an official patch with SOB so we can pull
this in?  Thanks.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]                     ` <1516833393.27592.108.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2018-01-24 22:40                       ` Bart Van Assche
       [not found]                         ` <1516833624.3987.19.camel-Sjgp3cTcYWE@public.gmane.org>
  0 siblings, 1 reply; 15+ messages in thread
From: Bart Van Assche @ 2018-01-24 22:40 UTC (permalink / raw)
  To: leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On Wed, 2018-01-24 at 17:36 -0500, Doug Ledford wrote:
> Can you please resend this as an official patch with SOB so we can pull
> this in?  Thanks.

Hello Doug,

Can you have a look at this patch and see whether this needs any further
adjustments: https://marc.info/?l=linux-rdma&m=151677697120770.

Thanks,

Bart.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error
       [not found]                         ` <1516833624.3987.19.camel-Sjgp3cTcYWE@public.gmane.org>
@ 2018-01-25  1:19                           ` Doug Ledford
  0 siblings, 0 replies; 15+ messages in thread
From: Doug Ledford @ 2018-01-25  1:19 UTC (permalink / raw)
  To: Bart Van Assche, leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
  Cc: jgg-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

[-- Attachment #1: Type: text/plain, Size: 814 bytes --]

On Wed, 2018-01-24 at 22:40 +0000, Bart Van Assche wrote:
> On Wed, 2018-01-24 at 17:36 -0500, Doug Ledford wrote:
> > Can you please resend this as an official patch with SOB so we can pull
> > this in?  Thanks.
> 
> Hello Doug,
> 
> Can you have a look at this patch and see whether this needs any further
> adjustments: https://marc.info/?l=linux-rdma&m=151677697120770.
> 
> Thanks,
> 
> Bart.

That's good.  I just missed it in my email client because the time/date
sorting of the threads had this thread later in my list and that lone
patch sitting by itself out of sight-out of mind.  Thanks for the
pointer ;-)

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-01-25  1:19 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-23 11:50 [PATCH rdma-next] RDMA/srpt: Fix RCU debug build error Leon Romanovsky
     [not found] ` <20180123115013.12213-1-leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2018-01-23 15:21   ` Jason Gunthorpe
     [not found]     ` <20180123152142.GB30619-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2018-01-23 16:04       ` Bart Van Assche
     [not found]         ` <1516723456.3339.3.camel-Sjgp3cTcYWE@public.gmane.org>
2018-01-23 18:57           ` Leon Romanovsky
     [not found]             ` <20180123185732.GA1393-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2018-01-23 22:30               ` Bart Van Assche
     [not found]                 ` <1516746607.3339.47.camel-Sjgp3cTcYWE@public.gmane.org>
2018-01-24  6:44                   ` Leon Romanovsky
2018-01-24 22:36                   ` Doug Ledford
     [not found]                     ` <1516833393.27592.108.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2018-01-24 22:40                       ` Bart Van Assche
     [not found]                         ` <1516833624.3987.19.camel-Sjgp3cTcYWE@public.gmane.org>
2018-01-25  1:19                           ` Doug Ledford
2018-01-23 22:03           ` Paul E. McKenney
     [not found]             ` <20180123220337.GE3741-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2018-01-23 22:29               ` Bart Van Assche
     [not found]                 ` <1516746545.3339.46.camel-Sjgp3cTcYWE@public.gmane.org>
2018-01-23 22:50                   ` Paul E. McKenney
     [not found]                     ` <20180123225054.GK3741-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
2018-01-23 22:59                       ` Bart Van Assche
     [not found]                         ` <1516748369.3339.59.camel-Sjgp3cTcYWE@public.gmane.org>
2018-01-24  6:43                           ` Leon Romanovsky
     [not found]                             ` <20180124064337.GB1393-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
2018-01-24 21:51                               ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox