[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 06 Jun 2016 16:53:19 -0700
From: Eric Dumazet <eric.dumazet@...il.com>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: Eric Dumazet <edumazet@...gle.com>,
"David S . Miller" <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>,
Jamal Hadi Salim <jhs@...atatu.com>,
John Fastabend <john.fastabend@...il.com>,
Kevin Athey <kda@...gle.com>,
Xiaotian Pei <xiaotian@...gle.com>
Subject: Re: [PATCH net-next 2/2] net: sched: do not acquire qdisc spinlock
in qdisc/class stats dump
On Mon, 2016-06-06 at 16:15 -0700, Cong Wang wrote:
> On Mon, Jun 6, 2016 at 9:37 AM, Eric Dumazet <edumazet@...gle.com> wrote:
> > void
> > -__gnet_stats_copy_basic(struct gnet_stats_basic_packed *bstats,
> > +__gnet_stats_copy_basic(const seqcount_t *running,
> > + struct gnet_stats_basic_packed *bstats,
> > struct gnet_stats_basic_cpu __percpu *cpu,
> > struct gnet_stats_basic_packed *b)
> > {
> > + unsigned int seq;
> > +
> > if (cpu) {
> > __gnet_stats_copy_basic_cpu(bstats, cpu);
> > - } else {
> > + return;
> > + }
> > + do {
> > + if (running)
> > + seq = read_seqcount_begin(running);
> > bstats->bytes = b->bytes;
> > bstats->packets = b->packets;
> > - }
> > + } while (running && read_seqcount_retry(running, seq));
> > }
>
> Why only these basic stats need to get read seqlock?
(seqcount)
> Queue stats (gnet_stats_copy_queue()) too, right?
All these values are 32bit values, right ?
struct gnet_stats_queue {
__u32 qlen;
__u32 backlog;
__u32 drops;
__u32 requeues;
__u32 overlimits;
};
Really sounds overkill to care about these, as probably no one needs to
get a 'consistent view of all these counters in a snapshot'.
Even as of today, the qlen/backlog pair is wrong. No one ever used these
values in an SNMP agent.
Note that qlen/backlog is changed both by enqueue/dequeue, so the
seqcount protection would not work.
With the percpu stats thing, stats can not be fetched in a 'consistent'
way.
Powered by blists - more mailing lists