Re: [PATCH] net: sched: keep __gnet_stats_copy_xxx() same semantics for percpu stats

[Cong Wang <xiyou.wangcong@gmail.com>]

On Wed, Nov 27, 2019 at 10:31 PM Dust Li <dust.li@linux.alibaba.com> wrote:
>
> __gnet_stats_copy_basic/queue() support both percpu stat and
> non-percpu stat, but they are handle in a different manner:
> 1. For percpu stat, percpu stats are added to the return value;
> 2. For non-percpu stat, non-percpu stats will overwrite the
>    return value;
> We should keep the same semantics for both type.
>
> This patch makes percpu stats follow non-percpu's manner by
> reset the return bstats before add the percpu bstats to it.
> Also changes the caller in sch_mq.c/sch_mqprio.c to make sure
> they dump the right statistics for percpu qdisc.
>
> One more thing, the sch->q.qlen is not set with nonlock child
> qdisc in mq_dump()/mqprio_dump(), add that.
>
> Fixes: 22e0f8b9322c ("net: sched: make bstats per cpu and estimator RCU safe")
> Signed-off-by: Dust Li <dust.li@linux.alibaba.com>
> Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
> ---
>  net/core/gen_stats.c   |  2 ++
>  net/sched/sch_mq.c     | 34 ++++++++++++++++------------------
>  net/sched/sch_mqprio.c | 35 +++++++++++++++++------------------
>  3 files changed, 35 insertions(+), 36 deletions(-)
>
> diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
> index 1d653fbfcf52..d71af69196c9 100644
> --- a/net/core/gen_stats.c
> +++ b/net/core/gen_stats.c
> @@ -120,6 +120,7 @@ __gnet_stats_copy_basic_cpu(struct gnet_stats_basic_packed *bstats,
>  {
>         int i;
>
> +       memset(bstats, 0, sizeof(*bstats));
>         for_each_possible_cpu(i) {
>                 struct gnet_stats_basic_cpu *bcpu = per_cpu_ptr(cpu, i);
>                 unsigned int start;
> @@ -288,6 +289,7 @@ __gnet_stats_copy_queue_cpu(struct gnet_stats_queue *qstats,
>  {
>         int i;
>
> +       memset(qstats, 0, sizeof(*qstats));


I think its caller is responsible to clear the stats, so you don't need to
clear them here? It looks like you do memset() twice.

Does this patch fix any bug? It looks more like a clean up to me, if so
please mark it for net-next.

Thanks.