irq disable in __netdev_alloc_frag() ?

irq disable in __netdev_alloc_frag() ?

[Alexei Starovoitov]

Hi Eric,

in the commit 6f532612cc24 ("net: introduce netdev_alloc_frag()")
you mentioned that the reason to disable interrupts
in __netdev_alloc_frag() is:
"- Must be IRQ safe (non NAPI drivers can use it)"

Is there a way to do this conditionally?

Without it I see 10% performance gain for my RX tests
(from 6.9Mpps to 7.7Mpps) and __netdev_alloc_frag()
itself goes from 6.6% to 2.1%
(popf seems to be quite costly)

Thanks
Alexei

Re: irq disable in __netdev_alloc_frag() ?

[Eric Dumazet]

On Wed, 2014-10-22 at 17:15 -0700, Alexei Starovoitov wrote:
> Hi Eric,
> 
> in the commit 6f532612cc24 ("net: introduce netdev_alloc_frag()")
> you mentioned that the reason to disable interrupts
> in __netdev_alloc_frag() is:
> "- Must be IRQ safe (non NAPI drivers can use it)"
> 
> Is there a way to do this conditionally?
> 
> Without it I see 10% performance gain for my RX tests
> (from 6.9Mpps to 7.7Mpps) and __netdev_alloc_frag()
> itself goes from 6.6% to 2.1%
> (popf seems to be quite costly)

Well, your driver is probably a NAPI one, so you need to
mask irqs, or to remove all non NAPI drivers from linux.

__netdev_alloc_frag() (__netdev_alloc_skb()) is used by all.

Problem is __netdev_alloc_frag() is generally deep inside caller
chain, so using a private pool might have quite an overhead.

Same could be said for skb_queue_head() /skb_queue_tail() /
sock_queue_rcv_skb() :
Many callers don't need to block irq.

Re: irq disable in __netdev_alloc_frag() ?

[Alexei Starovoitov]

On Wed, Oct 22, 2014 at 6:52 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> On Wed, 2014-10-22 at 17:15 -0700, Alexei Starovoitov wrote:
>> Hi Eric,
>>
>> in the commit 6f532612cc24 ("net: introduce netdev_alloc_frag()")
>> you mentioned that the reason to disable interrupts
>> in __netdev_alloc_frag() is:
>> "- Must be IRQ safe (non NAPI drivers can use it)"
>>
>> Is there a way to do this conditionally?
>>
>> Without it I see 10% performance gain for my RX tests
>> (from 6.9Mpps to 7.7Mpps) and __netdev_alloc_frag()
>> itself goes from 6.6% to 2.1%
>> (popf seems to be quite costly)
>
> Well, your driver is probably a NAPI one, so you need to
> mask irqs, or to remove all non NAPI drivers from linux.

yeah, the 10G+ nics I care about are all napi :)

> __netdev_alloc_frag() (__netdev_alloc_skb()) is used by all.
>
> Problem is __netdev_alloc_frag() is generally deep inside caller
> chain, so using a private pool might have quite an overhead.

yes. I was thinking, since dev is already passed
into __netdev_alloc_skb(), we can check whether
dev registered with napi via dev->napi_list and if so,
tell inner __netdev_alloc_frag() to skip irq disabling...

don't know about skb_queue_head() and friends.
I'm only looking at pure rx now. One challenge at a time.

Re: irq disable in __netdev_alloc_frag() ?

[Eric Dumazet]

On Wed, 2014-10-22 at 19:22 -0700, Alexei Starovoitov wrote:

> yes. I was thinking, since dev is already passed
> into __netdev_alloc_skb(), we can check whether
> dev registered with napi via dev->napi_list and if so,
> tell inner __netdev_alloc_frag() to skip irq disabling...
> 

This does not matter. The problem is not _this_ device, the problem is
that another device might trigger a hard irq, and this hard irq could
mess your data.

Re: irq disable in __netdev_alloc_frag() ?

[Alexander Duyck]

On 10/22/2014 06:52 PM, Eric Dumazet wrote:
> On Wed, 2014-10-22 at 17:15 -0700, Alexei Starovoitov wrote:
>> Hi Eric,
>>
>> in the commit 6f532612cc24 ("net: introduce netdev_alloc_frag()")
>> you mentioned that the reason to disable interrupts
>> in __netdev_alloc_frag() is:
>> "- Must be IRQ safe (non NAPI drivers can use it)"
>>
>> Is there a way to do this conditionally?
>>
>> Without it I see 10% performance gain for my RX tests
>> (from 6.9Mpps to 7.7Mpps) and __netdev_alloc_frag()
>> itself goes from 6.6% to 2.1%
>> (popf seems to be quite costly)
> Well, your driver is probably a NAPI one, so you need to
> mask irqs, or to remove all non NAPI drivers from linux.
>
> __netdev_alloc_frag() (__netdev_alloc_skb()) is used by all.
>
> Problem is __netdev_alloc_frag() is generally deep inside caller
> chain, so using a private pool might have quite an overhead.
>
> Same could be said for skb_queue_head() /skb_queue_tail() /
> sock_queue_rcv_skb() :
> Many callers don't need to block irq.

Couldn't __netdev_alloc_frag() be forked into two functions, one that is
only called from inside the NAPI context and one that is called for all
other contexts?  It would mean having to double the number of pages
being held per CPU, but I would think something like that would be doable.

Thanks,

Alex

Re: irq disable in __netdev_alloc_frag() ?

[Eric Dumazet]

On Wed, 2014-10-22 at 20:19 -0700, Alexander Duyck wrote:

> Couldn't __netdev_alloc_frag() be forked into two functions, one that is
> only called from inside the NAPI context and one that is called for all
> other contexts?  It would mean having to double the number of pages
> being held per CPU, but I would think something like that would be doable.

Possibly, but this looks like code bloat for me.

On my hosts, this hard irq masking is pure noise.

What CPU are you using Alexander ?

Same could be done with some kmem_cache_alloc() : SLAB uses hard irq
masking while some caches are never used from hard irq context.

Re: irq disable in __netdev_alloc_frag() ?

[Eric Dumazet]

On Wed, 2014-10-22 at 20:51 -0700, Eric Dumazet wrote:
> On Wed, 2014-10-22 at 20:19 -0700, Alexander Duyck wrote:
> 
> > Couldn't __netdev_alloc_frag() be forked into two functions, one that is
> > only called from inside the NAPI context and one that is called for all
> > other contexts?  It would mean having to double the number of pages
> > being held per CPU, but I would think something like that would be doable.
> 
> Possibly, but this looks like code bloat for me.
> 
> On my hosts, this hard irq masking is pure noise.
> 
> What CPU are you using Alexander ?

Sorry, the question was for Alexei ;)

Re: irq disable in __netdev_alloc_frag() ?

[Alexei Starovoitov]

On Wed, Oct 22, 2014 at 8:56 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2014-10-22 at 20:51 -0700, Eric Dumazet wrote:
>> On Wed, 2014-10-22 at 20:19 -0700, Alexander Duyck wrote:
>>
>> > Couldn't __netdev_alloc_frag() be forked into two functions, one that is
>> > only called from inside the NAPI context and one that is called for all
>> > other contexts?  It would mean having to double the number of pages
>> > being held per CPU, but I would think something like that would be doable.
>>
>> Possibly, but this looks like code bloat for me.
>>
>> On my hosts, this hard irq masking is pure noise.
>>
>> What CPU are you using Alexander ?
>
> Sorry, the question was for Alexei ;)

my numbers were from just released Xeon E5-1630 v3
which is haswell-ep
I think Alexander's idea is worth pursuing.
Code-wise the bloat is minimal: one extra netdev_alloc_cache
and few 'if' which one to choose.
Most of the time only one of them is used, since
systems with napi and not-napi nics are very rare.
I think it should help virtualization case as well,
since virtio_net is napi already.

Re: irq disable in __netdev_alloc_frag() ?

[Eric Dumazet]

On Wed, 2014-10-22 at 21:29 -0700, Alexei Starovoitov wrote:

> my numbers were from just released Xeon E5-1630 v3
> which is haswell-ep
> I think Alexander's idea is worth pursuing.
> Code-wise the bloat is minimal: one extra netdev_alloc_cache
> and few 'if' which one to choose.


> Most of the time only one of them is used, since
> systems with napi and not-napi nics are very rare.


> I think it should help virtualization case as well,
> since virtio_net is napi already.

Really, I doubt this will make any difference in real workloads
where bottleneck is memory bandwidth, not the time taken by
pushf/cli/popf.

What difference do you have on a real workload like : netperf TCP_RR or
UDP_RR ?

Re: irq disable in __netdev_alloc_frag() ?

[Alexei Starovoitov]

On Wed, Oct 22, 2014 at 10:14 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Really, I doubt this will make any difference in real workloads
> where bottleneck is memory bandwidth, not the time taken by
> pushf/cli/popf.
>
> What difference do you have on a real workload like : netperf TCP_RR or
> UDP_RR ?

there are different workloads. l2/l3 switching is very real
as well. For tcp_rr on the host the difference will be
minimal, since the time is spent elsewhere. For cases
that don't use tcp stack, but do pure l2/l3 processing
all improvements will add up. imo the kernel should be
optimized for all use cases when possible.

Re: irq disable in __netdev_alloc_frag() ?

[Jesper Dangaard Brouer]

On Wed, 22 Oct 2014 20:51:16 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On my hosts, this hard irq masking is pure noise.

On my hosts I can measure a significant difference between using
local_irq_disable() vs. local_irq_save(flags)

 *  2.860 ns cost for local_irq_{disable,enable} 
 * 14.840 ns cost for local_irq_save()+local_irq_restore() 

This is quite significant in my nanosec world ;-)

 
> What CPU are you using Alexander ?

I'm using a E5-2695 (Ivy-bridge)

You can easily reproduce my results on your own system with my
time_bench_sample module here:
 https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/lib/time_bench_sample.c#L173

> Same could be done with some kmem_cache_alloc() : SLAB uses hard irq
> masking while some caches are never used from hard irq context.

Sounds interesting.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

Re: irq disable in __netdev_alloc_frag() ?

[Christoph Lameter]

On Mon, 27 Oct 2014, Jesper Dangaard Brouer wrote:

> > Same could be done with some kmem_cache_alloc() : SLAB uses hard irq
> > masking while some caches are never used from hard irq context.
>
> Sounds interesting.

SLUB does not disable interrupts in the fast paths.

Re: irq disable in __netdev_alloc_frag() ?

[Eric Dumazet]

On Mon, Oct 27, 2014 at 7:30 PM, Christoph Lameter <cl@linux.com> wrote:
> On Mon, 27 Oct 2014, Jesper Dangaard Brouer wrote:
>
>> > Same could be done with some kmem_cache_alloc() : SLAB uses hard irq
>> > masking while some caches are never used from hard irq context.
>>
>> Sounds interesting.
>
> SLUB does not disable interrupts in the fast paths.
>

Unfortunately, SLUB is more expensive than SLAB for many networking workloads.

The cost of disabling interrupts is pure noise compared to cache line misses.

SLUB has poor behavior compared to SLAB with alien caches,
even with the side effect that 'struct page' is 64 bytes aligned
instead of being 56 bytes with SLAB

Note that I am not doing SLUB/SLAB tests every day, so it might be
better nowadays.

Re: irq disable in __netdev_alloc_frag() ?

[David Miller]

From: Eric Dumazet <edumazet@google.com>
Date: Mon, 27 Oct 2014 19:46:20 -0700

> Unfortunately, SLUB is more expensive than SLAB for many networking
> workloads.
> 
> The cost of disabling interrupts is pure noise compared to cache
> line misses.
> 
> SLUB has poor behavior compared to SLAB with alien caches, even with
> the side effect that 'struct page' is 64 bytes aligned instead of
> being 56 bytes with SLAB

And SLAB completely shits itself when lots of memory gets cached up on
a foreign node.

This discussion has happened many times, SLAB may be faster when things
work out nicely, but it acts poorly wrt. keeping foreign memory from
being cached too aggressively.

And there is a cost for that, which is that foreign memory has to be
properly balanced back to it's home node.

Re: irq disable in __netdev_alloc_frag() ?

[Christoph Lameter]

On Mon, 27 Oct 2014, Eric Dumazet wrote:

> Unfortunately, SLUB is more expensive than SLAB for many networking workloads.

hackbench performs better with SLUB.

> The cost of disabling interrupts is pure noise compared to cache line misses.
>
> SLUB has poor behavior compared to SLAB with alien caches,
> even with the side effect that 'struct page' is 64 bytes aligned
> instead of being 56 bytes with SLAB
>
> Note that I am not doing SLUB/SLAB tests every day, so it might be
> better nowadays.

Well we did some intensive work on these issues a couple of years ago.