Re: [RFC] skb_free_datagram() doing something expensive ?

[Eric Dumazet <dada1@cosmosbay.com>]

[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]

Eric Dumazet a écrit :
> Hi all
> 
> I noticed high contention on udp_memory_allocated on a typical VOIP 
> application.
> 
> (Now that oprofile correctly runs on my machine :) )
> 
> I can see that skb_free_datagram() is :
> 
> void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
> {
>        kfree_skb(skb);
>        sk_mem_reclaim(sk);
> }
> 
> So each time an UDP packet is received, we must touch udp_memory_allocated
> 
> Each time application reads a packet, we call sk_mem_reclaim() and touch 
> again udp_memory_allocated.
> 
> Surely this cannot be correct ?
> 
> If this is correct, time is to resurrect a patch to make 
> proto->memory_allocated a percpu_counter
> or something to have a percpu reserve of say 64 or 128 pages to avoid 
> cache line trashing...
> 
> tcp_memory_allocated do not have this problem, since tcp carefully calls 
> sk_mem_reclaim(sk) only on
> selected paths, not on fast path.
> 
> Thanks
> 
> 

What we can do is to avoid reclaiming space if forward_alloc is less than a page

We did that in the past, when introducing sk_mem_reclaim_partial() in 
commit 9993e7d313e80bdc005d09c7def91903e0068f07
([TCP]: Do not purge sk_forward_alloc entirely in tcp_delack_timer())

This patch gives a nice speedup on UDP, particularly for multiple
RTP flows, where each flow has a medium trafic (say VOIP trafic)

[PATCH] net: sk_free_datagram() should use sk_mem_reclaim_partial()

I noticed a contention on udp_memory_allocated on regular UDP applications.

While tcp_memory_allocated is seldom used, it appears each incoming UDP frame
is currently touching udp_memory_allocated when queued, and when received by
application.

One possible solution is to use sk_mem_reclaim_partial() instead of
sk_mem_reclaim(), so that we keep a small reserve (less than one page)
of memory for each UDP socket.

We did something very similar on TCP side in commit
9993e7d313e80bdc005d09c7def91903e0068f07
([TCP]: Do not purge sk_forward_alloc entirely in tcp_delack_timer())

A more complex solution would need to convert prot->memory_allocated to
use a percpu_counter with batches of 64 or 128 pages.

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

[-- Attachment #2: udp_mem_reclaim.patch --]
[-- Type: text/plain, Size: 611 bytes --]

diff --git a/net/core/datagram.c b/net/core/datagram.c
index ee63184..5e2ac0c 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -209,7 +209,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags,
 void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
 {
 	kfree_skb(skb);
-	sk_mem_reclaim(sk);
+	sk_mem_reclaim_partial(sk);
 }
 
 /**
@@ -248,8 +248,7 @@ int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags)
 		spin_unlock_bh(&sk->sk_receive_queue.lock);
 	}
 
-	kfree_skb(skb);
-	sk_mem_reclaim(sk);
+	skb_free_datagram(sk, skb);
 	return err;
 }