Message ID | 20191217084718.52098-1-dust.li@linux.alibaba.com |
---|---|
Headers | show |
Series | net: sched: unify __gnet_stats_copy_xxx() for percpu and non-percpu | expand |
From: Dust Li <dust.li@linux.alibaba.com> Date: Tue, 17 Dec 2019 16:47:16 +0800 > Currently, __gnet_stats_copy_xxx() will overwrite the return value when > percpu stats are not enabled. But when percpu stats are enabled, it will > add the percpu stats to the result. This inconsistency brings confusion to > its callers. > > This patch series unify the behaviour of __gnet_stats_copy_basic() and > __gnet_stats_copy_queue() for percpu and non-percpu stats and fix an > incorrect statistic for mqprio class. > > - Patch 1 unified __gnet_stats_copy_xxx() for both percpu and non-percpu > - Patch 2 depending on Patch 1, fixes the problem that 'tc class show' > for mqprio class is always 0. I think this is going to break the estimator. The callers of est_fetch_counters() expect continually incrementing statistics. It does relative subtractions from previous values each time, so if we just reset on the next statistics fetch it won't work. Sorry I can't apply this.
On 12/21/19 8:54 AM, David Miller wrote: > From: Dust Li <dust.li@linux.alibaba.com> > Date: Tue, 17 Dec 2019 16:47:16 +0800 > >> Currently, __gnet_stats_copy_xxx() will overwrite the return value when >> percpu stats are not enabled. But when percpu stats are enabled, it will >> add the percpu stats to the result. This inconsistency brings confusion to >> its callers. >> >> This patch series unify the behaviour of __gnet_stats_copy_basic() and >> __gnet_stats_copy_queue() for percpu and non-percpu stats and fix an >> incorrect statistic for mqprio class. >> >> - Patch 1 unified __gnet_stats_copy_xxx() for both percpu and non-percpu >> - Patch 2 depending on Patch 1, fixes the problem that 'tc class show' >> for mqprio class is always 0. > I think this is going to break the estimator. > > The callers of est_fetch_counters() expect continually incrementing > statistics. It does relative subtractions from previous values each > time, so if we just reset on the next statistics fetch it won't work. > > Sorry I can't apply this. Hi David, Thanks for your reply. I checked the callers of est_fetch_counters(). I think there is a little misunderstanding here.We memset() the 'gnet_stats_basic_packed *b'which is not the original data of the estimator,but just the return value. Actually, it has been already memseted in est_fetch_counters() now. So it shouldnot break the estimator. static void est_fetch_counters(struct net_rate_estimator *e, struct gnet_stats_basic_packed *b) { / memset(b, 0, sizeof(*b)); // <<--- Here b is already memseted/ if (e->stats_lock) spin_lock(e->stats_lock); __gnet_stats_copy_basic(e->running, b, e->cpu_bstats, e->bstats); if (e->stats_lock) spin_unlock(e->stats_lock); } The purpose of this memset() is to maintain a consistent semantics for both percpu and non-percpu statisticsin __gnet_stats_copy_basic(). Thanks Dust