netdev - Re: [PATCH net-next 6/6] net_sched: act: remove spinlock in fast path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 03 Jul 2015 14:08:05 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Jamal Hadi Salim <jhs@...atatu.com>
Cc:	John Fastabend <john.fastabend@...il.com>,
	Eric Dumazet <edumazet@...gle.com>,
	"David S. Miller" <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Alexei Starovoitov <ast@...mgrid.com>,
	Eric Dumazet <edumazet@...il.com>
Subject: Re: [PATCH net-next 6/6] net_sched: act: remove spinlock in fast
 path

On Fri, 2015-07-03 at 07:25 -0400, Jamal Hadi Salim wrote:
> On 07/02/15 16:59, Eric Dumazet wrote:
> > On Thu, 2015-07-02 at 09:35 -0700, John Fastabend wrote:
> 
> >
> > Point is to not dirty cache line for every packet ?
> >
> > Doing the test means we attempt dirtying only ~HZ times per second,
> > which really matters to handle millions of packets per second.
> >
> > My tests show a good enough performance, not sure we want a percpu thing
> > for this lastuse field.
> >
> 
> Does it harm to always set gact->tcf_tm.lastuse ?


Yes it harms, because of one false sharing on a cache line, for every
packet...

Samples: 20K of event 'cycles:pp', Event count (approx.): 15451660731                                                                                       
 32.50%  ksoftirqd/1  [kernel.kallsyms]  [k] tcf_gact
 19.43%  ksoftirqd/1  [kernel.kallsyms]  [k] put_compound_page
  4.34%  ksoftirqd/1  [kernel.kallsyms]  [k] __skb_get_hash
  3.88%  ksoftirqd/1  [kernel.kallsyms]  [k] memcpy_erms
  2.95%  ksoftirqd/1  [kernel.kallsyms]  [k] get_rps_cpu
  2.81%  ksoftirqd/1  [kernel.kallsyms]  [k] __build_skb


Note that gact->tcf_action & gact->tcf_tm.lastuse share same cache line


      │    Disassembly of section load0:
       │
       │    ffffffffa016f040 <load0>:
  0.11 │      nop
  0.06 │      push   %rbp
  0.14 │      mov    %rsp,%rbp
  0.06 │      push   %r12
       │      mov    %rdi,%r12
       │      push   %rbx
  0.16 │      mov    (%rsi),%rbx
 98.46 │      mov    0x20(%rbx),%eax         action = READ_ONCE(gact->tcf_action); 

_huge_ stall here because another cpu dirtied gact->tcf_tm.lastuse

  0.33 │      movzwl 0x98(%rbx),%edx
       │      test   %edx,%edx
       │      jne    87
       │20:   mov    0x88(%rbx),%rdx
  0.11 │      add    %gs:0x5fe9b0b9(%rip),%rdx
       │      mov    0x28(%r12),%ecx
  0.02 │      mov    0x8(%rdx),%edi
       │      mov    $0x1,%esi
  0.11 │      add    %rcx,(%rdx)
       │      mov    0xc8(%r12),%ecx
       │      add    0xd0(%r12),%rcx
  0.08 │      cmpw   $0x0,0x2(%rcx)
       │      je     5a
       │      movzwl 0x4(%rcx),%esi
       │5a:   add    %edi,%esi
       │      cmp    $0x2,%eax
  0.14 │      mov    %esi,0x8(%rdx)
       │      jne    77
       │      mov    0x90(%rbx),%rdx
       │      add    %gs:0x5fe9b075(%rip),%rdx
  0.11 │      addl   $0x1,0x8(%rdx)
       │77:   mov    -0x1e5640be(%rip),%rdx
       │      mov    %rdx,0x30(%rbx)                 gact->tcf_tm.lastuse = jiffies;
  0.08 │      pop    %rbx
  0.02 │      pop    %r12
       │      pop    %rbp
       │      retq
       │87:   mov    %edx,%edx
       │      mov    %rbx,%rdi
       │      callq  *-0x5fe90b70(,%rdx,8)
       │      jmp    20


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html