| Message ID | 1318413446-22258-1-git-send-email-mmaruseac@ixiacom.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, 2011-10-12 at 02:57 -0700, Mihai Maruseac wrote: > Instead of using the dev->next chain and trying to resync at each call to > dev_seq_start, use the ifindex, keeping the last index in seq->private field. Please consider only my last patch email, the first one was missing Signed-off-by line. Thanks, Mihai -- 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
On Wed, 12 Oct 2011 12:57:26 +0300 Mihai Maruseac <mihai.maruseac@gmail.com> wrote: It looks like you lost the ability to seek back and read the header (start token). How is the header handled, is possible to rewind the file and read it over again? > +static inline struct net_device *next_dev(struct seq_file *seq, loff_t *pos) > +{ > + struct dev_iter_state *state = seq->private; > + struct net *net = seq_file_net(seq); > + struct net_device *dev = NULL; > + loff_t off; > + > + ++*pos; > + dev = dev_get_by_index_rcu(net, state->ifindex); Looks good a couple of minor nits. 1. The function should not be inline, since it is in no way performance critical. The compiler will probably inline it anyway. 2. dev does not have to be initialized since it is assigned a few lines later. Most programmers are trained now to always initialize variables, but often it is unnecessary. 3. The name next_dev() is a little generic; maybe a better name. -- 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
On Wed, Oct 12, 2011 at 6:50 PM, Stephen Hemminger <shemminger@vyatta.com> wrote: > On Wed, 12 Oct 2011 12:57:26 +0300 > Mihai Maruseac <mihai.maruseac@gmail.com> wrote: > > It looks like you lost the ability to seek back and read the header > (start token). How is the header handled, is possible to rewind > the file and read it over again? We tested with a simple program reading a part of the /proc file and then doing a seek to the start and rereading. It worked with latest submitted patch. >> +static inline struct net_device *next_dev(struct seq_file *seq, loff_t *pos) >> +{ >> + struct dev_iter_state *state = seq->private; >> + struct net *net = seq_file_net(seq); >> + struct net_device *dev = NULL; >> + loff_t off; >> + >> + ++*pos; >> + dev = dev_get_by_index_rcu(net, state->ifindex); > > Looks good a couple of minor nits. > > 1. The function should not be inline, since it is in no way performance > critical. The compiler will probably inline it anyway. > > 2. dev does not have to be initialized since it is assigned a few > lines later. Most programmers are trained now to always initialize > variables, but often it is unnecessary. > > 3. The name next_dev() is a little generic; maybe a better name. > Fixed all of them, Thanks,
diff --git a/net/core/dev.c b/net/core/dev.c index 70ecb86..3ca0df8 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -4041,6 +4041,35 @@ static int dev_ifconf(struct net *net, char __user *arg) } #ifdef CONFIG_PROC_FS + +struct dev_iter_state { + struct seq_net_private p; + int ifindex; +}; + +static inline struct net_device *next_dev(struct seq_file *seq, loff_t *pos) +{ + struct dev_iter_state *state = seq->private; + struct net *net = seq_file_net(seq); + struct net_device *dev = NULL; + loff_t off; + + ++*pos; + dev = dev_get_by_index_rcu(net, state->ifindex); + state->ifindex++; + if (likely(dev)) + return dev; + + off = 1; + for_each_netdev_rcu(net, dev) + if (off++ == *pos) { + state->ifindex = dev->ifindex + 1; + return dev; + } + + return NULL; +} + /* * This is invoked by the /proc filesystem handler to display a device * in detail. @@ -4048,33 +4077,15 @@ static int dev_ifconf(struct net *net, char __user *arg) void *dev_seq_start(struct seq_file *seq, loff_t *pos) __acquires(RCU) { - struct net *net = seq_file_net(seq); - loff_t off; - struct net_device *dev; - rcu_read_lock(); if (!*pos) return SEQ_START_TOKEN; - - off = 1; - for_each_netdev_rcu(net, dev) - if (off++ == *pos) - return dev; - - return NULL; + return next_dev(seq, pos); } void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos) { - struct net_device *dev = v; - - if (v == SEQ_START_TOKEN) - dev = first_net_device_rcu(seq_file_net(seq)); - else - dev = next_net_device_rcu(dev); - - ++*pos; - return dev; + return next_dev(seq, pos); } void dev_seq_stop(struct seq_file *seq, void *v) @@ -4173,7 +4184,7 @@ static const struct seq_operations dev_seq_ops = { static int dev_seq_open(struct inode *inode, struct file *file) { return seq_open_net(inode, file, &dev_seq_ops, - sizeof(struct seq_net_private)); + sizeof(struct dev_iter_state)); } static const struct file_operations dev_seq_fops = {
Instead of using the dev->next chain and trying to resync at each call to dev_seq_start, use the ifindex, keeping the last index in seq->private field. Tests revealed the following results for ifconfig > /dev/null * 1000 interfaces: * 0.114s without patch * 0.089s with patch * 3000 interfaces: * 0.489s without patch * 0.110s with patch * 5000 interfaces: * 1.363s without patch * 0.250s with patch * 128000 interfaces (other setup): * ~100s without patch * ~30s with patch Signed-off-by: Mihai Maruseac <mmaruseac@ixiacom.com> --- net/core/dev.c | 53 ++++++++++++++++++++++++++++++++--------------------- 1 files changed, 32 insertions(+), 21 deletions(-)