* [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).