netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay.com>
To: David Miller <davem@davemloft.net>
Cc: haoki@redhat.com, herbert@gondor.apana.org.au,
	netdev@vger.kernel.org, tyasui@redhat.com, mhiramat@redhat.com,
	satoshi.oshima.fk@hitachi.com, billfink@mindspring.com,
	andi@firstfloor.org, johnpol@2ka.mipt.ru,
	shemminger@linux-foundation.org, yoshfuji@linux-ipv6.org,
	yumiko.sugita.yf@hitachi.com
Subject: Re: [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min
Date: Mon, 31 Dec 2007 09:54:32 +0100	[thread overview]
Message-ID: <4778AE48.1040701@cosmosbay.com> (raw)
In-Reply-To: <20071231.001925.151533664.davem@davemloft.net>

David Miller a écrit :
> From: Hideo AOKI <haoki@redhat.com>
> Date: Sun, 30 Dec 2007 04:01:46 -0500
> 
>> diff -pruN net-2.6.25-t12t19m-p4/net/ipv4/proc.c net-2.6.25-t12t19m-p5/net/ipv4/proc.c
>> --- net-2.6.25-t12t19m-p4/net/ipv4/proc.c	2007-12-27 10:19:02.000000000 -0500
>> +++ net-2.6.25-t12t19m-p5/net/ipv4/proc.c	2007-12-29 21:09:21.000000000 -0500
>> @@ -56,7 +56,8 @@ static int sockstat_seq_show(struct seq_
>>  		   sock_prot_inuse(&tcp_prot), atomic_read(&tcp_orphan_count),
>>  		   tcp_death_row.tw_count, atomic_read(&tcp_sockets_allocated),
>>  		   atomic_read(&tcp_memory_allocated));
>> -	seq_printf(seq, "UDP: inuse %d\n", sock_prot_inuse(&udp_prot));
>> +	seq_printf(seq, "UDP: inuse %d mem %d\n", sock_prot_inuse(&udp_prot),
>> +		   atomic_read(&udp_memory_allocated));
>>  	seq_printf(seq, "UDPLITE: inuse %d\n", sock_prot_inuse(&udplite_prot));
>>  	seq_printf(seq, "RAW: inuse %d\n", sock_prot_inuse(&raw_prot));
>>  	seq_printf(seq,  "FRAG: inuse %d memory %d\n",
> 
> More careless patch creation.  :-/
> 
> This breaks the build because udp_memory_allocated is not added until
> patch 2.
> 
> Once again I'll combine all three patches into one but I am extremely
> angry about how careless and broken these two patch submissions were.

I am a litle bit concerned about performance of IVR servers
using SIP protocol.

On those servers, each active channel typically emits/receives 50 UDP/RTP 
frames per second. With G729 codec, each packet contains 10 bytes of payload, 
and about 40 bytes of IP/UDP/RTP encapsulation. (So these messages are very
small)

As I am currently enjoying hollidays at home, I am not able to test on my 
server farm the performance impact of this new UDP receive accounting.

If I understand well the patch, each time a packet is received (on a socket
with no previous message available in its receive queue), we are going to 
atomic_inc(&some_global_var). Then the user thread that will transfert this
message to user land will atomic_dec(&some_global_var). (Granted server is
in normal condition, ie each UDP socket holds at most one message in its
receive or transmit queue)

I have some machines with 400 active SIP channels, so that new hot cache line
will probably slow down our SMP servers, because of cache line ping pong.

I will probably setup a test next week and let you know the results.

Maybe I read the patch incorrectly, or we could add some new sysctl so that
we not try to uncharge memory if a socket 'forward_alloc' is beyond a given 
limit (say 2 pages), so that number of atomic_inc/dec on udp_memory_allocated 
(or tcp_memory_allocated) is reduced.

Thank you

  reply	other threads:[~2007-12-31  8:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-30  8:57 [PATCH 0/3] UDP memory accounting and limitation (take 12) Hideo AOKI
2007-12-30  9:01 ` [PATCH 1/3] [UDP]: add udp_mem, udp_rmem_min and udp_wmem_min Hideo AOKI
2007-12-31  8:19   ` David Miller
2007-12-31  8:54     ` Eric Dumazet [this message]
2007-12-31  9:11       ` Herbert Xu
2007-12-31 12:13       ` David Miller
2007-12-31 14:42         ` Eric Dumazet
2008-01-11  6:00           ` David Miller
2007-12-31 18:58     ` Hideo AOKI
2007-12-30  9:02 ` [PATCH 2/3] [UDP]: memory accounting in IPv4 Hideo AOKI
2007-12-30  9:28   ` Eric Dumazet
2007-12-31 18:43     ` Hideo AOKI
2007-12-31 18:58       ` Eric Dumazet
2007-12-30  9:02 ` [PATCH 3/3] [UDP]: memory accounting in IPv6 Hideo AOKI

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4778AE48.1040701@cosmosbay.com \
    --to=dada1@cosmosbay.com \
    --cc=andi@firstfloor.org \
    --cc=billfink@mindspring.com \
    --cc=davem@davemloft.net \
    --cc=haoki@redhat.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=johnpol@2ka.mipt.ru \
    --cc=mhiramat@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=satoshi.oshima.fk@hitachi.com \
    --cc=shemminger@linux-foundation.org \
    --cc=tyasui@redhat.com \
    --cc=yoshfuji@linux-ipv6.org \
    --cc=yumiko.sugita.yf@hitachi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).