* [RFC PATCH 0/3] net: Improve snmp6_fill_stats
@ 2016-08-08 10:22 Jia He
2016-08-08 10:22 ` [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64 Jia He
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jia He @ 2016-08-08 10:22 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Jia He
This is the follow up work of commit a3a773726c9f ("net: Optimize
snmp stat aggregation by walking all the percpu data at once")
Jia He (3):
net: Remove unnecessary memset in __snmp6_fill_stats64
net: Replace for_each_possible_cpu with for_each_online_cpu
net: Remove the useless parameter of __snmp6_fill_statsdev
net/ipv6/addrconf.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64
2016-08-08 10:22 [RFC PATCH 0/3] net: Improve snmp6_fill_stats Jia He
@ 2016-08-08 10:22 ` Jia He
2016-08-08 11:12 ` Florian Westphal
2016-08-09 10:12 ` Eric Dumazet
2016-08-08 10:22 ` [RFC PATCH 2/3] net: Replace for_each_possible_cpu with for_each_online_cpu Jia He
2016-08-08 10:22 ` [RFC PATCH 3/3] net: Remove the useless parameter of __snmp6_fill_statsdev Jia He
2 siblings, 2 replies; 9+ messages in thread
From: Jia He @ 2016-08-08 10:22 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Jia He, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
buff[] will be assigned later, so memset is not necessary.
Signed-off-by: Jia He <hejianet@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
---
net/ipv6/addrconf.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ab3e796..43fa8d0 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4967,7 +4967,6 @@ static inline void __snmp6_fill_stats64(u64 *stats, void __percpu *mib,
BUG_ON(pad < 0);
- memset(buff, 0, sizeof(buff));
buff[0] = IPSTATS_MIB_MAX;
for_each_possible_cpu(c) {
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH 2/3] net: Replace for_each_possible_cpu with for_each_online_cpu
2016-08-08 10:22 [RFC PATCH 0/3] net: Improve snmp6_fill_stats Jia He
2016-08-08 10:22 ` [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64 Jia He
@ 2016-08-08 10:22 ` Jia He
2016-08-08 17:59 ` David Miller
2016-08-09 10:10 ` Eric Dumazet
2016-08-08 10:22 ` [RFC PATCH 3/3] net: Remove the useless parameter of __snmp6_fill_statsdev Jia He
2 siblings, 2 replies; 9+ messages in thread
From: Jia He @ 2016-08-08 10:22 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Jia He, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
In PowerPC server with large number cpus, the loop index in smt=1 could be
reduced to 1/8 compared with smt=8.
Thus cache misses can be reduced.
Signed-off-by: Jia He <hejianet@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
---
net/ipv6/addrconf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 43fa8d0..1fce613 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4969,7 +4969,7 @@ static inline void __snmp6_fill_stats64(u64 *stats, void __percpu *mib,
buff[0] = IPSTATS_MIB_MAX;
- for_each_possible_cpu(c) {
+ for_each_online_cpu(c) {
for (i = 1; i < IPSTATS_MIB_MAX; i++)
buff[i] += snmp_get_cpu_field64(mib, c, i, syncpoff);
}
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH 3/3] net: Remove the useless parameter of __snmp6_fill_statsdev
2016-08-08 10:22 [RFC PATCH 0/3] net: Improve snmp6_fill_stats Jia He
2016-08-08 10:22 ` [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64 Jia He
2016-08-08 10:22 ` [RFC PATCH 2/3] net: Replace for_each_possible_cpu with for_each_online_cpu Jia He
@ 2016-08-08 10:22 ` Jia He
2 siblings, 0 replies; 9+ messages in thread
From: Jia He @ 2016-08-08 10:22 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Jia He, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
In commit a3a773726c9f ("net: Optimize snmp stat aggregation by walking
all the percpu data at once"), __snmp6_fill_stats64 had been optimized
by removing parameter items, so do the same for __snmp6_fill_statsdev.
Signed-off-by: Jia He <hejianet@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: James Morris <jmorris@namei.org>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
---
net/ipv6/addrconf.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1fce613..37ea2bb 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4944,18 +4944,18 @@ static inline size_t inet6_if_nlmsg_size(void)
}
static inline void __snmp6_fill_statsdev(u64 *stats, atomic_long_t *mib,
- int items, int bytes)
+ int bytes)
{
int i;
- int pad = bytes - sizeof(u64) * items;
+ int pad = bytes - sizeof(u64) * ICMP6_MIB_MAX;
BUG_ON(pad < 0);
/* Use put_unaligned() because stats may not be aligned for u64. */
- put_unaligned(items, &stats[0]);
- for (i = 1; i < items; i++)
+ put_unaligned(ICMP6_MIB_MAX, &stats[0]);
+ for (i = 1; i < ICMP6_MIB_MAX; i++)
put_unaligned(atomic_long_read(&mib[i]), &stats[i]);
- memset(&stats[items], 0, pad);
+ memset(&stats[ICMP6_MIB_MAX], 0, pad);
}
static inline void __snmp6_fill_stats64(u64 *stats, void __percpu *mib,
@@ -4987,7 +4987,7 @@ static void snmp6_fill_stats(u64 *stats, struct inet6_dev *idev, int attrtype,
offsetof(struct ipstats_mib, syncp));
break;
case IFLA_INET6_ICMP6STATS:
- __snmp6_fill_statsdev(stats, idev->stats.icmpv6dev->mibs, ICMP6_MIB_MAX, bytes);
+ __snmp6_fill_statsdev(stats, idev->stats.icmpv6dev->mibs, bytes);
break;
}
}
--
2.5.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64
2016-08-08 10:22 ` [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64 Jia He
@ 2016-08-08 11:12 ` Florian Westphal
2016-08-08 13:04 ` hejianet
2016-08-09 10:12 ` Eric Dumazet
1 sibling, 1 reply; 9+ messages in thread
From: Florian Westphal @ 2016-08-08 11:12 UTC (permalink / raw)
To: Jia He
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
Jia He <hejianet@gmail.com> wrote:
> buff[] will be assigned later, so memset is not necessary.
>
> Signed-off-by: Jia He <hejianet@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: James Morris <jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy <kaber@trash.net>
> ---
> net/ipv6/addrconf.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index ab3e796..43fa8d0 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -4967,7 +4967,6 @@ static inline void __snmp6_fill_stats64(u64 *stats, void __percpu *mib,
>
> BUG_ON(pad < 0);
>
> - memset(buff, 0, sizeof(buff));
> buff[0] = IPSTATS_MIB_MAX;
>
> for_each_possible_cpu(c) {
for (i = 1; i < IPSTATS_MIB_MAX; i++)
buff[i] += snmp_get_cpu_field64(mib, c, i, syncpoff);
Without memset result of buff[i] += ... is undefined.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64
2016-08-08 11:12 ` Florian Westphal
@ 2016-08-08 13:04 ` hejianet
0 siblings, 0 replies; 9+ messages in thread
From: hejianet @ 2016-08-08 13:04 UTC (permalink / raw)
To: Florian Westphal
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
Yes, sorry about it,I am too hasty
B.R.
Jia He
On 8/8/16 7:12 PM, Florian Westphal wrote:
> Jia He <hejianet@gmail.com> wrote:
>> buff[] will be assigned later, so memset is not necessary.
>>
>> Signed-off-by: Jia He <hejianet@gmail.com>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
>> Cc: James Morris <jmorris@namei.org>
>> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
>> Cc: Patrick McHardy <kaber@trash.net>
>> ---
>> net/ipv6/addrconf.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index ab3e796..43fa8d0 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -4967,7 +4967,6 @@ static inline void __snmp6_fill_stats64(u64 *stats, void __percpu *mib,
>>
>> BUG_ON(pad < 0);
>>
>> - memset(buff, 0, sizeof(buff));
>> buff[0] = IPSTATS_MIB_MAX;
>>
>> for_each_possible_cpu(c) {
> for (i = 1; i < IPSTATS_MIB_MAX; i++)
> buff[i] += snmp_get_cpu_field64(mib, c, i, syncpoff);
>
> Without memset result of buff[i] += ... is undefined.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 2/3] net: Replace for_each_possible_cpu with for_each_online_cpu
2016-08-08 10:22 ` [RFC PATCH 2/3] net: Replace for_each_possible_cpu with for_each_online_cpu Jia He
@ 2016-08-08 17:59 ` David Miller
2016-08-09 10:10 ` Eric Dumazet
1 sibling, 0 replies; 9+ messages in thread
From: David Miller @ 2016-08-08 17:59 UTC (permalink / raw)
To: hejianet; +Cc: netdev, linux-kernel, kuznet, jmorris, yoshfuji, kaber
From: Jia He <hejianet@gmail.com>
Date: Mon, 8 Aug 2016 18:22:21 +0800
> In PowerPC server with large number cpus, the loop index in smt=1 could be
> reduced to 1/8 compared with smt=8.
> Thus cache misses can be reduced.
You can't do this, if cpus go down we still want to report the statistics
they collected while they were up.
So we must use the possible cpu list here.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 2/3] net: Replace for_each_possible_cpu with for_each_online_cpu
2016-08-08 10:22 ` [RFC PATCH 2/3] net: Replace for_each_possible_cpu with for_each_online_cpu Jia He
2016-08-08 17:59 ` David Miller
@ 2016-08-09 10:10 ` Eric Dumazet
1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2016-08-09 10:10 UTC (permalink / raw)
To: Jia He
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
On Mon, 2016-08-08 at 18:22 +0800, Jia He wrote:
> In PowerPC server with large number cpus, the loop index in smt=1 could be
> reduced to 1/8 compared with smt=8.
> Thus cache misses can be reduced.
>
> Signed-off-by: Jia He <hejianet@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: James Morris <jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy <kaber@trash.net>
> ---
> net/ipv6/addrconf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 43fa8d0..1fce613 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -4969,7 +4969,7 @@ static inline void __snmp6_fill_stats64(u64 *stats, void __percpu *mib,
>
> buff[0] = IPSTATS_MIB_MAX;
>
> - for_each_possible_cpu(c) {
> + for_each_online_cpu(c) {
> for (i = 1; i < IPSTATS_MIB_MAX; i++)
> buff[i] += snmp_get_cpu_field64(mib, c, i, syncpoff);
> }
This will break on machines with cpu hotplug.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64
2016-08-08 10:22 ` [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64 Jia He
2016-08-08 11:12 ` Florian Westphal
@ 2016-08-09 10:12 ` Eric Dumazet
1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2016-08-09 10:12 UTC (permalink / raw)
To: Jia He
Cc: netdev, linux-kernel, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy
On Mon, 2016-08-08 at 18:22 +0800, Jia He wrote:
> buff[] will be assigned later, so memset is not necessary.
>
> Signed-off-by: Jia He <hejianet@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: James Morris <jmorris@namei.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
> Cc: Patrick McHardy <kaber@trash.net>
> ---
> net/ipv6/addrconf.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index ab3e796..43fa8d0 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -4967,7 +4967,6 @@ static inline void __snmp6_fill_stats64(u64 *stats, void __percpu *mib,
>
> BUG_ON(pad < 0);
>
> - memset(buff, 0, sizeof(buff));
> buff[0] = IPSTATS_MIB_MAX;
>
> for_each_possible_cpu(c) {
This is completely buggy, since we performs additions, not assignments :
buff[i] += snmp_get_cpu_field64(mib, c, i, syncpoff);
Please do not send untested patches.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-08-09 10:12 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-08 10:22 [RFC PATCH 0/3] net: Improve snmp6_fill_stats Jia He
2016-08-08 10:22 ` [RFC PATCH 1/3] net: Remove unnecessary memset in __snmp6_fill_stats64 Jia He
2016-08-08 11:12 ` Florian Westphal
2016-08-08 13:04 ` hejianet
2016-08-09 10:12 ` Eric Dumazet
2016-08-08 10:22 ` [RFC PATCH 2/3] net: Replace for_each_possible_cpu with for_each_online_cpu Jia He
2016-08-08 17:59 ` David Miller
2016-08-09 10:10 ` Eric Dumazet
2016-08-08 10:22 ` [RFC PATCH 3/3] net: Remove the useless parameter of __snmp6_fill_statsdev Jia He
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.