From: Simon Horman <horms@verge.net.au>
To: Julian Anastasov <ja@ssi.bg>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
netdev@vger.kernel.org, netfilter-devel@vger.kernel.org,
netfilter@vger.kernel.org, lvs-devel@vger.kernel.org,
Hans Schillstrom <hans@schillstrom.com>
Subject: Re: [PATCH 03/18] ipvs: zero percpu stats
Date: Thu, 10 Mar 2011 10:34:42 +0900 [thread overview]
Message-ID: <20110310013442.GD3028@verge.net.au> (raw)
In-Reply-To: <alpine.LFD.2.00.1103061218470.1879@ja.ssi.bg>
On Sun, Mar 06, 2011 at 02:18:35PM +0200, Julian Anastasov wrote:
[ snip ]
> Zero the new percpu stats because we copy from there.
> Use the stats spin lock to synchronize the percpu zeroing with
> the percpu reading, both in user context and not in a hot path.
>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
Eric, do you have any thoughts on this?
It seems clean to me.
> ---
>
> diff -urp lvs-test-2.6-8a80c79/linux/net/netfilter/ipvs/ip_vs_ctl.c linux/net/netfilter/ipvs/ip_vs_ctl.c
> --- lvs-test-2.6-8a80c79/linux/net/netfilter/ipvs/ip_vs_ctl.c 2011-03-06 13:39:59.000000000 +0200
> +++ linux/net/netfilter/ipvs/ip_vs_ctl.c 2011-03-06 13:44:56.108275455 +0200
> @@ -713,8 +713,25 @@ static void ip_vs_trash_cleanup(struct n
> static void
> ip_vs_zero_stats(struct ip_vs_stats *stats)
> {
> + struct ip_vs_cpu_stats *cpustats = stats->cpustats;
> + int i;
> +
> spin_lock_bh(&stats->lock);
>
> + for_each_possible_cpu(i) {
> + struct ip_vs_cpu_stats *u = per_cpu_ptr(cpustats, i);
> + unsigned int start;
> +
> + /* Do not pretend to be writer, it is enough to
> + * sync with writers that modify the u64 counters
> + * because under stats->lock there are no other readers
> + */
> + do {
> + start = u64_stats_fetch_begin(&u->syncp);
> + memset(&u->ustats, 0, sizeof(u->ustats));
> + } while (u64_stats_fetch_retry(&u->syncp, start));
> + }
> +
> memset(&stats->ustats, 0, sizeof(stats->ustats));
> ip_vs_zero_estimator(stats);
>
> @@ -2015,16 +2032,19 @@ static int ip_vs_stats_percpu_show(struc
> seq_printf(seq,
> "CPU Conns Packets Packets Bytes Bytes\n");
>
> + /* Use spin lock early to synchronize with percpu zeroing */
> + spin_lock_bh(&tot_stats->lock);
> +
> for_each_possible_cpu(i) {
> struct ip_vs_cpu_stats *u = per_cpu_ptr(cpustats, i);
> unsigned int start;
> __u64 inbytes, outbytes;
>
> do {
> - start = u64_stats_fetch_begin_bh(&u->syncp);
> + start = u64_stats_fetch_begin(&u->syncp);
> inbytes = u->ustats.inbytes;
> outbytes = u->ustats.outbytes;
> - } while (u64_stats_fetch_retry_bh(&u->syncp, start));
> + } while (u64_stats_fetch_retry(&u->syncp, start));
>
> seq_printf(seq, "%3X %8X %8X %8X %16LX %16LX\n",
> i, u->ustats.conns, u->ustats.inpkts,
> @@ -2032,7 +2052,6 @@ static int ip_vs_stats_percpu_show(struc
> (__u64)outbytes);
> }
>
> - spin_lock_bh(&tot_stats->lock);
> seq_printf(seq, " ~ %8X %8X %8X %16LX %16LX\n\n",
> tot_stats->ustats.conns, tot_stats->ustats.inpkts,
> tot_stats->ustats.outpkts,
>
next prev parent reply other threads:[~2011-03-10 1:34 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-05 23:45 [patch v2 ] IPVS: Conditionally include sysctl code Simon Horman
2011-03-05 23:45 ` [PATCH 01/18] ipvs: move struct netns_ipvs Simon Horman
2011-03-05 23:45 ` [PATCH 02/18] ipvs: reorganize tot_stats Simon Horman
2011-03-05 23:45 ` [PATCH 03/18] ipvs: zero percpu stats Simon Horman
2011-03-06 9:06 ` Eric Dumazet
2011-03-06 12:18 ` Julian Anastasov
2011-03-10 1:34 ` Simon Horman [this message]
2011-03-10 2:53 ` David Miller
2011-03-10 4:27 ` Simon Horman
2011-03-13 10:57 ` Eric Dumazet
2011-03-13 23:29 ` Julian Anastasov
2011-03-05 23:45 ` [PATCH 04/18] ipvs: remove unused seqcount stats Simon Horman
2011-03-05 23:45 ` [PATCH 05/18] IPVS: Add ip_vs_route_me_harder() Simon Horman
2011-03-05 23:45 ` [PATCH 06/18] IPVS: Add sysctl_snat_reroute() Simon Horman
2011-03-05 23:45 ` [PATCH 07/18] IPVS: Add sysctl_nat_icmp_send() Simon Horman
2011-03-05 23:45 ` [PATCH 08/18] IPVS: Add {sysctl_sync_threshold,period}() Simon Horman
2011-03-05 23:45 ` [PATCH 09/18] IPVS: Add sysctl_sync_ver() Simon Horman
2011-03-05 23:45 ` [PATCH 10/18] IPVS: Add sysctl_expire_nodest_conn() Simon Horman
2011-03-05 23:45 ` [PATCH 11/18] IPVS: Add expire_quiescent_template() Simon Horman
2011-03-05 23:45 ` [PATCH 12/18] IPVS: Conditinally use sysctl_lblc{r}_expiration Simon Horman
2011-03-05 23:45 ` [PATCH 13/18] IPVS: ip_vs_todrop() becomes a noop when CONFIG_SYSCTL is undefined Simon Horman
2011-03-05 23:45 ` [PATCH 14/18] IPVS: Conditional ip_vs_conntrack_enabled() Simon Horman
2011-03-05 23:45 ` [PATCH 15/18] IPVS: Minimise ip_vs_leave when CONFIG_SYSCTL is undefined Simon Horman
2011-03-05 23:45 ` [PATCH 16/18] IPVS: Conditionally define and use ip_vs_lblc{r}_table Simon Horman
2011-03-05 23:45 ` [PATCH 17/18] IPVS: Add __ip_vs_control_{init,cleanup}_sysctl() Simon Horman
2011-03-05 23:46 ` [PATCH 18/18] IPVS: Conditionally include sysctl members of struct netns_ipvs Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110310013442.GD3028@verge.net.au \
--to=horms@verge.net.au \
--cc=eric.dumazet@gmail.com \
--cc=hans@schillstrom.com \
--cc=ja@ssi.bg \
--cc=lvs-devel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=netfilter@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.