On Mon, 2016-06-06 at 16:15 -0700, Cong Wang wrote:
> On Mon, Jun 6, 2016 at 9:37 AM, Eric Dumazet <[email protected]> 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.