* [PATCH tip/core/rcu 03/14] bridge: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 04/14] decnet: " Paul E. McKenney
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, Stephen Hemminger, David S. Miller, bridge,
netdev
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the uses in
br_multicast_del_pg() and br_multicast_new_port_group() are legitimate:
They are assigning a pointer to an element from an RCU-protected list,
and all elements of this list are already visible to caller.
This commit therefore silences these false positives by laundering
the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
---
net/bridge/br_multicast.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index d1c578630678..bcc4bbc16498 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -267,7 +267,7 @@ static void br_multicast_del_pg(struct net_bridge *br,
if (p != pg)
continue;
- rcu_assign_pointer(*pp, p->next);
+ ACCESS_ONCE(*pp) = p->next; /* OK: Both --rcu and visible. */
hlist_del_init(&p->mglist);
del_timer(&p->timer);
call_rcu_bh(&p->rcu, br_multicast_free_pg);
@@ -646,7 +646,7 @@ struct net_bridge_port_group *br_multicast_new_port_group(
p->addr = *group;
p->port = port;
p->state = state;
- rcu_assign_pointer(p->next, next);
+ ACCESS_ONCE(p->next) = next; /* OK: Both --rcu and visible. */
hlist_add_head(&p->mglist, &port->mglist);
setup_timer(&p->timer, br_multicast_port_group_expired,
(unsigned long)p);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 04/14] decnet: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
2013-11-16 0:40 ` [PATCH tip/core/rcu 03/14] bridge: Apply ACCESS_ONCE() to avoid sparse false positive Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 05/14] ipv4/ip_socketglue: " Paul E. McKenney
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Thomas Graf, Gao feng,
Stephen Hemminger, linux-decnet-user, netdev
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
dn_insert_route() is legitimate: It is assigning a pointer to an element
from an RCU-protected list, and all elements of this list are already
visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Graf <tgraf@suug.ch>
Cc: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: linux-decnet-user@lists.sourceforge.net
Cc: netdev@vger.kernel.org
---
net/decnet/dn_route.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index fe32388ea24f..a6ef8b025035 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -344,8 +344,9 @@ static int dn_insert_route(struct dn_route *rt, unsigned int hash, struct dn_rou
if (compare_keys(&rth->fld, &rt->fld)) {
/* Put it first */
*rthp = rth->dst.dn_next;
- rcu_assign_pointer(rth->dst.dn_next,
- dn_rt_hash_table[hash].chain);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(rth->dst.dn_next) =
+ dn_rt_hash_table[hash].chain;
rcu_assign_pointer(dn_rt_hash_table[hash].chain, rth);
dst_use(&rth->dst, now);
@@ -358,7 +359,8 @@ static int dn_insert_route(struct dn_route *rt, unsigned int hash, struct dn_rou
rthp = &rth->dst.dn_next;
}
- rcu_assign_pointer(rt->dst.dn_next, dn_rt_hash_table[hash].chain);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(rt->dst.dn_next) = dn_rt_hash_table[hash].chain;
rcu_assign_pointer(dn_rt_hash_table[hash].chain, rt);
dst_use(&rt->dst, now);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 05/14] ipv4/ip_socketglue: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
2013-11-16 0:40 ` [PATCH tip/core/rcu 03/14] bridge: Apply ACCESS_ONCE() to avoid sparse false positive Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 04/14] decnet: " Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 06/14] ipv6/ip6_tunnel: " Paul E. McKenney
` (6 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
ip_ra_control() is legitimate: It is assigning a pointer to an element
from an RCU-protected list, and all elements of this list are already
visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
---
net/ipv4/ip_sockglue.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index d9c4f113d709..a0e7f176e9c8 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -269,7 +269,8 @@ int ip_ra_control(struct sock *sk, unsigned char on,
}
/* dont let ip_call_ra_chain() use sk again */
ra->sk = NULL;
- rcu_assign_pointer(*rap, ra->next);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(*rap) = ra->next;
spin_unlock_bh(&ip_ra_lock);
if (ra->destructor)
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 06/14] ipv6/ip6_tunnel: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
` (2 preceding siblings ...)
2013-11-16 0:40 ` [PATCH tip/core/rcu 05/14] ipv4/ip_socketglue: " Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 07/14] ipv6/ip6_gre: " Paul E. McKenney
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
ip6_tnl_unlink() is legitimate: It is assigning a pointer to an element
from an RCU-protected list, and all elements of this list are already
visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
---
net/ipv6/ip6_tunnel.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 61355f7f4da5..2bea7a4e49ed 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -245,7 +245,8 @@ ip6_tnl_unlink(struct ip6_tnl_net *ip6n, struct ip6_tnl *t)
(iter = rtnl_dereference(*tp)) != NULL;
tp = &iter->next) {
if (t == iter) {
- rcu_assign_pointer(*tp, t->next);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(*tp) = t->next;
break;
}
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 07/14] ipv6/ip6_gre: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
` (3 preceding siblings ...)
2013-11-16 0:40 ` [PATCH tip/core/rcu 06/14] ipv6/ip6_tunnel: " Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 08/14] ipv6/sit: " Paul E. McKenney
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
ip6gre_tunnel_unlink() is legitimate: It is assigning a pointer to an
element from an RCU-protected list, and all elements of this list are
already visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
---
net/ipv6/ip6_gre.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 6b26e9feafb9..7bc9e1b3283e 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -276,7 +276,8 @@ static void ip6gre_tunnel_unlink(struct ip6gre_net *ign, struct ip6_tnl *t)
(iter = rtnl_dereference(*tp)) != NULL;
tp = &iter->next) {
if (t == iter) {
- rcu_assign_pointer(*tp, t->next);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(*tp) = t->next;
break;
}
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 08/14] ipv6/sit: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
` (4 preceding siblings ...)
2013-11-16 0:40 ` [PATCH tip/core/rcu 07/14] ipv6/ip6_gre: " Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 09/14] mac80211: " Paul E. McKenney
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
ipip6_tunnel_unlink() is legitimate: It is assigning a pointer to an
element from an RCU-protected list, and all elements of this list are
already visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
---
net/ipv6/sit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 7ee5cb96db34..9b976a4b463d 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -157,7 +157,8 @@ static void ipip6_tunnel_unlink(struct sit_net *sitn, struct ip_tunnel *t)
(iter = rtnl_dereference(*tp)) != NULL;
tp = &iter->next) {
if (t == iter) {
- rcu_assign_pointer(*tp, t->next);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(*tp) = t->next;
break;
}
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 09/14] mac80211: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
` (5 preceding siblings ...)
2013-11-16 0:40 ` [PATCH tip/core/rcu 08/14] ipv6/sit: " Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 10/14] bridge/br_mdb: " Paul E. McKenney
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, John W. Linville, Johannes Berg,
David S. Miller, linux-wireless, netdev
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the uses in
sta_info_hash_del() are legitimate: They are assigning a pointer to an
element from an RCU-protected list, and all elements of this list are
already visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "John W. Linville" <linville@tuxdriver.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
---
net/mac80211/sta_info.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index aeb967a0aeed..494f03c0831f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -74,8 +74,8 @@ static int sta_info_hash_del(struct ieee80211_local *local,
if (!s)
return -ENOENT;
if (s == sta) {
- rcu_assign_pointer(local->sta_hash[STA_HASH(sta->sta.addr)],
- s->hnext);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(local->sta_hash[STA_HASH(sta->sta.addr)]) = s->hnext;
return 0;
}
@@ -84,7 +84,8 @@ static int sta_info_hash_del(struct ieee80211_local *local,
s = rcu_dereference_protected(s->hnext,
lockdep_is_held(&local->sta_mtx));
if (rcu_access_pointer(s->hnext)) {
- rcu_assign_pointer(s->hnext, sta->hnext);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(s->hnext) = sta->hnext;
return 0;
}
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 10/14] bridge/br_mdb: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
` (6 preceding siblings ...)
2013-11-16 0:40 ` [PATCH tip/core/rcu 09/14] mac80211: " Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 11/14] bonding/bond_main: " Paul E. McKenney
2013-11-16 0:40 ` [PATCH tip/core/rcu 12/14] bonding/bond_alb.c: " Paul E. McKenney
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: Stephen Hemminger, tglx, laijs, edumazet, peterz, fweisbec,
bridge, josh, rostedt, David S. Miller, dhowells, sbw, niv,
netdev, mathieu.desnoyers, dipankar, darren, akpm,
Paul E. McKenney, mingo
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
__br_mdb_del() is legitimate: They are assigning a pointer to an element
from an RCU-protected list, and all elements of this list are already
visible to caller.
This commit therefore silences these false positives by laundering
the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
---
net/bridge/br_mdb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 85a09bb5ca51..de7197ba8695 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -447,7 +447,7 @@ static int __br_mdb_del(struct net_bridge *br, struct br_mdb_entry *entry)
if (p->port->state == BR_STATE_DISABLED)
goto unlock;
- rcu_assign_pointer(*pp, p->next);
+ ACCESS_ONCE(*pp) = p->next; /* OK: Both --rcu and visible. */
hlist_del_init(&p->mglist);
del_timer(&p->timer);
call_rcu_bh(&p->rcu, br_multicast_free_pg);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 11/14] bonding/bond_main: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
` (7 preceding siblings ...)
2013-11-16 0:40 ` [PATCH tip/core/rcu 10/14] bridge/br_mdb: " Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
2013-11-16 4:32 ` Ding Tianhong
2013-11-16 0:40 ` [PATCH tip/core/rcu 12/14] bonding/bond_alb.c: " Paul E. McKenney
9 siblings, 1 reply; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: Stephen Hemminger, tglx, laijs, edumazet, peterz, fweisbec,
bridge, josh, rostedt, David S. Miller, dhowells, sbw, niv,
netdev, mathieu.desnoyers, dipankar, darren, akpm,
Paul E. McKenney, mingo
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the uses in
bond_change_active_slave() and __bond_release_one() are legitimate:
They are assigning a pointer to an element from an RCU-protected list
(or a NULL pointer), and all elements of this list are already visible
to caller.
This commit therefore silences these false positives either by laundering
the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett, or by using RCU_INIT_POINTER() for NULL pointer assignments.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: bridge@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
---
drivers/net/bonding/bond_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 72df399c4ab3..bbd7fd3e46fe 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -890,7 +890,8 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
if (new_active)
bond_set_slave_active_flags(new_active);
} else {
- rcu_assign_pointer(bond->curr_active_slave, new_active);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(bond->curr_active_slave) = new_active;
}
if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
@@ -1801,7 +1802,7 @@ static int __bond_release_one(struct net_device *bond_dev,
}
if (all) {
- rcu_assign_pointer(bond->curr_active_slave, NULL);
+ RCU_INIT_POINTER(bond->curr_active_slave, NULL);
} else if (oldcurrent == slave) {
/*
* Note that we hold RTNL over this sequence, so there
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 11/14] bonding/bond_main: Apply ACCESS_ONCE() to avoid sparse false positive
2013-11-16 0:40 ` [PATCH tip/core/rcu 11/14] bonding/bond_main: " Paul E. McKenney
@ 2013-11-16 4:32 ` Ding Tianhong
2013-11-16 15:21 ` Paul E. McKenney
0 siblings, 1 reply; 12+ messages in thread
From: Ding Tianhong @ 2013-11-16 4:32 UTC (permalink / raw)
To: Paul E. McKenney
Cc: linux-kernel, mingo, laijs, dipankar, akpm, mathieu.desnoyers,
josh, niv, tglx, peterz, rostedt, dhowells, edumazet, darren,
fweisbec, sbw, Stephen Hemminger, David S. Miller, bridge, netdev
于 2013/11/16 8:40, Paul E. McKenney 写道:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>
> The sparse checking for rcu_assign_pointer() was recently upgraded
> to reject non-__kernel address spaces. This also rejects __rcu,
> which is almost always the right thing to do. However, the uses in
> bond_change_active_slave() and __bond_release_one() are legitimate:
> They are assigning a pointer to an element from an RCU-protected list
> (or a NULL pointer), and all elements of this list are already visible
> to caller.
>
> This commit therefore silences these false positives either by laundering
> the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
> Triplett, or by using RCU_INIT_POINTER() for NULL pointer assignments.
I think it is fit for net-next.
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: bridge@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> ---
> drivers/net/bonding/bond_main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 72df399c4ab3..bbd7fd3e46fe 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -890,7 +890,8 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> if (new_active)
> bond_set_slave_active_flags(new_active);
> } else {
> - rcu_assign_pointer(bond->curr_active_slave, new_active);
> + /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
> + ACCESS_ONCE(bond->curr_active_slave) = new_active;
> }
>
> if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
> @@ -1801,7 +1802,7 @@ static int __bond_release_one(struct net_device *bond_dev,
> }
>
> if (all) {
> - rcu_assign_pointer(bond->curr_active_slave, NULL);
> + RCU_INIT_POINTER(bond->curr_active_slave, NULL);
> } else if (oldcurrent == slave) {
> /*
> * Note that we hold RTNL over this sequence, so there
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH tip/core/rcu 11/14] bonding/bond_main: Apply ACCESS_ONCE() to avoid sparse false positive
2013-11-16 4:32 ` Ding Tianhong
@ 2013-11-16 15:21 ` Paul E. McKenney
0 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 15:21 UTC (permalink / raw)
To: Ding Tianhong
Cc: Stephen Hemminger, tglx, laijs, edumazet, David S. Miller, peterz,
fweisbec, bridge, linux-kernel, rostedt, josh, dhowells, sbw, niv,
netdev, mathieu.desnoyers, dipankar, darren, akpm, mingo
On Sat, Nov 16, 2013 at 12:32:16PM +0800, Ding Tianhong wrote:
> 于 2013/11/16 8:40, Paul E. McKenney 写道:
> > From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >
> > The sparse checking for rcu_assign_pointer() was recently upgraded
> > to reject non-__kernel address spaces. This also rejects __rcu,
> > which is almost always the right thing to do. However, the uses in
> > bond_change_active_slave() and __bond_release_one() are legitimate:
> > They are assigning a pointer to an element from an RCU-protected list
> > (or a NULL pointer), and all elements of this list are already visible
> > to caller.
> >
> > This commit therefore silences these false positives either by laundering
> > the pointers using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
> > Triplett, or by using RCU_INIT_POINTER() for NULL pointer assignments.
>
> I think it is fit for net-next.
Thank you!
If this is queued there, I would be happy to drop it from my tree.
There are no dependencies on anything in my tree.
Thanx, Paul
> > Reported-by: kbuild test robot <fengguang.wu@intel.com>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: bridge@lists.linux-foundation.org
> > Cc: netdev@vger.kernel.org
> > ---
> > drivers/net/bonding/bond_main.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index 72df399c4ab3..bbd7fd3e46fe 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -890,7 +890,8 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> > if (new_active)
> > bond_set_slave_active_flags(new_active);
> > } else {
> > - rcu_assign_pointer(bond->curr_active_slave, new_active);
> > + /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
> > + ACCESS_ONCE(bond->curr_active_slave) = new_active;
> > }
> >
> > if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
> > @@ -1801,7 +1802,7 @@ static int __bond_release_one(struct net_device *bond_dev,
> > }
> >
> > if (all) {
> > - rcu_assign_pointer(bond->curr_active_slave, NULL);
> > + RCU_INIT_POINTER(bond->curr_active_slave, NULL);
> > } else if (oldcurrent == slave) {
> > /*
> > * Note that we hold RTNL over this sequence, so there
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH tip/core/rcu 12/14] bonding/bond_alb.c: Apply ACCESS_ONCE() to avoid sparse false positive
[not found] ` <1384562417-817-1-git-send-email-paulmck@linux.vnet.ibm.com>
` (8 preceding siblings ...)
2013-11-16 0:40 ` [PATCH tip/core/rcu 11/14] bonding/bond_main: " Paul E. McKenney
@ 2013-11-16 0:40 ` Paul E. McKenney
9 siblings, 0 replies; 12+ messages in thread
From: Paul E. McKenney @ 2013-11-16 0:40 UTC (permalink / raw)
To: linux-kernel
Cc: mingo, laijs, dipankar, akpm, mathieu.desnoyers, josh, niv, tglx,
peterz, rostedt, dhowells, edumazet, darren, fweisbec, sbw,
Paul E. McKenney, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
The sparse checking for rcu_assign_pointer() was recently upgraded
to reject non-__kernel address spaces. This also rejects __rcu,
which is almost always the right thing to do. However, the use in
bond_alb_handle_active_change() is legitimate: It is assigning a pointer
to an element from an RCU-protected list, and all elements of this list
are already visible to caller.
This commit therefore silences this false positive by laundering the
pointer using ACCESS_ONCE() as suggested by Eric Dumazet and Josh
Triplett.
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Cc: netdev@vger.kernel.org
---
drivers/net/bonding/bond_alb.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 91f179d5135c..67d3b2893aa3 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1667,7 +1667,8 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
}
swap_slave = bond->curr_active_slave;
- rcu_assign_pointer(bond->curr_active_slave, new_slave);
+ /* Both --rcu and visible, so ACCESS_ONCE() is OK. */
+ ACCESS_ONCE(bond->curr_active_slave) = new_slave;
if (!new_slave || list_empty(&bond->slave_list))
return;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 12+ messages in thread