| Message ID | 1566454953-29321-1-git-send-email-lirongqing@baidu.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] net: drop_monitor: change the stats variable to u64 in net_dm_stats_put | expand |
On Thu, Aug 22, 2019 at 02:22:33PM +0800, Li RongQing wrote: > only the element drop of struct net_dm_stats is used, so simplify it to u64 Thanks for the patch, but I don't really see the value here. The struct allows for easy extensions in the future. What do you gain from this change? We merely read stats and report them to user space, so I guess it's not about performance either. > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > --- > net/core/drop_monitor.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c > index bfc024024aa3..ed10a40cf629 100644 > --- a/net/core/drop_monitor.c > +++ b/net/core/drop_monitor.c > @@ -1329,11 +1329,11 @@ static int net_dm_cmd_config_get(struct sk_buff *skb, struct genl_info *info) > return rc; > } > > -static void net_dm_stats_read(struct net_dm_stats *stats) > +static void net_dm_stats_read(u64 *stats) > { > int cpu; > > - memset(stats, 0, sizeof(*stats)); > + *stats = 0; > for_each_possible_cpu(cpu) { > struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu); > struct net_dm_stats *cpu_stats = &data->stats; > @@ -1345,14 +1345,14 @@ static void net_dm_stats_read(struct net_dm_stats *stats) > dropped = cpu_stats->dropped; > } while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start)); > > - stats->dropped += dropped; > + *stats += dropped; > } > } > > static int net_dm_stats_put(struct sk_buff *msg) > { > - struct net_dm_stats stats; > struct nlattr *attr; > + u64 stats; > > net_dm_stats_read(&stats); > > @@ -1361,7 +1361,7 @@ static int net_dm_stats_put(struct sk_buff *msg) > return -EMSGSIZE; > > if (nla_put_u64_64bit(msg, NET_DM_ATTR_STATS_DROPPED, > - stats.dropped, NET_DM_ATTR_PAD)) > + stats, NET_DM_ATTR_PAD)) > goto nla_put_failure; > > nla_nest_end(msg, attr); > -- > 2.16.2 >
> -----邮件原件----- > 发件人: Ido Schimmel [mailto:idosch@idosch.org] > 发送时间: 2019年8月22日 20:00 > 收件人: Li,Rongqing <lirongqing@baidu.com> > 抄送: netdev@vger.kernel.org; idosch@mellanox.com > 主题: Re: [PATCH][net-next] net: drop_monitor: change the stats variable to > u64 in net_dm_stats_put > > On Thu, Aug 22, 2019 at 02:22:33PM +0800, Li RongQing wrote: > > only the element drop of struct net_dm_stats is used, so simplify it > > to u64 > > Thanks for the patch, but I don't really see the value here. The struct allows for > easy extensions in the future. What do you gain from this change? We merely > read stats and report them to user space, so I guess it's not about > performance either. > I think u64 can reduce to call memset and dereference stats.drop If it is for future, keep it -RongQing
From: Li,Rongqing > Sent: 22 August 2019 13:32 > > On Thu, Aug 22, 2019 at 02:22:33PM +0800, Li RongQing wrote: > > > only the element drop of struct net_dm_stats is used, so simplify it > > > to u64 > > > > Thanks for the patch, but I don't really see the value here. The struct allows for > > easy extensions in the future. What do you gain from this change? We merely > > read stats and report them to user space, so I guess it's not about > > performance either. > > > > I think u64 can reduce to call memset and dereference stats.drop The compiler should inline the memset(). Also you should have called it 'dropped' not 'stats'. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c index bfc024024aa3..ed10a40cf629 100644 --- a/net/core/drop_monitor.c +++ b/net/core/drop_monitor.c @@ -1329,11 +1329,11 @@ static int net_dm_cmd_config_get(struct sk_buff *skb, struct genl_info *info) return rc; } -static void net_dm_stats_read(struct net_dm_stats *stats) +static void net_dm_stats_read(u64 *stats) { int cpu; - memset(stats, 0, sizeof(*stats)); + *stats = 0; for_each_possible_cpu(cpu) { struct per_cpu_dm_data *data = &per_cpu(dm_cpu_data, cpu); struct net_dm_stats *cpu_stats = &data->stats; @@ -1345,14 +1345,14 @@ static void net_dm_stats_read(struct net_dm_stats *stats) dropped = cpu_stats->dropped; } while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start)); - stats->dropped += dropped; + *stats += dropped; } } static int net_dm_stats_put(struct sk_buff *msg) { - struct net_dm_stats stats; struct nlattr *attr; + u64 stats; net_dm_stats_read(&stats); @@ -1361,7 +1361,7 @@ static int net_dm_stats_put(struct sk_buff *msg) return -EMSGSIZE; if (nla_put_u64_64bit(msg, NET_DM_ATTR_STATS_DROPPED, - stats.dropped, NET_DM_ATTR_PAD)) + stats, NET_DM_ATTR_PAD)) goto nla_put_failure; nla_nest_end(msg, attr);
only the element drop of struct net_dm_stats is used, so simplify it to u64 Signed-off-by: Li RongQing <lirongqing@baidu.com> --- net/core/drop_monitor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)