Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-22 Thread Jamal Hadi Salim
On 16-05-20 01:50 AM, Eric Dumazet wrote: It wont be inaccurate (no drift), as the writers hold the qdisc spinlock. It it the same with say IP/TCP SNMP counters : When an incoming frame is handled, it might change many SNMP counters, but an SNMP agent might fetch the whole set of SNMP values i

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-22 Thread Jamal Hadi Salim
On 16-05-19 08:35 AM, Eric Dumazet wrote: From: Eric Dumazet Large tc dumps (tc -s {qdisc|class} sh dev ethX) done by Google BwE host agent [1] are problematic at scale : For each qdisc/class found in the dump, we currently lock the root qdisc spinlock in order to get stats. Sampling stats eve

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-20 Thread David Miller
From: Eric Dumazet Date: Thu, 19 May 2016 05:35:20 -0700 > From: Eric Dumazet > > Large tc dumps (tc -s {qdisc|class} sh dev ethX) done by Google BwE host > agent [1] are problematic at scale : > > For each qdisc/class found in the dump, we currently lock the root qdisc > spinlock in order

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-20 Thread Eric Dumazet
On Fri, 2016-05-20 at 06:01 -0700, Eric Dumazet wrote: > Tricky, since sch_direct_xmit() releases the qdisc spinlock and grabs it > again, while owning the ' running seqcount' > > Needs more LOCKDEP tricks ;) > That would be : @@ -137,10 +137,10 @@ int sch_direct_xmit(struct sk_buff *skb, stru

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-20 Thread Eric Dumazet
On Fri, 2016-05-20 at 05:44 -0700, Eric Dumazet wrote: > On Thu, 2016-05-19 at 19:45 -0700, Eric Dumazet wrote: > > On Thu, 2016-05-19 at 18:50 -0700, Cong Wang wrote: > > > On Thu, May 19, 2016 at 5:35 AM, Eric Dumazet > > > wrote: > > > > > > > > These stats are using u64 or u32 fields, so read

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-20 Thread Eric Dumazet
On Thu, 2016-05-19 at 19:45 -0700, Eric Dumazet wrote: > On Thu, 2016-05-19 at 18:50 -0700, Cong Wang wrote: > > On Thu, May 19, 2016 at 5:35 AM, Eric Dumazet > > wrote: > > > > > > These stats are using u64 or u32 fields, so reading integral values > > > should not prevent writers from doing con

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-19 Thread Eric Dumazet
On Thu, 2016-05-19 at 22:23 -0700, Cong Wang wrote: > On Thu, May 19, 2016 at 7:45 PM, Eric Dumazet wrote: > > On Thu, 2016-05-19 at 18:50 -0700, Cong Wang wrote: > >> On Thu, May 19, 2016 at 5:35 AM, Eric Dumazet > >> wrote: > >> > > >> > These stats are using u64 or u32 fields, so reading inte

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-19 Thread Cong Wang
On Thu, May 19, 2016 at 7:45 PM, Eric Dumazet wrote: > On Thu, 2016-05-19 at 18:50 -0700, Cong Wang wrote: >> On Thu, May 19, 2016 at 5:35 AM, Eric Dumazet wrote: >> > >> > These stats are using u64 or u32 fields, so reading integral values >> > should not prevent writers from doing concurrent up

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-19 Thread Eric Dumazet
On Thu, 2016-05-19 at 18:50 -0700, Cong Wang wrote: > On Thu, May 19, 2016 at 5:35 AM, Eric Dumazet wrote: > > > > These stats are using u64 or u32 fields, so reading integral values > > should not prevent writers from doing concurrent updates if the kernel > > arch is a 64bit one. > > > > Being a

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-19 Thread Cong Wang
On Thu, May 19, 2016 at 5:35 AM, Eric Dumazet wrote: > > These stats are using u64 or u32 fields, so reading integral values > should not prevent writers from doing concurrent updates if the kernel > arch is a 64bit one. > > Being able to atomically fetch all counters like packets and bytes sent >

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-19 Thread Eric Dumazet
On Thu, 2016-05-19 at 09:08 -0700, Alexei Starovoitov wrote: > initially I thought that above line could have been qdisc_root_sleeping_lock() > but then realized that moving out ASSERT_RTNL makes more sense. Good call. > The only thing not clear to me is why '!defined(CONFIG_LOCKDEP)' ? > Just ext

Re: [RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-19 Thread Alexei Starovoitov
On Thu, May 19, 2016 at 05:35:20AM -0700, Eric Dumazet wrote: > From: Eric Dumazet > > Large tc dumps (tc -s {qdisc|class} sh dev ethX) done by Google BwE host > agent [1] are problematic at scale : > > For each qdisc/class found in the dump, we currently lock the root qdisc > spinlock in or

[RFC net-next] net: sched: do not acquire qdisc spinlock in qdisc/class stats dump

2016-05-19 Thread Eric Dumazet
From: Eric Dumazet Large tc dumps (tc -s {qdisc|class} sh dev ethX) done by Google BwE host agent [1] are problematic at scale : For each qdisc/class found in the dump, we currently lock the root qdisc spinlock in order to get stats. Sampling stats every 5 seconds from thousands of HTB class