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