* [PATCH] net: dev_getfirstbyhwtype() optimization
@ 2010-03-18 21:27 Eric Dumazet
2010-03-18 21:54 ` Laurent Chavey
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Eric Dumazet @ 2010-03-18 21:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Use RCU to avoid RTNL use in dev_getfirstbyhwtype()
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/core/dev.c b/net/core/dev.c
index 17b1686..0f2e9fc 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -772,14 +772,17 @@ EXPORT_SYMBOL(__dev_getfirstbyhwtype);
struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type)
{
- struct net_device *dev;
+ struct net_device *dev, *ret = NULL;
- rtnl_lock();
- dev = __dev_getfirstbyhwtype(net, type);
- if (dev)
- dev_hold(dev);
- rtnl_unlock();
- return dev;
+ rcu_read_lock();
+ for_each_netdev_rcu(net, dev)
+ if (dev->type == type) {
+ dev_hold(dev);
+ ret = dev;
+ break;
+ }
+ rcu_read_unlock();
+ return ret;
}
EXPORT_SYMBOL(dev_getfirstbyhwtype);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net: dev_getfirstbyhwtype() optimization 2010-03-18 21:27 [PATCH] net: dev_getfirstbyhwtype() optimization Eric Dumazet @ 2010-03-18 21:54 ` Laurent Chavey 2010-03-19 2:32 ` Paul E. McKenney 2010-03-22 3:39 ` David Miller 2 siblings, 0 replies; 6+ messages in thread From: Laurent Chavey @ 2010-03-18 21:54 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Miller, netdev reviewed_by: Laurent Chavey <chavey@google.com> On Thu, Mar 18, 2010 at 2:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > Use RCU to avoid RTNL use in dev_getfirstbyhwtype() > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > diff --git a/net/core/dev.c b/net/core/dev.c > index 17b1686..0f2e9fc 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -772,14 +772,17 @@ EXPORT_SYMBOL(__dev_getfirstbyhwtype); > > struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type) > { > - struct net_device *dev; > + struct net_device *dev, *ret = NULL; > > - rtnl_lock(); > - dev = __dev_getfirstbyhwtype(net, type); > - if (dev) > - dev_hold(dev); > - rtnl_unlock(); > - return dev; > + rcu_read_lock(); > + for_each_netdev_rcu(net, dev) > + if (dev->type == type) { > + dev_hold(dev); > + ret = dev; > + break; > + } > + rcu_read_unlock(); > + return ret; > } > EXPORT_SYMBOL(dev_getfirstbyhwtype); > > > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dev_getfirstbyhwtype() optimization 2010-03-18 21:27 [PATCH] net: dev_getfirstbyhwtype() optimization Eric Dumazet 2010-03-18 21:54 ` Laurent Chavey @ 2010-03-19 2:32 ` Paul E. McKenney 2010-03-19 5:14 ` Eric Dumazet 2010-03-22 3:39 ` David Miller 2 siblings, 1 reply; 6+ messages in thread From: Paul E. McKenney @ 2010-03-19 2:32 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Miller, netdev On Thu, Mar 18, 2010 at 10:27:25PM +0100, Eric Dumazet wrote: > Use RCU to avoid RTNL use in dev_getfirstbyhwtype() > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > diff --git a/net/core/dev.c b/net/core/dev.c > index 17b1686..0f2e9fc 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -772,14 +772,17 @@ EXPORT_SYMBOL(__dev_getfirstbyhwtype); > > struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type) > { > - struct net_device *dev; > + struct net_device *dev, *ret = NULL; > > - rtnl_lock(); > - dev = __dev_getfirstbyhwtype(net, type); > - if (dev) > - dev_hold(dev); > - rtnl_unlock(); > - return dev; > + rcu_read_lock(); > + for_each_netdev_rcu(net, dev) > + if (dev->type == type) { > + dev_hold(dev); > + ret = dev; > + break; > + } > + rcu_read_unlock(); > + return ret; Looks good, but I don't understand how it helps to introduce the local variable "ret". Thanx, Paul > } > EXPORT_SYMBOL(dev_getfirstbyhwtype); > > > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dev_getfirstbyhwtype() optimization 2010-03-19 2:32 ` Paul E. McKenney @ 2010-03-19 5:14 ` Eric Dumazet 2010-03-19 11:54 ` Paul E. McKenney 0 siblings, 1 reply; 6+ messages in thread From: Eric Dumazet @ 2010-03-19 5:14 UTC (permalink / raw) To: paulmck; +Cc: David Miller, netdev Le jeudi 18 mars 2010 à 19:32 -0700, Paul E. McKenney a écrit : > On Thu, Mar 18, 2010 at 10:27:25PM +0100, Eric Dumazet wrote: > > Use RCU to avoid RTNL use in dev_getfirstbyhwtype() > > > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > --- > > diff --git a/net/core/dev.c b/net/core/dev.c > > index 17b1686..0f2e9fc 100644 > > --- a/net/core/dev.c > > +++ b/net/core/dev.c > > @@ -772,14 +772,17 @@ EXPORT_SYMBOL(__dev_getfirstbyhwtype); > > > > struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type) > > { > > - struct net_device *dev; > > + struct net_device *dev, *ret = NULL; > > > > - rtnl_lock(); > > - dev = __dev_getfirstbyhwtype(net, type); > > - if (dev) > > - dev_hold(dev); > > - rtnl_unlock(); > > - return dev; > > + rcu_read_lock(); > > + for_each_netdev_rcu(net, dev) > > + if (dev->type == type) { > > + dev_hold(dev); > > + ret = dev; > > + break; > > + } > > + rcu_read_unlock(); > > + return ret; > > Looks good, but I don't understand how it helps to introduce the > local variable "ret". > Thanks for reviewing Paul ! Not only it helps, its necessary :) for_each_netdev_rcu(net, dev) { if (cond) { dev_hold(dev); break; } } makes no guarantee dev is NULL if we hit the list end : /** * list_for_each_entry_rcu - iterate over rcu list of given type * @pos: the type * to use as a loop cursor. * @head: the head for your list. * @member: the name of the list_struct within the struct. * * This list-traversal primitive may safely run concurrently with * the _rcu list-mutation primitives such as list_add_rcu() * as long as the traversal is guarded by rcu_read_lock(). */ #define list_for_each_entry_rcu(pos, head, member) \ for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \ prefetch(pos->member.next), &pos->member != (head); \ pos = list_entry_rcu(pos->member.next, typeof(*pos), member)) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dev_getfirstbyhwtype() optimization 2010-03-19 5:14 ` Eric Dumazet @ 2010-03-19 11:54 ` Paul E. McKenney 0 siblings, 0 replies; 6+ messages in thread From: Paul E. McKenney @ 2010-03-19 11:54 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Miller, netdev On Fri, Mar 19, 2010 at 06:14:02AM +0100, Eric Dumazet wrote: > Le jeudi 18 mars 2010 à 19:32 -0700, Paul E. McKenney a écrit : > > On Thu, Mar 18, 2010 at 10:27:25PM +0100, Eric Dumazet wrote: > > > Use RCU to avoid RTNL use in dev_getfirstbyhwtype() > > > > > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > > > --- > > > diff --git a/net/core/dev.c b/net/core/dev.c > > > index 17b1686..0f2e9fc 100644 > > > --- a/net/core/dev.c > > > +++ b/net/core/dev.c > > > @@ -772,14 +772,17 @@ EXPORT_SYMBOL(__dev_getfirstbyhwtype); > > > > > > struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type) > > > { > > > - struct net_device *dev; > > > + struct net_device *dev, *ret = NULL; > > > > > > - rtnl_lock(); > > > - dev = __dev_getfirstbyhwtype(net, type); > > > - if (dev) > > > - dev_hold(dev); > > > - rtnl_unlock(); > > > - return dev; > > > + rcu_read_lock(); > > > + for_each_netdev_rcu(net, dev) > > > + if (dev->type == type) { > > > + dev_hold(dev); > > > + ret = dev; > > > + break; > > > + } > > > + rcu_read_unlock(); > > > + return ret; > > > > Looks good, but I don't understand how it helps to introduce the > > local variable "ret". > > > > Thanks for reviewing Paul ! > > Not only it helps, its necessary :) > > for_each_netdev_rcu(net, dev) { > if (cond) { > dev_hold(dev); > break; > } > } > makes no guarantee dev is NULL if we hit the list end : > > > /** > * list_for_each_entry_rcu - iterate over rcu list of given type > * @pos: the type * to use as a loop cursor. > * @head: the head for your list. > * @member: the name of the list_struct within the struct. > * > * This list-traversal primitive may safely run concurrently with > * the _rcu list-mutation primitives such as list_add_rcu() > * as long as the traversal is guarded by rcu_read_lock(). > */ > #define list_for_each_entry_rcu(pos, head, member) \ > for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \ > prefetch(pos->member.next), &pos->member != (head); \ > pos = list_entry_rcu(pos->member.next, typeof(*pos), member)) Right! Got it, thank you! Thanx, Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: dev_getfirstbyhwtype() optimization 2010-03-18 21:27 [PATCH] net: dev_getfirstbyhwtype() optimization Eric Dumazet 2010-03-18 21:54 ` Laurent Chavey 2010-03-19 2:32 ` Paul E. McKenney @ 2010-03-22 3:39 ` David Miller 2 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2010-03-22 3:39 UTC (permalink / raw) To: eric.dumazet; +Cc: netdev From: Eric Dumazet <eric.dumazet@gmail.com> Date: Thu, 18 Mar 2010 22:27:25 +0100 > Use RCU to avoid RTNL use in dev_getfirstbyhwtype() > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Applied, thanks Eric. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-22 3:39 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-03-18 21:27 [PATCH] net: dev_getfirstbyhwtype() optimization Eric Dumazet 2010-03-18 21:54 ` Laurent Chavey 2010-03-19 2:32 ` Paul E. McKenney 2010-03-19 5:14 ` Eric Dumazet 2010-03-19 11:54 ` Paul E. McKenney 2010-03-22 3:39 ` 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).