: memory accounting and limitation (take 10)

PATCH In review Hideo AOKI 3 participants 9 messages Dec 15, 2007 – Dec 16, 2007

Subsystems (6)

Networking Network drivers Documentation IPv4/IPv6 +2 more subsystems

lore

Share & tools

lore

Hideo AOKI

author

2007-12-15 05:11 UTC #0 lore raw

cover : memory accounting and limitation (take 10)

Hello,

This is the latest patch set of UDP memory accounting and limitation.

To reduce number of atomic access to global variable, the patch set supports per socket accounting using sk_forward_alloc like stream protocols.

My colleagues and I tested the patch set on net-2.6 tree.

Please consider applying.

Changelog take 9 -> take 10:
* supported using sk_forward_alloc
* introduced several memory accounting functions with spin lock
* changed detagram receive functions to be able to customize destructor
* fixed accounting bugs in previous takes

Changelog take 8 -> take 9:
* introduced mem_schdeule functions for datargram protocols
* removed protocol check function, from patch set
* restructured patch set

Changelog take 7 -> take 8:

* sk_datagram_pages(): avoided using divide instruction
* udp_recvmsg(): fixed referring released truesize in accounting

Best regards,
Hideo Aoki

--
Hitachi Computer Products (America) Inc.

Hideo AOKI

author

2007-12-15 05:20 UTC #1 lore raw

PATCH 1/4 : fix send buffer check

This patch introduces sndbuf size check before memory allocation for send buffer.

Signed-off-by Satoshi Oshima <[email protected]>

Signed-off-by Hideo Aoki <[email protected]>

---

 ip_output.c |    5 +++++
 1 file changed, 5 insertions(+)

net/ipv4/ip_output.c +5 -0

diff -pruN net-2.6/net/ipv4/ip_output.c net-2.6-udp-take10a4-p1/net/ipv4/ip_output.c
@@ -1004,6 +1004,11 @@ alloc_new_skb:
 					frag = &skb_shinfo(skb)->frags[i];
 				}
 			} else if (i < MAX_SKB_FRAGS) {
+				if (atomic_read(&sk->sk_wmem_alloc) + PAGE_SIZE
+				    > 2 * sk->sk_sndbuf) {
+					err = -ENOBUFS;
+					goto error;
+				}
 				if (copy > PAGE_SIZE)
 					copy = PAGE_SIZE;
 				page = alloc_pages(sk->sk_allocation, 0);

Signature · 1 line

Hitachi Computer Products (America) Inc.

Hideo AOKI

author

2007-12-15 05:20 UTC #2 lore raw

PATCH 2/4 : datagram: mem_scheudle functions

In reply to Hideo AOKI

This patch includes changes in network core sub system for memory accounting.

Memory scheduling, charging, uncharging and reclaiming functions are added. These functions use sk_forward_alloc to store socket local accounting. They also need to use lock to keep consistency of sk_forward_alloc and memory_allocated. They currently support only datagram protocols.

sk_datagram_rfree() is a receive buffer detractor for datagram protocols which are capable of protocol specific memory accounting.

To enable memory accounting in releasing receive buffer, sock_queue_rcv_skb() is modified although the interface isn't changed. The body of the function is implemented in sock_queue_rcv_skb_with_owner(). Additionally, skb_set_owner_r() is moved to sock.h to core/datagram.c because we want to use it as a call back function.

Cc Satoshi Oshima <[email protected]>

Signed-off-by Takahiro Yasui <[email protected]>

Signed-off-by Masami Hiramatsu <[email protected]>

Signed-off-by Hideo Aoki <[email protected]>

---

 include/net/sock.h  |  117 +++++++++++++++++++++++++++++++++++++++++++++++++---
 net/core/datagram.c |   72 ++++++++++++++++++++++++++++++++
 net/core/sock.c     |   13 ++++-
 3 files changed, 193 insertions(+), 9 deletions(-)

include/net/sock.h +111 -6

diff -pruN net-2.6-udp-take10a4-p1/include/net/sock.h net-2.6-udp-take10a4-p2/include/net/sock.h
@@ -750,6 +750,9 @@ static inline struct inode *SOCK_INODE(s
 	return &container_of(socket, struct socket_alloc, socket)->vfs_inode;
 }
 
+/*
+ * Functions for memory accounting
+ */
 extern void __sk_stream_mem_reclaim(struct sock *sk);
 extern int sk_stream_mem_schedule(struct sock *sk, int size, int kind);

@@ -778,6 +781,107 @@ static inline int sk_stream_wmem_schedul
 	       sk_stream_mem_schedule(sk, size, 0);
 }
 
+extern void __sk_datagram_mem_reclaim(struct sock *sk);
+extern int sk_stream_mem_schedule(struct sock *sk, int size, int kind);
+
+#define SK_DATAGRAM_MEM_QUANTUM ((unsigned int)PAGE_SIZE)
+
+static inline int sk_datagram_pages(int amt)
+{
+	/* Cast to unsigned as an optimization, since amt is always positive. */
+	return DIV_ROUND_UP((unsigned int)amt, SK_DATAGRAM_MEM_QUANTUM);
+}
+
+extern void __sk_datagram_mem_reclaim(struct sock *sk);
+extern int sk_datagram_mem_schedule(struct sock *sk, int size, int kind);
+
+static inline void sk_datagram_mem_reclaim(struct sock *sk)
+{
+	unsigned long flags;
+
+	if (!sk->sk_prot->memory_allocated)
+		return;
+
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
+	__sk_datagram_mem_reclaim(sk);
+	spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+}
+
+static inline int sk_datagram_rmem_schedule(struct sock *sk, int size)
+{
+	return size <= sk->sk_forward_alloc ||
+		sk_datagram_mem_schedule(sk, size, 1);
+}
+
+static inline int sk_datagram_wmem_schedule(struct sock *sk, int size)
+{
+	return size <= sk->sk_forward_alloc ||
+		sk_datagram_mem_schedule(sk, size, 0);
+}
+
+static inline void sk_mem_reclaim(struct sock *sk)
+{
+	if (sk->sk_type == SOCK_DGRAM)
+		sk_datagram_mem_reclaim(sk);
+}
+
+static inline int sk_wmem_schedule(struct sock *sk, int size)
+{
+	if (sk->sk_type == SOCK_DGRAM)
+		return sk_datagram_wmem_schedule(sk, size);
+	else
+		return 1;
+}
+
+static inline int sk_account_wmem_charge(struct sock *sk, int size)
+{
+	unsigned long flags;
+
+	/* account if protocol supports memory accounting. */
+	if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
+		return 1;
+
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
+	if (sk_datagram_wmem_schedule(sk, size)) {
+		sk->sk_forward_alloc -= size;
+		spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+		return 1;
+	}
+	spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+	return 0;
+}
+
+static inline int sk_account_rmem_charge(struct sock *sk, int size)
+{
+	unsigned long flags;
+
+	/* account if protocol supports memory accounting. */
+	if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
+		return 1;
+
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
+	if (sk_datagram_rmem_schedule(sk, size)) {
+		sk->sk_forward_alloc -= size;
+		spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+		return 1;
+	}
+	spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+	return 0;
+}
+
+static inline void sk_account_uncharge(struct sock *sk, int size)
+{
+	unsigned long flags;
+
+	/* account if protocol supports memory accounting. */
+	if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
+		return;
+
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
+	sk->sk_forward_alloc += size;
+	spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+}
+
 /* Used by processes to "lock" a socket state, so that
  * interrupts and bottom half handlers won't change it
  * from under us. It essentially blocks any incoming

@@ -1159,18 +1263,19 @@ static inline void skb_set_owner_w(struc
 	atomic_add(skb->truesize, &sk->sk_wmem_alloc);
 }
 
-static inline void skb_set_owner_r(struct sk_buff *skb, struct sock *sk)
-{
-	skb->sk = sk;
-	skb->destructor = sock_rfree;
-	atomic_add(skb->truesize, &sk->sk_rmem_alloc);
-}
+extern void skb_set_owner_r(struct sk_buff *skb, struct sock *sk);
+
+void sk_datagram_rfree(struct sk_buff *skb);
 
 extern void sk_reset_timer(struct sock *sk, struct timer_list* timer,
 			   unsigned long expires);
 
 extern void sk_stop_timer(struct sock *sk, struct timer_list* timer);
 
+extern int sock_queue_rcv_skb_with_owner(struct sock *sk, struct sk_buff *skb,
+					 void set_owner_r(struct sk_buff *nskb,
+							  struct sock* nsk));
+
 extern int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
 
 static inline int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)

net/core/datagram.c +72 -0

84 lines not on this page · raw message

net/core/sock.c +10 -3

diff -pruN net-2.6-udp-take10a4-p1/net/core/sock.c net-2.6-udp-take10a4-p2/net/core/sock.c
@@ -263,8 +263,9 @@ static void sock_disable_timestamp(struc
 	}
 }
 
-
-int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+int sock_queue_rcv_skb_with_owner(struct sock *sk, struct sk_buff *skb,
+				  void set_owner_r(struct sk_buff *nskb,
+						   struct sock* nsk))
 {
 	int err = 0;
 	int skb_len;

@@ -283,7 +284,7 @@ int sock_queue_rcv_skb(struct sock *sk,
 		goto out;
 
 	skb->dev = NULL;
-	skb_set_owner_r(skb, sk);
+	set_owner_r(skb, sk);
 
 	/* Cache the SKB length before we tack it onto the receive
 	 * queue.  Once it is added it no longer belongs to us and

@@ -299,6 +300,12 @@ int sock_queue_rcv_skb(struct sock *sk,
 out:
 	return err;
 }
+EXPORT_SYMBOL(sock_queue_rcv_skb_with_owner);
+
+int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+	return sock_queue_rcv_skb_with_owner(sk, skb, skb_set_owner_r);
+}
 EXPORT_SYMBOL(sock_queue_rcv_skb);
 
 int sk_receive_skb(struct sock *sk, struct sk_buff *skb, const int nested)

Signature · 1 line

Hitachi Computer Products (America) Inc.

Herbert Xu

2007-12-15 15:32 UTC #3 lore raw

Re: [PATCH 2/4] [CORE]: datagram: mem_scheudle functions

Quoted 8 lines · Hideo AOKI

This patch includes changes in network core sub system for memory accounting.
Memory scheduling, charging, uncharging and reclaiming functions are added. These functions use sk_forward_alloc to store socket local accounting. They also need to use lock to keep consistency of sk_forward_alloc and memory_allocated. They currently support only datagram protocols.

Thanks for the patch. I think it's generally on the right track but there's still a few issues with the implementation.

Review

Quoted 1 line

(quoted diff) +1 -0

+	spin_lock_irqsave(&sk->sk_lock.slock, flags);

Please use bh_lock_sock since this must never be used from an IRQ handler.

include/net/sock.h static inline int sk_stream_wmem_schedul · line 826

Quoted 5 lines

include/net/sock.h +5 -0

+static inline void sk_mem_reclaim(struct sock *sk)
+{
+	if (sk->sk_type == SOCK_DGRAM)
+		sk_datagram_mem_reclaim(sk);
+}

Please get rid of these wrappers since we should only get here for datagram protocols.

Quoted 7 lines

include/net/sock.h +7 -0

+static inline int sk_account_wmem_charge(struct sock *sk, int size)
+{
+	unsigned long flags;
+
+	/* account if protocol supports memory accounting. */
+	if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
+		return 1;

include/net/sock.h static inline int sk_stream_wmem_schedul · line 852

Quoted 10 lines

include/net/sock.h +10 -0

+
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
+	if (sk_datagram_wmem_schedule(sk, size)) {
+		sk->sk_forward_alloc -= size;
+		spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+		return 1;
+	}
+	spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+	return 0;
+}

This is probably too big to inline.

Quoted 7 lines

include/net/sock.h +7 -0

+static inline int sk_account_rmem_charge(struct sock *sk, int size)
+{
+	unsigned long flags;
+
+	/* account if protocol supports memory accounting. */
+	if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
+		return 1;

include/net/sock.h static inline int sk_stream_wmem_schedul · line 870

Quoted 10 lines

include/net/sock.h +10 -0

+
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
+	if (sk_datagram_rmem_schedule(sk, size)) {
+		sk->sk_forward_alloc -= size;
+		spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+		return 1;
+	}
+	spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+	return 0;
+}

Why are we duplicating the rmem/wmem versions when they're identical?

net/core/sock.c static void sock_disable_timestamp(struc · line 268

Quoted 4 lines

net/core/sock.c +3 -1

-int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+int sock_queue_rcv_skb_with_owner(struct sock *sk, struct sk_buff *skb,
+				  void set_owner_r(struct sk_buff *nskb,
+						   struct sock* nsk))

Just make a new function for this rather than playing with function pointers.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Hideo AOKI

author

2007-12-16 21:23 UTC #4 lore raw

Re: [PATCH 2/4] [CORE]: datagram: mem_scheudle functions

Hello,

Thank you for your quick comments.

Quoted 4 lines · Herbert Xu

Review
Quoted 1 line
(quoted diff) +1 -0
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
Please use bh_lock_sock since this must never be used from an IRQ handler.

I'll try to re-implement this locking mechanism as David suggested.

Quoted 8 lines

include/net/sock.h static inline int sk_stream_wmem_schedul · line 826
Quoted 5 lines
include/net/sock.h +5 -0
+static inline void sk_mem_reclaim(struct sock *sk)
+{
+	if (sk->sk_type == SOCK_DGRAM)
+		sk_datagram_mem_reclaim(sk);
+}
Please get rid of these wrappers since we should only get here for datagram protocols.

In my understanding, TCP also uses ip_append_data() and ip_ufo_append_data() via ip_send_reply(). Then I thought TCP could reach datagram memory accounting functions, and I used the wrappers since I didn't want to change TCP's sk_alloc_forward and memory_allocated.

I'll try to remove these wrappers in next patch set. But I'd appreciate it if you let me know whether we can keep the wrappers in case TCP memory accounting doesn't work well due to accounting in IP layer.

Quoted 31 lines

Quoted 7 lines

include/net/sock.h +7 -0

+static inline int sk_account_wmem_charge(struct sock *sk, int size)
+{
+	unsigned long flags;
+
+	/* account if protocol supports memory accounting. */
+	if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
+		return 1;

include/net/sock.h static inline int sk_stream_wmem_schedul · line 852

Quoted 10 lines

include/net/sock.h +10 -0

+
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
+	if (sk_datagram_wmem_schedule(sk, size)) {
+		sk->sk_forward_alloc -= size;
+		spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+		return 1;
+	}
+	spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+	return 0;
+}

This is probably too big to inline.

Quoted 11 lines

include/net/sock.h +11 -0

+static inline int sk_account_rmem_charge(struct sock *sk, int size)
+{
+	unsigned long flags;
+
+	/* account if protocol supports memory accounting. */
+	if (!sk->sk_prot->memory_allocated || sk->sk_type != SOCK_DGRAM)
+		return 1;
+
+	spin_lock_irqsave(&sk->sk_lock.slock, flags);
+	if (sk_datagram_rmem_schedule(sk, size)) {
+		sk->sk_forward_alloc -= size;

Quoted 8 lines

include/net/sock.h static inline int sk_stream_wmem_schedul · line 852
Quoted 6 lines
include/net/sock.h +6 -0
+		spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+		return 1;
+	}
+	spin_unlock_irqrestore(&sk->sk_lock.slock, flags);
+	return 0;
+}
Why are we duplicating the rmem/wmem versions when they're identical?

Good catch. I'll merge the body of these functions into one function. Furthermore, I'll stop using inline if the code is large.

Quoted 7 lines

net/core/sock.c static void sock_disable_timestamp(struc · line 268
Quoted 4 lines
net/core/sock.c +3 -1
-int sock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+int sock_queue_rcv_skb_with_owner(struct sock *sk, struct sk_buff *skb,
+				  void set_owner_r(struct sk_buff *nskb,
+						   struct sock* nsk))
Just make a new function for this rather than playing with function pointers.

I understood. I'll fix it.

Again, many thanks for the review.

Regards,
Hideo

Signature · 1 line

Hitachi Computer Products (America) Inc.

Hideo AOKI

author

2007-12-15 05:22 UTC #5 lore raw

PATCH 3/4 : add udp_mem, udp_rmem_min and udp_wmem_min

In reply to Hideo AOKI

This patch adds sysctl parameters for customizing UDP memory accounting:

     /proc/sys/net/ipv4/udp_mem
     /proc/sys/net/ipv4/udp_rmem_min
     /proc/sys/net/ipv4/udp_wmem_min

Udp_mem indicates number of pages which can be used for all UDP sockets. Each UDP packet is dropped, when the number of pages for socket buffer is beyond udp_mem and the socket already consumes minimum buffer.

This patch is also introduced memory_allocated variable for UDP protocol.

Cc Satoshi Oshima <[email protected]>

Signed-off-by Hideo Aoki <[email protected]>

---

 Documentation/networking/ip-sysctl.txt |   18 ++++++++++++++++++
 include/net/udp.h                      |    9 +++++++++
 net/ipv4/af_inet.c                     |    3 +++
 net/ipv4/proc.c                        |    3 ++-
 net/ipv4/sysctl_net_ipv4.c             |   31 +++++++++++++++++++++++++++++++
 net/ipv4/udp.c                         |   27 +++++++++++++++++++++++++++
 6 files changed, 90 insertions(+), 1 deletion(-)

Documentation/networking/ip-sysctl.txt +18 -0

diff -pruN net-2.6-udp-take10a4-p2/Documentation/networking/ip-sysctl.txt net-2.6-udp-take10a4-p3/Documentation/networking/ip-sysctl.txt
@@ -446,6 +446,24 @@ tcp_dma_copybreak - INTEGER
 	and CONFIG_NET_DMA is enabled.
 	Default: 4096
 
+UDP variables:
+
+udp_mem - INTEGER
+	Number of pages allowed for queueing by all UDP sockets.
+	Default is calculated at boot time from amount of available memory.
+
+udp_rmem_min - INTEGER
+	Minimal size of receive buffer used by UDP sockets. Each UDP socket
+	is able to use the size for receiving data, even if total pages of UDP
+	sockets exceed udp_mem. The unit is byte.
+	Default: 4096
+
+udp_wmem_min - INTEGER
+	Minimal size of send buffer used by UDP sockets. Each UDP socket is
+	able to use the size for sending data, even if total pages of UDP
+	sockets exceed udp_mem. The unit is byte.
+	Default: 4096
+
 CIPSOv4 Variables:
 
 cipso_cache_enable - BOOLEAN

include/net/udp.h +9 -0

diff -pruN net-2.6-udp-take10a4-p2/include/net/udp.h net-2.6-udp-take10a4-p3/include/net/udp.h
@@ -65,6 +65,13 @@ extern rwlock_t udp_hash_lock;
 
 extern struct proto udp_prot;
 
+extern atomic_t udp_memory_allocated;
+
+/* sysctl variables for udp */
+extern int sysctl_udp_mem;
+extern int sysctl_udp_rmem_min;
+extern int sysctl_udp_wmem_min;
+
 struct sk_buff;
 
 /*

@@ -173,4 +180,6 @@ extern void udp_proc_unregister(struct u
 extern int  udp4_proc_init(void);
 extern void udp4_proc_exit(void);
 #endif
+
+extern void udp_init(void);
 #endif	/* _UDP_H */

net/ipv4/af_inet.c +3 -0

diff -pruN net-2.6-udp-take10a4-p2/net/ipv4/af_inet.c net-2.6-udp-take10a4-p3/net/ipv4/af_inet.c
@@ -1418,6 +1418,9 @@ static int __init inet_init(void)
 	/* Setup TCP slab cache for open requests. */
 	tcp_init();
 
+	/* Setup UDP memory threshold */
+	udp_init();
+
 	/* Add UDP-Lite (RFC 3828) */
 	udplite4_register();

net/ipv4/proc.c +2 -1

diff -pruN net-2.6-udp-take10a4-p2/net/ipv4/proc.c net-2.6-udp-take10a4-p3/net/ipv4/proc.c
@@ -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",

net/ipv4/sysctl_net_ipv4.c +31 -0

diff -pruN net-2.6-udp-take10a4-p2/net/ipv4/sysctl_net_ipv4.c net-2.6-udp-take10a4-p3/net/ipv4/sysctl_net_ipv4.c
@@ -18,6 +18,7 @@
 #include <net/ip.h>
 #include <net/route.h>
 #include <net/tcp.h>
+#include <net/udp.h>
 #include <net/cipso_ipv4.h>
 #include <net/inet_frag.h>

@@ -885,6 +886,36 @@ ctl_table ipv4_table[] = {
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "udp_mem",
+		.data		= &sysctl_udp_mem,
+		.maxlen		= sizeof(sysctl_udp_mem),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &zero
+	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "udp_rmem_min",
+		.data		= &sysctl_udp_rmem_min,
+		.maxlen		= sizeof(sysctl_udp_rmem_min),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &zero
+	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "udp_wmem_min",
+		.data		= &sysctl_udp_wmem_min,
+		.maxlen		= sizeof(sysctl_udp_wmem_min),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec_minmax,
+		.strategy	= &sysctl_intvec,
+		.extra1		= &zero
+	},
 	{ .ctl_name = 0 }
 };

net/ipv4/udp.c +27 -0

diff -pruN net-2.6-udp-take10a4-p2/net/ipv4/udp.c net-2.6-udp-take10a4-p3/net/ipv4/udp.c
@@ -82,6 +82,7 @@
 #include <asm/system.h>
 #include <asm/uaccess.h>
 #include <asm/ioctls.h>
+#include <linux/bootmem.h>
 #include <linux/types.h>
 #include <linux/fcntl.h>
 #include <linux/module.h>

@@ -114,6 +115,11 @@ DEFINE_SNMP_STAT(struct udp_mib, udp_sta
 struct hlist_head udp_hash[UDP_HTABLE_SIZE];
 DEFINE_RWLOCK(udp_hash_lock);
 
+atomic_t udp_memory_allocated;
+int sysctl_udp_mem __read_mostly;
+int sysctl_udp_rmem_min __read_mostly;
+int sysctl_udp_wmem_min __read_mostly;
+
 static inline int __udp_lib_lport_inuse(__u16 num,
 					const struct hlist_head udptable[])
 {

@@ -1449,6 +1455,10 @@ struct proto udp_prot = {
 	.hash		   = udp_lib_hash,
 	.unhash		   = udp_lib_unhash,
 	.get_port	   = udp_v4_get_port,
+	.memory_allocated  = &udp_memory_allocated,
+	.sysctl_mem	   = &sysctl_udp_mem,
+	.sysctl_wmem	   = &sysctl_udp_wmem_min,
+	.sysctl_rmem	   = &sysctl_udp_rmem_min,
 	.obj_size	   = sizeof(struct udp_sock),
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_udp_setsockopt,

@@ -1644,6 +1654,23 @@ void udp4_proc_exit(void)
 }
 #endif /* CONFIG_PROC_FS */
 
+void __init udp_init(void)
+{
+	unsigned long limit;
+
+	/* Set the pressure threshold up by the same strategy of TCP. It is a
+	 * fraction of global memory that is up to 1/2 at 256 MB, decreasing
+	 * toward zero with the amount of memory, with a floor of 128 pages.
+	 */
+	limit = min(nr_all_pages, 1UL<<(28-PAGE_SHIFT)) >> (20-PAGE_SHIFT);
+	limit = (limit * (nr_all_pages >> (20-PAGE_SHIFT))) >> (PAGE_SHIFT-11);
+	limit = max(limit, 128UL);
+	sysctl_udp_mem = limit / 4 * 3;
+
+	sysctl_udp_rmem_min = SK_DATAGRAM_MEM_QUANTUM;
+	sysctl_udp_wmem_min = SK_DATAGRAM_MEM_QUANTUM;
+}
+
 EXPORT_SYMBOL(udp_disconnect);
 EXPORT_SYMBOL(udp_hash);
 EXPORT_SYMBOL(udp_hash_lock);

Signature · 1 line

Hitachi Computer Products (America) Inc.

Hideo AOKI

author

2007-12-15 05:22 UTC #6 lore raw

PATCH 4/4 : memory accounting in IPv4

In reply to Hideo AOKI

This patch adds UDP memory usage accounting in IPv4.

Send buffer accounting is performed by IP layer, because skbuff is allocated in the layer.

Receive buffer is charged, when the buffer successfully received. Destructor of the buffer does un charging and reclaiming, when the buffer is freed. To set destructor at proper place, we introduce udp_set_owner_r().

In addition, to make sure that sk_forward_alloc is totally uncharged in socket destruction, a reclaiming is added to inet_sock_destruct().

Cc Satoshi Oshima <[email protected]>

Signed-off-by Takahiro Yasui <[email protected]>

Signed-off-by Masami Hiramatsu <[email protected]>

Signed-off-by Hideo Aoki <[email protected]>

---

 af_inet.c   |    2 ++
 ip_output.c |   46 ++++++++++++++++++++++++++++++++++++++++++++--
 udp.c       |   16 +++++++++++++++-
 3 files changed, 61 insertions(+), 3 deletions(-)

net/ipv4/af_inet.c +2 -0

diff -pruN net-2.6-udp-take10a4-p3/net/ipv4/af_inet.c net-2.6-udp-take10a4-p4/net/ipv4/af_inet.c
@@ -144,6 +144,8 @@ void inet_sock_destruct(struct sock *sk)
 		printk("Attempt to release alive inet socket %p\n", sk);
 		return;
 	}
+	if (sk->sk_type == SOCK_DGRAM)
+		sk_datagram_mem_reclaim(sk);
 
 	BUG_TRAP(!atomic_read(&sk->sk_rmem_alloc));
 	BUG_TRAP(!atomic_read(&sk->sk_wmem_alloc));

net/ipv4/ip_output.c +44 -2

133 lines not on this page · raw message

net/ipv4/udp.c +15 -1

diff -pruN net-2.6-udp-take10a4-p3/net/ipv4/udp.c net-2.6-udp-take10a4-p4/net/ipv4/udp.c
@@ -934,6 +934,13 @@ int udp_disconnect(struct sock *sk, int
 	return 0;
 }
 
+void udp_set_owner_r(struct sk_buff *skb, struct sock *sk)
+{
+	skb->sk = sk;
+	skb->destructor = sk_datagram_rfree;
+	atomic_add(skb->truesize, &sk->sk_rmem_alloc);
+}
+
 /* returns:
  *  -1: error
  *   0: success

@@ -1022,10 +1029,17 @@ int udp_queue_rcv_skb(struct sock * sk,
 			goto drop;
 	}
 
-	if ((rc = sock_queue_rcv_skb(sk,skb)) < 0) {
+	if (!sk_account_rmem_charge(sk, skb->truesize)) {
+		UDP_INC_STATS_BH(UDP_MIB_RCVBUFERRORS, up->pcflag);
+		goto drop;
+	}
+
+	if ((rc = sock_queue_rcv_skb_with_owner(sk, skb, udp_set_owner_r)) < 0) {
 		/* Note that an ENOMEM error is charged twice */
 		if (rc == -ENOMEM)
 			UDP_INC_STATS_BH(UDP_MIB_RCVBUFERRORS, up->pcflag);
+		sk_account_uncharge(sk, skb->truesize);
+		sk_datagram_mem_reclaim(sk);
 		goto drop;
 	}

Signature · 1 line

Hitachi Computer Products (America) Inc.

David Miller

maintainer

2007-12-16 05:34 UTC #7 lore raw

In reply to Hideo AOKI

From: Hideo AOKI <[email protected]>
Date: Sat, 15 Dec 2007 00:07:44 -0500

Quoted 6 lines

Changelog take 9 -> take 10:
* supported using sk_forward_alloc
* introduced several memory accounting functions with spin lock
* changed detagram receive functions to be able to customize destructor
* fixed accounting bugs in previous takes

This is not what Herbert and I meant with our suggestion.

We meant to convert all of UDP and datagram handling to lock sockets precisely like TCP does, by calling lock_sock() on entry to functions like udp_recvmsg() and release_sock() on exit from those functions.

Then in the packet input processing, a sequence, just like TCP, such as:

	bh_lock_sock_nested(sk);
	if (!sock_owned_by_user(sk)) {
		udp_do_rcv(sk, skb);
	} else
		sk_add_backlog(sk, skb);

Then a suitably defined ->backlog_rcv is hooked up for these protocols as well.

Again, use TCP as a guide.

There is much more work involved to implement this properly, and make the accounting code sharable with TCP, than the simplistic and minimal spin lock code you added here.

Please do this correctly, thank you.

Hideo AOKI

author

2007-12-16 21:24 UTC #8 lore raw

Quoted 29 lines · David Miller

From: Hideo AOKI <[email protected]>
Date: Sat, 15 Dec 2007 00:07:44 -0500
Quoted 6 lines
Changelog take 9 -> take 10:
* supported using sk_forward_alloc
* introduced several memory accounting functions with spin lock
* changed detagram receive functions to be able to customize destructor
* fixed accounting bugs in previous takes
This is not what Herbert and I meant with our suggestion.
We meant to convert all of UDP and datagram handling to lock sockets precisely like TCP does, by calling lock_sock() on entry to functions like udp_recvmsg() and release_sock() on exit from those functions.
Then in the packet input processing, a sequence, just like TCP, such as:
	bh_lock_sock_nested(sk);
	if (!sock_owned_by_user(sk)) {
		udp_do_rcv(sk, skb);
	} else
		sk_add_backlog(sk, skb);
Then a suitably defined ->backlog_rcv is hooked up for these protocols as well.

Quoted 7 lines

Again, use TCP as a guide.
There is much more work involved to implement this properly, and make the accounting code sharable with TCP, than the simplistic and minimal spin lock code you added here.
Please do this correctly, thank you.

Hello,

I appreciate your suggestions.

I'll try to re-implement locking mechanism like TCP.

Regards,
Hideo

Signature · 1 line

Hitachi Computer Products (America) Inc.