LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH 03/18] ipvs: zero percpu stats

To: Simon Horman <horms@xxxxxxxxxxxx>
Subject: Re: [PATCH 03/18] ipvs: zero percpu stats
Cc: netdev@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, netfilter@xxxxxxxxxxxxxxx, lvs-devel@xxxxxxxxxxxxxxx, Julian Anastasov <ja@xxxxxx>, Hans Schillstrom <hans@xxxxxxxxxxxxxxx>
From: Eric Dumazet <eric.dumazet@xxxxxxxxx>
Date: Sun, 06 Mar 2011 10:06:07 +0100
Le dimanche 06 mars 2011 à 08:45 +0900, Simon Horman a écrit :
> From: Julian Anastasov <ja@xxxxxx>
> 
>       Zero the new percpu stats because we copy from there.
> 
> Signed-off-by: Julian Anastasov <ja@xxxxxx>
> Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index a2a67ad..fd74527 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -715,8 +715,25 @@ static void ip_vs_trash_cleanup(struct net *net)
>  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 we are the only reader.
> +              */
> +             do {
> +                     start = u64_stats_fetch_begin(&u->syncp);
> +                     memset(&u->ustats, 0, sizeof(u->ustats));
> +             } while (u64_stats_fetch_retry(&u->syncp, start));


Sorry this makes no sense to me.

This code _is_ a writer, and hardly a hot path.

Why try to pretend its a reader and confuse people ?

Either :

- Another writer can modify the counters in same time, and we must
synchronize with them (we are a writer after all)

- Another reader can read the counters in same time, and we must let
them catch we mihjt have cleared half of their values.

- No reader or writer can access data, no synch is needed, a pure
memset() is OK.


> +     }
> +
>       memset(&stats->ustats, 0, sizeof(stats->ustats));
>       ip_vs_zero_estimator(stats);
>  


--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

<Prev in Thread] Current Thread [Next in Thread>