netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next] tipc: use rcu dereference functions properly
@ 2019-07-01 16:54 Xin Long
  2019-07-02 22:08 ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Xin Long @ 2019-07-01 16:54 UTC (permalink / raw)
  To: network dev; +Cc: Jon Maloy, Ying Xue, tipc-discussion, davem

For these places are protected by rcu_read_lock, we change from
rcu_dereference_rtnl to rcu_dereference, as there is no need to
check if rtnl lock is held.

For these places are protected by rtnl_lock, we change from
rcu_dereference_rtnl to rtnl_dereference/rcu_dereference_protected,
as no extra memory barriers are needed under rtnl_lock() which also
protects tn->bearer_list[] and dev->tipc_ptr/b->media_ptr updating.

rcu_dereference_rtnl will be only used in the places where it could
be under rcu_read_lock or rtnl_lock.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/tipc/bearer.c    | 14 +++++++-------
 net/tipc/udp_media.c |  8 ++++----
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 2bed658..a809c0e 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -62,7 +62,7 @@ static struct tipc_bearer *bearer_get(struct net *net, int bearer_id)
 {
 	struct tipc_net *tn = tipc_net(net);
 
-	return rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+	return rcu_dereference(tn->bearer_list[bearer_id]);
 }
 
 static void bearer_disable(struct net *net, struct tipc_bearer *b);
@@ -210,7 +210,7 @@ void tipc_bearer_add_dest(struct net *net, u32 bearer_id, u32 dest)
 	struct tipc_bearer *b;
 
 	rcu_read_lock();
-	b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+	b = rcu_dereference(tn->bearer_list[bearer_id]);
 	if (b)
 		tipc_disc_add_dest(b->disc);
 	rcu_read_unlock();
@@ -222,7 +222,7 @@ void tipc_bearer_remove_dest(struct net *net, u32 bearer_id, u32 dest)
 	struct tipc_bearer *b;
 
 	rcu_read_lock();
-	b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+	b = rcu_dereference(tn->bearer_list[bearer_id]);
 	if (b)
 		tipc_disc_remove_dest(b->disc);
 	rcu_read_unlock();
@@ -444,7 +444,7 @@ int tipc_l2_send_msg(struct net *net, struct sk_buff *skb,
 	struct net_device *dev;
 	int delta;
 
-	dev = (struct net_device *)rcu_dereference_rtnl(b->media_ptr);
+	dev = (struct net_device *)rcu_dereference(b->media_ptr);
 	if (!dev)
 		return 0;
 
@@ -481,7 +481,7 @@ int tipc_bearer_mtu(struct net *net, u32 bearer_id)
 	struct tipc_bearer *b;
 
 	rcu_read_lock();
-	b = rcu_dereference_rtnl(tipc_net(net)->bearer_list[bearer_id]);
+	b = rcu_dereference(tipc_net(net)->bearer_list[bearer_id]);
 	if (b)
 		mtu = b->mtu;
 	rcu_read_unlock();
@@ -574,8 +574,8 @@ static int tipc_l2_rcv_msg(struct sk_buff *skb, struct net_device *dev,
 	struct tipc_bearer *b;
 
 	rcu_read_lock();
-	b = rcu_dereference_rtnl(dev->tipc_ptr) ?:
-		rcu_dereference_rtnl(orig_dev->tipc_ptr);
+	b = rcu_dereference(dev->tipc_ptr) ?:
+		rcu_dereference(orig_dev->tipc_ptr);
 	if (likely(b && test_bit(0, &b->up) &&
 		   (skb->pkt_type <= PACKET_MULTICAST))) {
 		skb_mark_not_on_list(skb);
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index b8962df..62b85db 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -231,7 +231,7 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
 	}
 
 	skb_set_inner_protocol(skb, htons(ETH_P_TIPC));
-	ub = rcu_dereference_rtnl(b->media_ptr);
+	ub = rcu_dereference(b->media_ptr);
 	if (!ub) {
 		err = -ENODEV;
 		goto out;
@@ -490,7 +490,7 @@ int tipc_udp_nl_dump_remoteip(struct sk_buff *skb, struct netlink_callback *cb)
 		}
 	}
 
-	ub = rcu_dereference_rtnl(b->media_ptr);
+	ub = rtnl_dereference(b->media_ptr);
 	if (!ub) {
 		rtnl_unlock();
 		return -EINVAL;
@@ -532,7 +532,7 @@ int tipc_udp_nl_add_bearer_data(struct tipc_nl_msg *msg, struct tipc_bearer *b)
 	struct udp_bearer *ub;
 	struct nlattr *nest;
 
-	ub = rcu_dereference_rtnl(b->media_ptr);
+	ub = rtnl_dereference(b->media_ptr);
 	if (!ub)
 		return -ENODEV;
 
@@ -806,7 +806,7 @@ static void tipc_udp_disable(struct tipc_bearer *b)
 {
 	struct udp_bearer *ub;
 
-	ub = rcu_dereference_rtnl(b->media_ptr);
+	ub = rtnl_dereference(b->media_ptr);
 	if (!ub) {
 		pr_err("UDP bearer instance not found\n");
 		return;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tipc: use rcu dereference functions properly
  2019-07-01 16:54 [PATCH net-next] tipc: use rcu dereference functions properly Xin Long
@ 2019-07-02 22:08 ` David Miller
  2019-07-03  8:33   ` Xin Long
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-07-02 22:08 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion

From: Xin Long <lucien.xin@gmail.com>
Date: Tue,  2 Jul 2019 00:54:55 +0800

> For these places are protected by rcu_read_lock, we change from
> rcu_dereference_rtnl to rcu_dereference, as there is no need to
> check if rtnl lock is held.
> 
> For these places are protected by rtnl_lock, we change from
> rcu_dereference_rtnl to rtnl_dereference/rcu_dereference_protected,
> as no extra memory barriers are needed under rtnl_lock() which also
> protects tn->bearer_list[] and dev->tipc_ptr/b->media_ptr updating.
> 
> rcu_dereference_rtnl will be only used in the places where it could
> be under rcu_read_lock or rtnl_lock.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

In the cases where RTNL is held, even if rcu_read_lock() is also taken,
we should use rtnl_dereference() because that avoids the READ_ONCE().

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tipc: use rcu dereference functions properly
  2019-07-02 22:08 ` David Miller
@ 2019-07-03  8:33   ` Xin Long
  2019-07-06  6:48     ` Xin Long
  0 siblings, 1 reply; 6+ messages in thread
From: Xin Long @ 2019-07-03  8:33 UTC (permalink / raw)
  To: David Miller; +Cc: network dev, Jon Maloy, Ying Xue, tipc-discussion

On Wed, Jul 3, 2019 at 6:08 AM David Miller <davem@davemloft.net> wrote:
>
> From: Xin Long <lucien.xin@gmail.com>
> Date: Tue,  2 Jul 2019 00:54:55 +0800
>
> > For these places are protected by rcu_read_lock, we change from
> > rcu_dereference_rtnl to rcu_dereference, as there is no need to
> > check if rtnl lock is held.
> >
> > For these places are protected by rtnl_lock, we change from
> > rcu_dereference_rtnl to rtnl_dereference/rcu_dereference_protected,
> > as no extra memory barriers are needed under rtnl_lock() which also
> > protects tn->bearer_list[] and dev->tipc_ptr/b->media_ptr updating.
> >
> > rcu_dereference_rtnl will be only used in the places where it could
> > be under rcu_read_lock or rtnl_lock.
> >
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> In the cases where RTNL is held, even if rcu_read_lock() is also taken,
> we should use rtnl_dereference() because that avoids the READ_ONCE().
Right, that's what I did in this patch.

But for the places where it's sometimes called under rtnl_lock() only and
sometimes called under rcu_read_lock() only, like tipc_udp_is_known_peer()
and tipc_udp_rcast_add(), I kept rcu_dereference_rtnl(). makes sense?

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tipc: use rcu dereference functions properly
  2019-07-03  8:33   ` Xin Long
@ 2019-07-06  6:48     ` Xin Long
  2019-07-06 22:15       ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: Xin Long @ 2019-07-06  6:48 UTC (permalink / raw)
  To: David Miller; +Cc: network dev, Jon Maloy, Ying Xue, tipc-discussion

On Wed, Jul 3, 2019 at 4:33 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Wed, Jul 3, 2019 at 6:08 AM David Miller <davem@davemloft.net> wrote:
> >
> > From: Xin Long <lucien.xin@gmail.com>
> > Date: Tue,  2 Jul 2019 00:54:55 +0800
> >
> > > For these places are protected by rcu_read_lock, we change from
> > > rcu_dereference_rtnl to rcu_dereference, as there is no need to
> > > check if rtnl lock is held.
> > >
> > > For these places are protected by rtnl_lock, we change from
> > > rcu_dereference_rtnl to rtnl_dereference/rcu_dereference_protected,
> > > as no extra memory barriers are needed under rtnl_lock() which also
> > > protects tn->bearer_list[] and dev->tipc_ptr/b->media_ptr updating.
> > >
> > > rcu_dereference_rtnl will be only used in the places where it could
> > > be under rcu_read_lock or rtnl_lock.
> > >
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >
> > In the cases where RTNL is held, even if rcu_read_lock() is also taken,
> > we should use rtnl_dereference() because that avoids the READ_ONCE().
> Right, that's what I did in this patch.
>
> But for the places where it's sometimes called under rtnl_lock() only and
> sometimes called under rcu_read_lock() only, like tipc_udp_is_known_peer()
> and tipc_udp_rcast_add(), I kept rcu_dereference_rtnl(). makes sense?
Hi, David, I saw this patch in "Changes Requested".

I've checked all places with this patch, no function calling rcu_dereference()
and rcu_dereference_rtnl() will be ONLY called under rtnl_lock() protection.
So I can't see a problem with it.

Do I need to resend?

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tipc: use rcu dereference functions properly
  2019-07-06  6:48     ` Xin Long
@ 2019-07-06 22:15       ` David Miller
  2019-07-07 20:19         ` David Miller
  0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2019-07-06 22:15 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion

From: Xin Long <lucien.xin@gmail.com>
Date: Sat, 6 Jul 2019 14:48:48 +0800

> Hi, David, I saw this patch in "Changes Requested".

I just put it back to Under Review, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next] tipc: use rcu dereference functions properly
  2019-07-06 22:15       ` David Miller
@ 2019-07-07 20:19         ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-07-07 20:19 UTC (permalink / raw)
  To: lucien.xin; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion

From: David Miller <davem@davemloft.net>
Date: Sat, 06 Jul 2019 15:15:44 -0700 (PDT)

> From: Xin Long <lucien.xin@gmail.com>
> Date: Sat, 6 Jul 2019 14:48:48 +0800
> 
>> Hi, David, I saw this patch in "Changes Requested".
> 
> I just put it back to Under Review, thanks.

Applied to net-next, thank you.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-07 20:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-01 16:54 [PATCH net-next] tipc: use rcu dereference functions properly Xin Long
2019-07-02 22:08 ` David Miller
2019-07-03  8:33   ` Xin Long
2019-07-06  6:48     ` Xin Long
2019-07-06 22:15       ` David Miller
2019-07-07 20:19         ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).