[RFC 0/5] RCU fixes for rcu_assign_pointer usage

[RFC 0/5] RCU fixes for rcu_assign_pointer usage

[Joel Fernandes (Google)]

These patches fix various RCU API usage issues found due to sparse errors as a
result of the recent check to add rcu_check_sparse() to rcu_assign_pointer().

This is very early RFC stage, and is only build tested. I am also only sending
to the RCU group for initial review before sending to LKML. Thanks for any feedback!

There are still more usages that cause errors such as rbtree which I am
looking into.

Joel Fernandes (5):
net: rtnetlink: Fix incorrect RCU API usage
ixgbe: Fix incorrect RCU API usage
sched/cpufreq: Fix incorrect RCU API usage
sched/toplogy: Use smp_store_release() instead of rcu_assign_pointer
rcuwait: Replace rcu_assign_pointer with smp_store_release

drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++-----
include/linux/rcuwait.h                       |  2 +-
kernel/sched/cpufreq.c                        |  8 ++++++--
kernel/sched/sched.h                          |  2 +-
kernel/sched/topology.c                       | 16 ++++++++--------
net/core/rtnetlink.c                          |  4 ++--
7 files changed, 31 insertions(+), 21 deletions(-)

--
2.21.0.rc0.258.g878e2cd30e-goog

[RFC 1/5] net: rtnetlink: Fix incorrect RCU API usage

[Joel Fernandes (Google)]

From: Joel Fernandes <joel@joelfernandes.org>

rtnl_register_internal() and rtnl_unregister_all tries to directly
dereference an RCU protected pointed outside RCU read side section.
While this is Ok to do since a lock is held, let us use the correct
API to avoid programmer bugs in the future.

This also fixes sparse warnings arising from not using RCU API.

net/core/rtnetlink.c:332:13: warning: incorrect type in assignment
(different address spaces) net/core/rtnetlink.c:332:13:    expected
struct rtnl_link **tab net/core/rtnetlink.c:332:13:    got struct
rtnl_link *[noderef] <asn:4>*<noident>

Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
---
 net/core/rtnetlink.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5ea1bed08ede..98be4b4818a9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -188,7 +188,7 @@ static int rtnl_register_internal(struct module *owner,
 	msgindex = rtm_msgindex(msgtype);
 
 	rtnl_lock();
-	tab = rtnl_msg_handlers[protocol];
+	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
 	if (tab == NULL) {
 		tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
 		if (!tab)
@@ -329,7 +329,7 @@ void rtnl_unregister_all(int protocol)
 	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
 
 	rtnl_lock();
-	tab = rtnl_msg_handlers[protocol];
+	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
 	if (!tab) {
 		rtnl_unlock();
 		return;
-- 
2.21.0.rc0.258.g878e2cd30e-goog

[RFC 2/5] ixgbe: Fix incorrect RCU API usage

[Joel Fernandes (Google)]

From: Joel Fernandes <joel@joelfernandes.org>

Recently, I added an RCU annotation check in rcu_assign_pointer. This
caused a sparse error to be reported by the ixgbe driver.

Further looking, it seems the adapter->xdp_prog pointer is not annotated
with __rcu. Annonating it fixed the error, but caused a bunch of other
warnings.

This patch tries to fix all warnings by using RCU API properly. This
makes sense to do because not using RCU properly can result in various
hard to find bugs. This is a best effort fix and is only build tested.
The sparse errors and warnings go away with the change. I request
maintainers / developers in this area to test it properly.

Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |  4 ++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++++-----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 08d85e336bd4..3b14daf27516 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -311,7 +311,7 @@ struct ixgbe_ring {
 	struct ixgbe_ring *next;	/* pointer to next ring in q_vector */
 	struct ixgbe_q_vector *q_vector; /* backpointer to host q_vector */
 	struct net_device *netdev;	/* netdev ring belongs to */
-	struct bpf_prog *xdp_prog;
+	struct bpf_prog __rcu *xdp_prog;
 	struct device *dev;		/* device for DMA mapping */
 	void *desc;			/* descriptor ring memory */
 	union {
@@ -560,7 +560,7 @@ struct ixgbe_adapter {
 	unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
 	/* OS defined structs */
 	struct net_device *netdev;
-	struct bpf_prog *xdp_prog;
+	struct bpf_prog __rcu *xdp_prog;
 	struct pci_dev *pdev;
 	struct mii_bus *mii_bus;
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index daff8183534b..aad7b800aacd 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2199,7 +2199,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
 	u32 act;
 
 	rcu_read_lock();
-	xdp_prog = READ_ONCE(rx_ring->xdp_prog);
+	xdp_prog = rcu_dereference(rx_ring->xdp_prog);
 
 	if (!xdp_prog)
 		goto xdp_out;
@@ -6547,7 +6547,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
 			     rx_ring->queue_index) < 0)
 		goto err;
 
-	rx_ring->xdp_prog = adapter->xdp_prog;
+	rcu_assign_pointer(rx_ring->xdp_prog, adapter->xdp_prog);
 
 	return 0;
 err:
@@ -10246,7 +10246,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 	if (nr_cpu_ids > MAX_XDP_QUEUES)
 		return -ENOMEM;
 
-	old_prog = xchg(&adapter->xdp_prog, prog);
+	old_prog = rcu_dereference(adapter->xdp_prog);
+	rcu_assign_pointer(adapter->xdp_prog, prog);
 
 	/* If transitioning XDP modes reconfigure rings */
 	if (!!prog != !!old_prog) {
@@ -10271,13 +10272,18 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	struct bpf_prog *prog;
+	int ret;
 
 	switch (xdp->command) {
 	case XDP_SETUP_PROG:
 		return ixgbe_xdp_setup(dev, xdp->prog);
 	case XDP_QUERY_PROG:
-		xdp->prog_id = adapter->xdp_prog ?
-			adapter->xdp_prog->aux->id : 0;
+		rcu_read_lock();
+		prog = rcu_dereference(adapter->xdp_prog);
+		xdp->prog_id = prog ? prog->aux->id : 0;
+		rcu_read_unlock();
+
 		return 0;
 	case XDP_QUERY_XSK_UMEM:
 		return ixgbe_xsk_umem_query(adapter, &xdp->xsk.umem,
-- 
2.21.0.rc0.258.g878e2cd30e-goog

[RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage

[Joel Fernandes (Google)]

From: Joel Fernandes <joel@joelfernandes.org>

Recently I added an RCU annotation check to rcu_assign_pointer(). All
pointers assigned to RCU protected data are to be annotated with __rcu
inorder to be able to use rcu_assign_pointer() similar to checks in
other RCU APIs.

This resulted in a sparse error: kernel//sched/cpufreq.c:41:9: sparse:
error: incompatible types in comparison expression (different address
spaces)

Fix this by using the correct APIs for RCU accesses. This will
potentially avoid any future bugs in the code. If it is felt that RCU
protection is not needed here, then the rcu_assign_pointer call can be
dropped and replaced with, say, WRITE_ONCE or smp_store_release. Or, may
be we add a new API to do it. But calls rcu_assign_pointer seems an
abuse of the RCU API.

Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
---
 kernel/sched/cpufreq.c | 8 ++++++--
 kernel/sched/sched.h   | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c
index 22bd8980f32f..c9aeb3bf5dc2 100644
--- a/kernel/sched/cpufreq.c
+++ b/kernel/sched/cpufreq.c
@@ -7,7 +7,7 @@
  */
 #include "sched.h"
 
-DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
 
 /**
  * cpufreq_add_update_util_hook - Populate the CPU's update_util_data pointer.
@@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
 	if (WARN_ON(!data || !func))
 		return;
 
-	if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
+	rcu_read_lock();
+	if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
+		rcu_read_unlock();
 		return;
+	}
+	rcu_read_unlock();
 
 	data->func = func;
 	rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index d04530bf251f..2ab545d40381 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2166,7 +2166,7 @@ static inline u64 irq_time_read(int cpu)
 #endif /* CONFIG_IRQ_TIME_ACCOUNTING */
 
 #ifdef CONFIG_CPU_FREQ
-DECLARE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
+DECLARE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
 
 /**
  * cpufreq_update_util - Take a note about CPU utilization changes.
-- 
2.21.0.rc0.258.g878e2cd30e-goog

[RFC 4/5] sched/toplogy: Use smp_store_release() instead of rcu_assign_pointer

[Joel Fernandes (Google)]

From: Joel Fernandes <joel@joelfernandes.org>

The scheduler's topology code seems to want to use rcu_assign_pointer()
to initialize various pointers for no apparent reason.

With a guess that what was needed here is smp_store_release(), I am
replacing it with that. This suppresses the new sparse errors caused by
an annotation check I added to rcu_assign_pointer(). Let us avoid (ab)using
RCU API and be explicit about what we want.

Fixes sparse errors:
kernel//sched/topology.c:206:1: sparse: warning: symbol
'sched_energy_mutex' was not declared. Should it be static?
kernel//sched/topology.c:207:6: sparse: warning: symbol
'sched_energy_update' was not declared. Should it be static?  >>
kernel//sched/topology.c:378:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:387:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:612:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:615:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:618:9: sparse: error: incompatible types in
comparison expression (different address spaces)
kernel//sched/topology.c:621:9: sparse: error: incompatible types in
comparison expression (different address spaces)

Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
---
 kernel/sched/topology.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 3f35ba1d8fde..e7a424d8de8e 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -375,7 +375,7 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
 
 	/* Attach the new list of performance domains to the root domain. */
 	tmp = rd->pd;
-	rcu_assign_pointer(rd->pd, pd);
+	smp_store_release(&rd->pd, pd);
 	if (tmp)
 		call_rcu(&tmp->rcu, destroy_perf_domain_rcu);
 
@@ -384,7 +384,7 @@ static bool build_perf_domains(const struct cpumask *cpu_map)
 free:
 	free_pd(pd);
 	tmp = rd->pd;
-	rcu_assign_pointer(rd->pd, NULL);
+	smp_store_release(&rd->pd, NULL);
 	if (tmp)
 		call_rcu(&tmp->rcu, destroy_perf_domain_rcu);
 
@@ -609,19 +609,19 @@ static void update_top_cache_domain(int cpu)
 		sds = sd->shared;
 	}
 
-	rcu_assign_pointer(per_cpu(sd_llc, cpu), sd);
+	smp_store_release(&per_cpu(sd_llc, cpu), sd);
 	per_cpu(sd_llc_size, cpu) = size;
 	per_cpu(sd_llc_id, cpu) = id;
-	rcu_assign_pointer(per_cpu(sd_llc_shared, cpu), sds);
+	smp_store_release(&per_cpu(sd_llc_shared, cpu), sds);
 
 	sd = lowest_flag_domain(cpu, SD_NUMA);
-	rcu_assign_pointer(per_cpu(sd_numa, cpu), sd);
+	smp_store_release(&per_cpu(sd_numa, cpu), sd);
 
 	sd = highest_flag_domain(cpu, SD_ASYM_PACKING);
-	rcu_assign_pointer(per_cpu(sd_asym_packing, cpu), sd);
+	smp_store_release(&per_cpu(sd_asym_packing, cpu), sd);
 
 	sd = lowest_flag_domain(cpu, SD_ASYM_CPUCAPACITY);
-	rcu_assign_pointer(per_cpu(sd_asym_cpucapacity, cpu), sd);
+	smp_store_release(&per_cpu(sd_asym_cpucapacity, cpu), sd);
 }
 
 /*
@@ -668,7 +668,7 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)
 
 	rq_attach_root(rq, rd);
 	tmp = rq->sd;
-	rcu_assign_pointer(rq->sd, sd);
+	smp_store_release(&rq->sd, sd);
 	dirty_sched_domain_sysctl(cpu);
 	destroy_sched_domains(tmp);
 
-- 
2.21.0.rc0.258.g878e2cd30e-goog

[RFC 5/5] rcuwait: Replace rcu_assign_pointer with smp_store_release

[Joel Fernandes (Google)]

From: Joel Fernandes <joel@joelfernandes.org>

This suppresses a sparse error generated to the recently added
rcu_assign_pointer sparse check:

>> kernel//locking/percpu-rwsem.c:162:9: sparse: error: incompatible
types in comparison expression (different address spaces)

Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
---
 include/linux/rcuwait.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/rcuwait.h b/include/linux/rcuwait.h
index 90bfa3279a01..da613efee45c 100644
--- a/include/linux/rcuwait.h
+++ b/include/linux/rcuwait.h
@@ -44,7 +44,7 @@ extern void rcuwait_wake_up(struct rcuwait *w);
 	 */                                                             \
 	WARN_ON(current->exit_state);                                   \
 									\
-	rcu_assign_pointer((w)->task, current);				\
+	smp_store_release(&((w)->task), current);			\
 	for (;;) {							\
 		/*							\
 		 * Implicit barrier (A) pairs with (B) in		\
-- 
2.21.0.rc0.258.g878e2cd30e-goog

Re: [RFC 0/5] RCU fixes for rcu_assign_pointer usage

[Joel Fernandes]

On Tue, Feb 19, 2019 at 8:08 PM Joel Fernandes (Google)
<joel@joelfernandes.org> wrote:
>
> These patches fix various RCU API usage issues found due to sparse errors as a
> result of the recent check to add rcu_check_sparse() to rcu_assign_pointer().
>
> This is very early RFC stage, and is only build tested. I am also only sending
> to the RCU group for initial review before sending to LKML. Thanks for any feedback!
>
> There are still more usages that cause errors such as rbtree which I am
> looking into.
>

Looks like it got sent to LKML anyway, ;-) That's Ok since it is
prefixed as RFC.

Re: [RFC 1/5] net: rtnetlink: Fix incorrect RCU API usage

[Paul E. McKenney]

On Tue, Feb 19, 2019 at 11:08:23PM -0500, Joel Fernandes (Google) wrote:
> From: Joel Fernandes <joel@joelfernandes.org>
> 
> rtnl_register_internal() and rtnl_unregister_all tries to directly
> dereference an RCU protected pointed outside RCU read side section.
> While this is Ok to do since a lock is held, let us use the correct
> API to avoid programmer bugs in the future.
> 
> This also fixes sparse warnings arising from not using RCU API.
> 
> net/core/rtnetlink.c:332:13: warning: incorrect type in assignment
> (different address spaces) net/core/rtnetlink.c:332:13:    expected
> struct rtnl_link **tab net/core/rtnetlink.c:332:13:    got struct
> rtnl_link *[noderef] <asn:4>*<noident>
> 
> Signed-off-by: Joel Fernandes <joel@joelfernandes.org>

First, thank you for doing this!

I was going to complain that these were update-side accesses, but it
looks like rtnl_dereference() already handles both readers and updaters.

So looks good to me, but the maintainers of course have the final word.

							Thanx, Paul

> ---
>  net/core/rtnetlink.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 5ea1bed08ede..98be4b4818a9 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -188,7 +188,7 @@ static int rtnl_register_internal(struct module *owner,
>  	msgindex = rtm_msgindex(msgtype);
>  
>  	rtnl_lock();
> -	tab = rtnl_msg_handlers[protocol];
> +	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
>  	if (tab == NULL) {
>  		tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
>  		if (!tab)
> @@ -329,7 +329,7 @@ void rtnl_unregister_all(int protocol)
>  	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
>  
>  	rtnl_lock();
> -	tab = rtnl_msg_handlers[protocol];
> +	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
>  	if (!tab) {
>  		rtnl_unlock();
>  		return;
> -- 
> 2.21.0.rc0.258.g878e2cd30e-goog
> 

Re: [RFC 0/5] RCU fixes for rcu_assign_pointer usage

[Paul E. McKenney]

On Tue, Feb 19, 2019 at 08:11:36PM -0800, Joel Fernandes wrote:
> On Tue, Feb 19, 2019 at 8:08 PM Joel Fernandes (Google)
> <joel@joelfernandes.org> wrote:
> >
> > These patches fix various RCU API usage issues found due to sparse errors as a
> > result of the recent check to add rcu_check_sparse() to rcu_assign_pointer().
> >
> > This is very early RFC stage, and is only build tested. I am also only sending
> > to the RCU group for initial review before sending to LKML. Thanks for any feedback!
> >
> > There are still more usages that cause errors such as rbtree which I am
> > looking into.
> 
> Looks like it got sent to LKML anyway, ;-) That's Ok since it is
> prefixed as RFC.

As is only right and proper.  ;-)

I don't see an immediate problem with them, but it would be good to get
the relevant developers and maintainers on CC for the next version.  I
cannot claim to know that code very well.

							Thanx, Paul

Re: [RFC 1/5] net: rtnetlink: Fix incorrect RCU API usage

[Joel Fernandes]

On Wed, Feb 20, 2019 at 08:40:34AM -0800, Paul E. McKenney wrote:
> On Tue, Feb 19, 2019 at 11:08:23PM -0500, Joel Fernandes (Google) wrote:
> > From: Joel Fernandes <joel@joelfernandes.org>
> > 
> > rtnl_register_internal() and rtnl_unregister_all tries to directly
> > dereference an RCU protected pointed outside RCU read side section.
> > While this is Ok to do since a lock is held, let us use the correct
> > API to avoid programmer bugs in the future.
> > 
> > This also fixes sparse warnings arising from not using RCU API.
> > 
> > net/core/rtnetlink.c:332:13: warning: incorrect type in assignment
> > (different address spaces) net/core/rtnetlink.c:332:13:    expected
> > struct rtnl_link **tab net/core/rtnetlink.c:332:13:    got struct
> > rtnl_link *[noderef] <asn:4>*<noident>
> > 
> > Signed-off-by: Joel Fernandes <joel@joelfernandes.org>
> 
> First, thank you for doing this!

No problem, it is my pleasure. It is just good to see these warnings/errors
show up (which I didn't anticipate when I first wrote the check) so we can
harden the kernel more fwiw.

> I was going to complain that these were update-side accesses, but it
> looks like rtnl_dereference() already handles both readers and updaters.
> 
> So looks good to me, but the maintainers of course have the final word.

Thanks!
Also my confidence level is a bit less for patches 4/5 and 5/5, could
you share your thoughts on those? The scheduler code seems to use
rcu_assign_pointer() in those where it seems a WRITE_ONCE() would just suffice.
In fact, in some cases I replaced with smp_store_release() just to be safe.
Speaking of which, do you feel those are legit uses of rcu_assign_pointer()
or would you expect rcu_assign_pointer() to be used only for RCU protected
pointers? I am hoping it is the latter since that is what the sparse check
expects (and RCU protected pointer being assigned to).

 - Joel


> 
> > ---
> >  net/core/rtnetlink.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index 5ea1bed08ede..98be4b4818a9 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -188,7 +188,7 @@ static int rtnl_register_internal(struct module *owner,
> >  	msgindex = rtm_msgindex(msgtype);
> >  
> >  	rtnl_lock();
> > -	tab = rtnl_msg_handlers[protocol];
> > +	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
> >  	if (tab == NULL) {
> >  		tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
> >  		if (!tab)
> > @@ -329,7 +329,7 @@ void rtnl_unregister_all(int protocol)
> >  	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
> >  
> >  	rtnl_lock();
> > -	tab = rtnl_msg_handlers[protocol];
> > +	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
> >  	if (!tab) {
> >  		rtnl_unlock();
> >  		return;
> > -- 
> > 2.21.0.rc0.258.g878e2cd30e-goog
> > 
> 

Re: [RFC 0/5] RCU fixes for rcu_assign_pointer usage

[Joel Fernandes]

On Wed, Feb 20, 2019 at 08:42:43AM -0800, Paul E. McKenney wrote:
> On Tue, Feb 19, 2019 at 08:11:36PM -0800, Joel Fernandes wrote:
> > On Tue, Feb 19, 2019 at 8:08 PM Joel Fernandes (Google)
> > <joel@joelfernandes.org> wrote:
> > >
> > > These patches fix various RCU API usage issues found due to sparse errors as a
> > > result of the recent check to add rcu_check_sparse() to rcu_assign_pointer().
> > >
> > > This is very early RFC stage, and is only build tested. I am also only sending
> > > to the RCU group for initial review before sending to LKML. Thanks for any feedback!
> > >
> > > There are still more usages that cause errors such as rbtree which I am
> > > looking into.
> > 
> > Looks like it got sent to LKML anyway, ;-) That's Ok since it is
> > prefixed as RFC.
> 
> As is only right and proper.  ;-)
> 
> I don't see an immediate problem with them, but it would be good to get
> the relevant developers and maintainers on CC for the next version.  I
> cannot claim to know that code very well.

Definitely will CC them next time, sorry about that. I'll stop being so shy
but I have some scars that are still healing ;-)

 - Joel

Re: [RFC 0/5] RCU fixes for rcu_assign_pointer usage

[Paul E. McKenney]

On Wed, Feb 20, 2019 at 01:09:52PM -0500, Joel Fernandes wrote:
> On Wed, Feb 20, 2019 at 08:42:43AM -0800, Paul E. McKenney wrote:
> > On Tue, Feb 19, 2019 at 08:11:36PM -0800, Joel Fernandes wrote:
> > > On Tue, Feb 19, 2019 at 8:08 PM Joel Fernandes (Google)
> > > <joel@joelfernandes.org> wrote:
> > > >
> > > > These patches fix various RCU API usage issues found due to sparse errors as a
> > > > result of the recent check to add rcu_check_sparse() to rcu_assign_pointer().
> > > >
> > > > This is very early RFC stage, and is only build tested. I am also only sending
> > > > to the RCU group for initial review before sending to LKML. Thanks for any feedback!
> > > >
> > > > There are still more usages that cause errors such as rbtree which I am
> > > > looking into.
> > > 
> > > Looks like it got sent to LKML anyway, ;-) That's Ok since it is
> > > prefixed as RFC.
> > 
> > As is only right and proper.  ;-)
> > 
> > I don't see an immediate problem with them, but it would be good to get
> > the relevant developers and maintainers on CC for the next version.  I
> > cannot claim to know that code very well.
> 
> Definitely will CC them next time, sorry about that. I'll stop being so shy
> but I have some scars that are still healing ;-)

If you don't get at least a few scars, you aren't trying hard enough!  ;-)

							Thanx, Paul

Re: [RFC 0/5] RCU fixes for rcu_assign_pointer usage

[Joel Fernandes]

On Wed, Feb 20, 2019 at 10:28:20AM -0800, Paul E. McKenney wrote:
> On Wed, Feb 20, 2019 at 01:09:52PM -0500, Joel Fernandes wrote:
> > On Wed, Feb 20, 2019 at 08:42:43AM -0800, Paul E. McKenney wrote:
> > > On Tue, Feb 19, 2019 at 08:11:36PM -0800, Joel Fernandes wrote:
> > > > On Tue, Feb 19, 2019 at 8:08 PM Joel Fernandes (Google)
> > > > <joel@joelfernandes.org> wrote:
> > > > >
> > > > > These patches fix various RCU API usage issues found due to sparse errors as a
> > > > > result of the recent check to add rcu_check_sparse() to rcu_assign_pointer().
> > > > >
> > > > > This is very early RFC stage, and is only build tested. I am also only sending
> > > > > to the RCU group for initial review before sending to LKML. Thanks for any feedback!
> > > > >
> > > > > There are still more usages that cause errors such as rbtree which I am
> > > > > looking into.
> > > > 
> > > > Looks like it got sent to LKML anyway, ;-) That's Ok since it is
> > > > prefixed as RFC.
> > > 
> > > As is only right and proper.  ;-)
> > > 
> > > I don't see an immediate problem with them, but it would be good to get
> > > the relevant developers and maintainers on CC for the next version.  I
> > > cannot claim to know that code very well.
> > 
> > Definitely will CC them next time, sorry about that. I'll stop being so shy
> > but I have some scars that are still healing ;-)
> 
> If you don't get at least a few scars, you aren't trying hard enough!  ;-)

Well said ;-) I noted this as a quote in my "famous quotes list" ;)

thanks,

 - Joel