Skip to contentSkip to conversation

New interface for memory accounting (take 1)

PATCH In review Hideo AOKI 3 participants 12 messages Dec 30, 2007 – Dec 31, 2007

Subsystems (5)
lore
Share & tools
lore

Hideo AOKI

author
#0 lore raw

cover New interface for memory accounting (take 1)

Hello,

This patch set introduces new memory accounting interface. Current interface is written for stream protocols only. To enable memory accounting in other protocols (e.g. UDP), I enhanced the interface and updated TCP and SCTP memory accounting.

The patch set consists of the following 4 patches.

[1/4] introducing new memory accounting interface [2/4] adding memory accounting points to consolidate functions [3/4] updating TCP to use new interface
[4/4] updating SCTP to use new interface

The patch set was tested on net-2.6.25 tree.

Best regards,
Hideo Aoki

--
Hitachi Computer Products (America) Inc.

author
#1 lore raw

PATCH 1/4 : introducing new memory accounting interface

This patch introduces new memory accounting functions for each network protocol. Most of them are renamed from memory accounting functions for stream protocols. At the same time, some stream memory accounting functions are removed since other functions do same thing.

Renaming:

	sk_stream_free_skb()		->	sk_wmem_free_skb()
	__sk_stream_mem_reclaim()	->	__sk_mem_reclaim()
	sk_stream_mem_reclaim()		->	sk_mem_reclaim()
	sk_stream_mem_schedule 		->    	__sk_mem_schedule()
	sk_stream_pages()      		->	sk_mem_pages()
	sk_stream_rmem_schedule()	->	sk_rmem_schedule()
	sk_stream_wmem_schedule()	->	sk_wmem_schedule()
	sk_charge_skb()			->	sk_mem_charge()

Removeing
sk_stream_rfree(): consolidates into sock_rfree()
sk_stream_set_owner_r(): consolidates into skb_set_owner_r() sk_stream_mem_schedule()

The following functions are added.
sk_has_account(): check if the protocol supports accounting sk_mem_uncharge(): do the opposite of sk_mem_charge()

In addition, to achieve consolidation, updating sk_wmem_queued is removed from sk_mem_charge().

Cc Satoshi Oshima <[email protected]>
Cc Masami Hiramatsu <[email protected]>

---

 include/net/sock.h |   95 ++++++++++++++++++++++++++++++---------------------
 net/core/sock.c    |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/core/stream.c  |   82 --------------------------------------------
 3 files changed, 152 insertions(+), 122 deletions(-)
include/net/sock.h +55 -40
diff -pruN net-2.6.25/include/net/sock.h net-2.6.25-t12t19m-p1/include/net/sock.h
@@ -460,25 +460,6 @@ static inline int sk_stream_memory_free(
 	return sk->sk_wmem_queued < sk->sk_sndbuf;
 }
 
-extern void sk_stream_rfree(struct sk_buff *skb);
-
-static inline void sk_stream_set_owner_r(struct sk_buff *skb, struct sock *sk)
-{
-	skb->sk = sk;
-	skb->destructor = sk_stream_rfree;
-	atomic_add(skb->truesize, &sk->sk_rmem_alloc);
-	sk->sk_forward_alloc -= skb->truesize;
-}
-
-static inline void sk_stream_free_skb(struct sock *sk, struct sk_buff *skb)
-{
-	skb_truesize_check(skb);
-	sock_set_flag(sk, SOCK_QUEUE_SHRUNK);
-	sk->sk_wmem_queued   -= skb->truesize;
-	sk->sk_forward_alloc += skb->truesize;
-	__kfree_skb(skb);
-}
-
 /* The per-socket spinlock must be held here. */
 static inline void sk_add_backlog(struct sock *sk, struct sk_buff *skb)
 {
@@ -576,7 +557,7 @@ struct proto {
 	/*
 	 * Pressure flag: try to collapse.
 	 * Technical note: it is used by multiple contexts non atomically.
-	 * All the sk_stream_mem_schedule() is of this nature: accounting
+	 * All the __sk_mem_schedule() is of this nature: accounting
 	 * is strict, actions are advisory and have some latency.
 	 */
 	int			*memory_pressure;
@@ -712,33 +693,73 @@ static inline struct inode *SOCK_INODE(s
 	return &container_of(socket, struct socket_alloc, socket)->vfs_inode;
 }
 
-extern void __sk_stream_mem_reclaim(struct sock *sk);
-extern int sk_stream_mem_schedule(struct sock *sk, int size, int kind);
+/*
+ * Functions for memory accounting
+ */
+extern int __sk_mem_schedule(struct sock *sk, int size, int kind);
+extern void __sk_mem_reclaim(struct sock *sk);
 
-#define SK_STREAM_MEM_QUANTUM ((int)PAGE_SIZE)
-#define SK_STREAM_MEM_QUANTUM_SHIFT ilog2(SK_STREAM_MEM_QUANTUM)
+#define SK_MEM_QUANTUM ((int)PAGE_SIZE)
+#define SK_MEM_QUANTUM_SHIFT ilog2(SK_MEM_QUANTUM)
+#define SK_MEM_SEND	0
+#define SK_MEM_RECV	1
 
-static inline int sk_stream_pages(int amt)
+static inline int sk_mem_pages(int amt)
 {
-	return (amt + SK_STREAM_MEM_QUANTUM - 1) >> SK_STREAM_MEM_QUANTUM_SHIFT;
+	return (amt + SK_MEM_QUANTUM - 1) >> SK_MEM_QUANTUM_SHIFT;
 }
 
-static inline void sk_stream_mem_reclaim(struct sock *sk)
+static inline int sk_has_account(struct sock *sk)
 {
-	if (sk->sk_forward_alloc >= SK_STREAM_MEM_QUANTUM)
-		__sk_stream_mem_reclaim(sk);
+	/* return true if protocol supports memory accounting */
+	return !!sk->sk_prot->memory_allocated;
 }
 
-static inline int sk_stream_rmem_schedule(struct sock *sk, struct sk_buff *skb)
+static inline int sk_wmem_schedule(struct sock *sk, int size)
 {
-	return (int)skb->truesize <= sk->sk_forward_alloc ||
-		sk_stream_mem_schedule(sk, skb->truesize, 1);
+	if (!sk_has_account(sk))
+		return 1;
+	return size <= sk->sk_forward_alloc ||
+		__sk_mem_schedule(sk, size, SK_MEM_SEND);
 }
 
-static inline int sk_stream_wmem_schedule(struct sock *sk, int size)
+static inline int sk_rmem_schedule(struct sock *sk, int size)
 {
+	if (!sk_has_account(sk))
+		return 1;
 	return size <= sk->sk_forward_alloc ||
-	       sk_stream_mem_schedule(sk, size, 0);
+		__sk_mem_schedule(sk, size, SK_MEM_RECV);
+}
+
+static inline void sk_mem_reclaim(struct sock *sk)
+{
+	if (!sk_has_account(sk))
+		return;
+	if (sk->sk_forward_alloc >= SK_MEM_QUANTUM)
+		__sk_mem_reclaim(sk);
+}
+
+static inline void sk_mem_charge(struct sock *sk, int size)
+{
+	if (!sk_has_account(sk))
+		return;
+	sk->sk_forward_alloc -= size;
+}
+
+static inline void sk_mem_uncharge(struct sock *sk, int size)
+{
+	if (!sk_has_account(sk))
+		return;
+	sk->sk_forward_alloc += size;
+}
+
+static inline void sk_wmem_free_skb(struct sock *sk, struct sk_buff *skb)
+{
+	skb_truesize_check(skb);
+	sock_set_flag(sk, SOCK_QUEUE_SHRUNK);
+	sk->sk_wmem_queued -= skb->truesize;
+	sk_mem_uncharge(sk, skb->truesize);
+	__kfree_skb(skb);
 }
 
 /* Used by processes to "lock" a socket state, so that
@@ -1076,12 +1097,6 @@ static inline int sk_can_gso(const struc
 
 extern void sk_setup_caps(struct sock *sk, struct dst_entry *dst);
 
-static inline void sk_charge_skb(struct sock *sk, struct sk_buff *skb)
-{
-	sk->sk_wmem_queued   += skb->truesize;
-	sk->sk_forward_alloc -= skb->truesize;
-}
-
 static inline int skb_copy_to_page(struct sock *sk, char __user *from,
 				   struct sk_buff *skb, struct page *page,
 				   int off, int copy)
net/core/sock.c +97 -0

103 lines not on this page · raw message

net/core/stream.c +0 -82

94 lines not on this page · raw message

Signature · 1 line
Hitachi Computer Products (America) Inc.
author
#2 lore raw

PATCH 2/4 : adding memory accounting points

In reply to Hideo AOKI

To consolidate memory accounting functions, this patch adds memory accounting calls to network core functions. Moreover, present memory accounting call is renamed to new accounting call.

Cc Satoshi Oshima <[email protected]>
Cc Masami Hiramatsu <[email protected]>

---

 include/net/sock.h  |    3 ++-
 net/core/datagram.c |    2 ++
 net/core/sock.c     |    7 +++++++
 net/core/stream.c   |    2 +-
 4 files changed, 12 insertions(+), 2 deletions(-)
include/net/sock.h +2 -1
diff -pruN net-2.6.25-t12t19m-p1/include/net/sock.h net-2.6.25-t12t19m-p2/include/net/sock.h
@@ -1116,7 +1116,7 @@ static inline int skb_copy_to_page(struc
 	skb->data_len	     += copy;
 	skb->truesize	     += copy;
 	sk->sk_wmem_queued   += copy;
-	sk->sk_forward_alloc -= copy;
+	sk_mem_charge(sk, copy);
 	return 0;
 }
 
@@ -1142,6 +1142,7 @@ static inline void skb_set_owner_r(struc
 	skb->sk = sk;
 	skb->destructor = sock_rfree;
 	atomic_add(skb->truesize, &sk->sk_rmem_alloc);
+	sk_mem_charge(sk, skb->truesize);
 }
 
 extern void sk_reset_timer(struct sock *sk, struct timer_list* timer,
net/core/datagram.c +2 -0
diff -pruN net-2.6.25-t12t19m-p1/net/core/datagram.c net-2.6.25-t12t19m-p2/net/core/datagram.c
@@ -209,6 +209,7 @@ struct sk_buff *skb_recv_datagram(struct
 void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
 {
 	kfree_skb(skb);
+	sk_mem_reclaim(sk);
 }
 
 /**
@@ -248,6 +249,7 @@ int skb_kill_datagram(struct sock *sk, s
 	}
 
 	kfree_skb(skb);
+	sk_mem_reclaim(sk);
 	return err;
 }
 
net/core/sock.c +7 -0
diff -pruN net-2.6.25-t12t19m-p1/net/core/sock.c net-2.6.25-t12t19m-p2/net/core/sock.c
@@ -282,6 +282,11 @@ int sock_queue_rcv_skb(struct sock *sk,
 	if (err)
 		goto out;
 
+	if (!sk_rmem_schedule(sk, skb->truesize)) {
+		err = -ENOBUFS;
+		goto out;
+	}
+
 	skb->dev = NULL;
 	skb_set_owner_r(skb, sk);
 
@@ -1107,7 +1112,9 @@ void sock_rfree(struct sk_buff *skb)
 {
 	struct sock *sk = skb->sk;
 
+	skb_truesize_check(skb);
 	atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
+	sk_mem_uncharge(skb->sk, skb->truesize);
 }
 
 
net/core/stream.c +1 -1
diff -pruN net-2.6.25-t12t19m-p1/net/core/stream.c net-2.6.25-t12t19m-p2/net/core/stream.c
@@ -195,7 +195,7 @@ void sk_stream_kill_queues(struct sock *
 	BUG_TRAP(skb_queue_empty(&sk->sk_write_queue));
 
 	/* Account for returned memory. */
-	sk_stream_mem_reclaim(sk);
+	sk_mem_reclaim(sk);
 
 	BUG_TRAP(!sk->sk_wmem_queued);
 	BUG_TRAP(!sk->sk_forward_alloc);
Signature · 1 line
Hitachi Computer Products (America) Inc.
maintainer
#3 lore raw

Re: [PATCH 2/4] [CORE]: adding memory accounting points

From: Hideo AOKI <[email protected]>
Date: Sun, 30 Dec 2007 03:51:44 -0500

Quoted 8 lines

To consolidate memory accounting functions, this patch adds memory accounting calls to network core functions. Moreover, present memory accounting call is renamed to new accounting call.

Cc Satoshi Oshima <[email protected]>
Cc Masami Hiramatsu <[email protected]>

This patch would not apply, because is contained changes present in the first patch, specifically:

Quoted 13 lines

diff -pruN net-2.6.25-t12t19m-p1/include/net/sock.h net-2.6.25-t12t19m-p2/include/net/sock.h

include/net/sock.h +1 -1
@@ -1116,7 +1116,7 @@ static inline int skb_copy_to_page(struc
 	skb->data_len	     += copy;
 	skb->truesize	     += copy;
 	sk->sk_wmem_queued   += copy;
-	sk->sk_forward_alloc -= copy;
+	sk_mem_charge(sk, copy);
 	return 0;
 }
 

include/net/sock.h static inline void skb_set_owner_r(struc · line 1148

Quoted 7 lines
include/net/sock.h +1 -0
@@ -1142,6 +1142,7 @@ static inline void skb_set_owner_r(struc
 	skb->sk = sk;
 	skb->destructor = sock_rfree;
 	atomic_add(skb->truesize, &sk->sk_rmem_alloc);
+	sk_mem_charge(sk, skb->truesize);
 }
 
 extern void sk_reset_timer(struct sock *sk, struct timer_list* timer,

And now I see exactly what you did, and it is quite careless.

You wrote one big patch then tried to split it up by hand. This proves to me that you did not test the patches individually. Even worse, you did not even try to apply each patch nor compile the tree each step along the way as a basic sanity check.

This wastes a lot of my time, as well as the time of other developers who might want to try out and test your changes.

I will fix it up this time, but please do not ever do this again.

author
#4 lore raw

Re: [PATCH 2/4] [CORE]: adding memory accounting points

Quoted 2 lines · David Miller

This patch would not apply, because is contained changes present in the first patch, specifically:

<snip>

Quoted 6 lines

And now I see exactly what you did, and it is quite careless.

You wrote one big patch then tried to split it up by hand. This proves to me that you did not test the patches individually. Even worse, you did not even try to apply each patch nor compile the tree each step along the way as a basic sanity check.

Hello David,

You are right. Since I felt the patch was big, I divided into three for review. And I mistook during the dividing.

Quoted 2 lines

This wastes a lot of my time, as well as the time of other developers who might want to try out and test your changes.

I apologize for wasting your time.

Quoted 1 line

I will fix it up this time, but please do not ever do this again.

I really appreciate the fix. And I understood this.

Best regards,
Hideo

Signature · 1 line
Hitachi Computer Products (America) Inc.
author
#5 lore raw

PATCH 3/4 : using new interface

In reply to Hideo AOKI

This patch replaces present memory accounting calls with new interface in TCP.

Cc Satoshi Oshima <[email protected]>
Cc Masami Hiramatsu <[email protected]>

---

 include/net/tcp.h     |    4 ++--
 net/ipv4/tcp.c        |   23 ++++++++++++-----------
 net/ipv4/tcp_input.c  |   26 +++++++++++++-------------
 net/ipv4/tcp_output.c |   26 ++++++++++++++++----------
 net/ipv4/tcp_timer.c  |    8 ++++----
 5 files changed, 47 insertions(+), 40 deletions(-)
include/net/tcp.h +2 -2
diff -pruN net-2.6.25-t12t19m-p2/include/net/tcp.h net-2.6.25-t12t19m-p3/include/net/tcp.h
@@ -1196,8 +1196,8 @@ static inline void tcp_write_queue_purge
 	struct sk_buff *skb;
 
 	while ((skb = __skb_dequeue(&sk->sk_write_queue)) != NULL)
-		sk_stream_free_skb(sk, skb);
-	sk_stream_mem_reclaim(sk);
+		sk_wmem_free_skb(sk, skb);
+	sk_mem_reclaim(sk);
 }
 
 static inline struct sk_buff *tcp_write_queue_head(struct sock *sk)
net/ipv4/tcp.c +12 -11
diff -pruN net-2.6.25-t12t19m-p2/net/ipv4/tcp.c net-2.6.25-t12t19m-p3/net/ipv4/tcp.c
@@ -308,7 +308,7 @@ struct tcp_splice_state {
 /*
  * Pressure flag: try to collapse.
  * Technical note: it is used by multiple contexts non atomically.
- * All the sk_stream_mem_schedule() is of this nature: accounting
+ * All the __sk_mem_schedule() is of this nature: accounting
  * is strict, actions are advisory and have some latency.
  */
 int tcp_memory_pressure __read_mostly;
@@ -485,7 +485,8 @@ static inline void skb_entail(struct soc
 	tcb->sacked  = 0;
 	skb_header_release(skb);
 	tcp_add_write_queue_tail(sk, skb);
-	sk_charge_skb(sk, skb);
+	sk->sk_wmem_queued += skb->truesize;
+	sk_mem_charge(sk, skb->truesize);
 	if (tp->nonagle & TCP_NAGLE_PUSH)
 		tp->nonagle &= ~TCP_NAGLE_PUSH;
 }
@@ -638,7 +639,7 @@ struct sk_buff *sk_stream_alloc_skb(stru
 
 	skb = alloc_skb_fclone(size + sk->sk_prot->max_header, gfp);
 	if (skb) {
-		if (sk_stream_wmem_schedule(sk, skb->truesize)) {
+		if (sk_wmem_schedule(sk, skb->truesize)) {
 			/*
 			 * Make sure that we have exactly size bytes
 			 * available to the caller, no more, no less.
@@ -707,7 +708,7 @@ new_segment:
 			tcp_mark_push(tp, skb);
 			goto new_segment;
 		}
-		if (!sk_stream_wmem_schedule(sk, copy))
+		if (!sk_wmem_schedule(sk, copy))
 			goto wait_for_memory;
 
 		if (can_coalesce) {
@@ -721,7 +722,7 @@ new_segment:
 		skb->data_len += copy;
 		skb->truesize += copy;
 		sk->sk_wmem_queued += copy;
-		sk->sk_forward_alloc -= copy;
+		sk_mem_charge(sk, copy);
 		skb->ip_summed = CHECKSUM_PARTIAL;
 		tp->write_seq += copy;
 		TCP_SKB_CB(skb)->end_seq += copy;
@@ -928,7 +929,7 @@ new_segment:
 				if (copy > PAGE_SIZE - off)
 					copy = PAGE_SIZE - off;
 
-				if (!sk_stream_wmem_schedule(sk, copy))
+				if (!sk_wmem_schedule(sk, copy))
 					goto wait_for_memory;
 
 				if (!page) {
@@ -1019,7 +1020,7 @@ do_fault:
 		 * reset, where we can be unlinking the send_head.
 		 */
 		tcp_check_send_head(sk, skb);
-		sk_stream_free_skb(sk, skb);
+		sk_wmem_free_skb(sk, skb);
 	}
 
 do_error:
@@ -1738,7 +1739,7 @@ void tcp_close(struct sock *sk, long tim
 		__kfree_skb(skb);
 	}
 
-	sk_stream_mem_reclaim(sk);
+	sk_mem_reclaim(sk);
 
 	/* As outlined in RFC 2525, section 2.17, we send a RST here because
 	 * data was lost. To witness the awful effects of the old behavior of
@@ -1841,7 +1842,7 @@ adjudge_to_death:
 		}
 	}
 	if (sk->sk_state != TCP_CLOSE) {
-		sk_stream_mem_reclaim(sk);
+		sk_mem_reclaim(sk);
 		if (tcp_too_many_orphans(sk,
 				atomic_read(sk->sk_prot->orphan_count))) {
 			if (net_ratelimit())
@@ -2658,11 +2659,11 @@ void __init tcp_init(void)
 	limit = ((unsigned long)sysctl_tcp_mem[1]) << (PAGE_SHIFT - 7);
 	max_share = min(4UL*1024*1024, limit);
 
-	sysctl_tcp_wmem[0] = SK_STREAM_MEM_QUANTUM;
+	sysctl_tcp_wmem[0] = SK_MEM_QUANTUM;
 	sysctl_tcp_wmem[1] = 16*1024;
 	sysctl_tcp_wmem[2] = max(64*1024, max_share);
 
-	sysctl_tcp_rmem[0] = SK_STREAM_MEM_QUANTUM;
+	sysctl_tcp_rmem[0] = SK_MEM_QUANTUM;
 	sysctl_tcp_rmem[1] = 87380;
 	sysctl_tcp_rmem[2] = max(87380, max_share);
 
net/ipv4/tcp_input.c +13 -13
diff -pruN net-2.6.25-t12t19m-p2/net/ipv4/tcp_input.c net-2.6.25-t12t19m-p3/net/ipv4/tcp_input.c
@@ -591,7 +591,7 @@ static void tcp_event_data_recv(struct s
 			 * restart window, so that we send ACKs quickly.
 			 */
 			tcp_incr_quickack(sk);
-			sk_stream_mem_reclaim(sk);
+			sk_mem_reclaim(sk);
 		}
 	}
 	icsk->icsk_ack.lrcvtime = now;
@@ -2848,7 +2848,7 @@ static int tcp_clean_rtx_queue(struct so
 			break;
 
 		tcp_unlink_write_queue(skb, sk);
-		sk_stream_free_skb(sk, skb);
+		sk_wmem_free_skb(sk, skb);
 		tcp_clear_all_retrans_hints(tp);
 	}
 
@@ -3564,7 +3564,7 @@ static void tcp_fin(struct sk_buff *skb,
 	__skb_queue_purge(&tp->out_of_order_queue);
 	if (tcp_is_sack(tp))
 		tcp_sack_reset(&tp->rx_opt);
-	sk_stream_mem_reclaim(sk);
+	sk_mem_reclaim(sk);
 
 	if (!sock_flag(sk, SOCK_DEAD)) {
 		sk->sk_state_change(sk);
@@ -3847,12 +3847,12 @@ static void tcp_data_queue(struct sock *
 queue_and_out:
 			if (eaten < 0 &&
 			    (atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf ||
-			     !sk_stream_rmem_schedule(sk, skb))) {
+			     !sk_rmem_schedule(sk, skb->truesize))) {
 				if (tcp_prune_queue(sk) < 0 ||
-				    !sk_stream_rmem_schedule(sk, skb))
+				    !sk_rmem_schedule(sk, skb->truesize))
 					goto drop;
 			}
-			sk_stream_set_owner_r(skb, sk);
+			skb_set_owner_r(skb, sk);
 			__skb_queue_tail(&sk->sk_receive_queue, skb);
 		}
 		tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
@@ -3921,9 +3921,9 @@ drop:
 	TCP_ECN_check_ce(tp, skb);
 
 	if (atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf ||
-	    !sk_stream_rmem_schedule(sk, skb)) {
+	    !sk_rmem_schedule(sk, skb->truesize)) {
 		if (tcp_prune_queue(sk) < 0 ||
-		    !sk_stream_rmem_schedule(sk, skb))
+		    !sk_rmem_schedule(sk, skb->truesize))
 			goto drop;
 	}
 
@@ -3934,7 +3934,7 @@ drop:
 	SOCK_DEBUG(sk, "out of order segment: rcv_next %X seq %X - %X\n",
 		   tp->rcv_nxt, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
 
-	sk_stream_set_owner_r(skb, sk);
+	skb_set_owner_r(skb, sk);
 
 	if (!skb_peek(&tp->out_of_order_queue)) {
 		/* Initial out of order segment, build 1 SACK. */
@@ -4076,7 +4076,7 @@ tcp_collapse(struct sock *sk, struct sk_
 		memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
 		TCP_SKB_CB(nskb)->seq = TCP_SKB_CB(nskb)->end_seq = start;
 		__skb_insert(nskb, skb->prev, skb, list);
-		sk_stream_set_owner_r(nskb, sk);
+		skb_set_owner_r(nskb, sk);
 
 		/* Copy data, releasing collapsed skbs. */
 		while (copy > 0) {
@@ -4174,7 +4174,7 @@ static int tcp_prune_queue(struct sock *
 		     sk->sk_receive_queue.next,
 		     (struct sk_buff*)&sk->sk_receive_queue,
 		     tp->copied_seq, tp->rcv_nxt);
-	sk_stream_mem_reclaim(sk);
+	sk_mem_reclaim(sk);
 
 	if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf)
 		return 0;
@@ -4194,7 +4194,7 @@ static int tcp_prune_queue(struct sock *
 		 */
 		if (tcp_is_sack(tp))
 			tcp_sack_reset(&tp->rx_opt);
-		sk_stream_mem_reclaim(sk);
+		sk_mem_reclaim(sk);
 	}
 
 	if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf)
@@ -4696,7 +4696,7 @@ int tcp_rcv_established(struct sock *sk,
 				/* Bulk data transfer: receiver */
 				__skb_pull(skb,tcp_header_len);
 				__skb_queue_tail(&sk->sk_receive_queue, skb);
-				sk_stream_set_owner_r(skb, sk);
+				skb_set_owner_r(skb, sk);
 				tp->rcv_nxt = TCP_SKB_CB(skb)->end_seq;
 			}
 
net/ipv4/tcp_output.c +16 -10
diff -pruN net-2.6.25-t12t19m-p2/net/ipv4/tcp_output.c net-2.6.25-t12t19m-p3/net/ipv4/tcp_output.c
@@ -637,7 +637,8 @@ static void tcp_queue_skb(struct sock *s
 	tp->write_seq = TCP_SKB_CB(skb)->end_seq;
 	skb_header_release(skb);
 	tcp_add_write_queue_tail(sk, skb);
-	sk_charge_skb(sk, skb);
+	sk->sk_wmem_queued += skb->truesize;
+	sk_mem_charge(sk, skb->truesize);
 }
 
 static void tcp_set_skb_tso_segs(struct sock *sk, struct sk_buff *skb, unsigned int mss_now)
@@ -701,7 +702,8 @@ int tcp_fragment(struct sock *sk, struct
 	if (buff == NULL)
 		return -ENOMEM; /* We'll just try again later. */
 
-	sk_charge_skb(sk, buff);
+	sk->sk_wmem_queued += buff->truesize;
+	sk_mem_charge(sk, buff->truesize);
 	nlen = skb->len - len - nsize;
 	buff->truesize += nlen;
 	skb->truesize -= nlen;
@@ -825,7 +827,7 @@ int tcp_trim_head(struct sock *sk, struc
 
 	skb->truesize	     -= len;
 	sk->sk_wmem_queued   -= len;
-	sk->sk_forward_alloc += len;
+	sk_mem_uncharge(sk, len);
 	sock_set_flag(sk, SOCK_QUEUE_SHRUNK);
 
 	/* Any change of skb->len requires recalculation of tso
@@ -1197,7 +1199,8 @@ static int tso_fragment(struct sock *sk,
 	if (unlikely(buff == NULL))
 		return -ENOMEM;
 
-	sk_charge_skb(sk, buff);
+	sk->sk_wmem_queued += buff->truesize;
+	sk_mem_charge(sk, buff->truesize);
 	buff->truesize += nlen;
 	skb->truesize -= nlen;
 
@@ -1350,7 +1353,8 @@ static int tcp_mtu_probe(struct sock *sk
 	/* We're allowed to probe.  Build it now. */
 	if ((nskb = sk_stream_alloc_skb(sk, probe_size, GFP_ATOMIC)) == NULL)
 		return -1;
-	sk_charge_skb(sk, nskb);
+	sk->sk_wmem_queued += nskb->truesize;
+	sk_mem_charge(sk, nskb->truesize);
 
 	skb = tcp_send_head(sk);
 
@@ -1377,7 +1381,7 @@ static int tcp_mtu_probe(struct sock *sk
 			 * Throw it away. */
 			TCP_SKB_CB(nskb)->flags |= TCP_SKB_CB(skb)->flags;
 			tcp_unlink_write_queue(skb, sk);
-			sk_stream_free_skb(sk, skb);
+			sk_wmem_free_skb(sk, skb);
 		} else {
 			TCP_SKB_CB(nskb)->flags |= TCP_SKB_CB(skb)->flags &
 						   ~(TCPCB_FLAG_FIN|TCPCB_FLAG_PSH);
@@ -1744,7 +1748,7 @@ static void tcp_retrans_try_collapse(str
 		/* changed transmit queue under us so clear hints */
 		tcp_clear_retrans_hints_partial(tp);
 
-		sk_stream_free_skb(sk, next_skb);
+		sk_wmem_free_skb(sk, next_skb);
 	}
 }
 
@@ -2139,8 +2143,9 @@ int tcp_send_synack(struct sock *sk)
 			tcp_unlink_write_queue(skb, sk);
 			skb_header_release(nskb);
 			__tcp_add_write_queue_head(sk, nskb);
-			sk_stream_free_skb(sk, skb);
-			sk_charge_skb(sk, nskb);
+			sk_wmem_free_skb(sk, skb);
+			sk->sk_wmem_queued += nskb->truesize;
+			sk_mem_charge(sk, nskb->truesize);
 			skb = nskb;
 		}
 
@@ -2343,7 +2348,8 @@ int tcp_connect(struct sock *sk)
 	tp->retrans_stamp = TCP_SKB_CB(buff)->when;
 	skb_header_release(buff);
 	__tcp_add_write_queue_tail(sk, buff);
-	sk_charge_skb(sk, buff);
+	sk->sk_wmem_queued += buff->truesize;
+	sk_mem_charge(sk, buff->truesize);
 	tp->packets_out += tcp_skb_pcount(buff);
 	tcp_transmit_skb(sk, buff, 1, GFP_KERNEL);
 
net/ipv4/tcp_timer.c +4 -4
diff -pruN net-2.6.25-t12t19m-p2/net/ipv4/tcp_timer.c net-2.6.25-t12t19m-p3/net/ipv4/tcp_timer.c
@@ -186,7 +186,7 @@ static void tcp_delack_timer(unsigned lo
 		goto out_unlock;
 	}
 
-	sk_stream_mem_reclaim(sk);
+	sk_mem_reclaim(sk);
 
 	if (sk->sk_state == TCP_CLOSE || !(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
 		goto out;
@@ -226,7 +226,7 @@ static void tcp_delack_timer(unsigned lo
 
 out:
 	if (tcp_memory_pressure)
-		sk_stream_mem_reclaim(sk);
+		sk_mem_reclaim(sk);
 out_unlock:
 	bh_unlock_sock(sk);
 	sock_put(sk);
@@ -420,7 +420,7 @@ static void tcp_write_timer(unsigned lon
 	TCP_CHECK_TIMER(sk);
 
 out:
-	sk_stream_mem_reclaim(sk);
+	sk_mem_reclaim(sk);
 out_unlock:
 	bh_unlock_sock(sk);
 	sock_put(sk);
@@ -514,7 +514,7 @@ static void tcp_keepalive_timer (unsigne
 	}
 
 	TCP_CHECK_TIMER(sk);
-	sk_stream_mem_reclaim(sk);
+	sk_mem_reclaim(sk);
 
 resched:
 	inet_csk_reset_keepalive_timer (sk, elapsed);
Signature · 1 line
Hitachi Computer Products (America) Inc.
author
#6 lore raw

PATCH 4/4 : using new interface

In reply to Hideo AOKI

This patch replaces present memory accounting calls with new interface in SCTP.

Cc Satoshi Oshima <[email protected]>
Cc Masami Hiramatsu <[email protected]>

---

 include/net/sctp/sctp.h |    3 +--
 net/sctp/protocol.c     |    2 +-
 net/sctp/sm_statefuns.c |    2 +-
 net/sctp/socket.c       |   11 ++++++-----
 net/sctp/ulpevent.c     |    2 +-
 net/sctp/ulpqueue.c     |    2 +-
 6 files changed, 11 insertions(+), 11 deletions(-)
include/net/sctp/sctp.h +1 -2
diff -pruN net-2.6.25-t12t19m-p3/include/net/sctp/sctp.h net-2.6.25-t12t19m-p4/include/net/sctp/sctp.h
@@ -463,8 +463,7 @@ static inline void sctp_skb_set_owner_r(
 	skb->destructor = sctp_sock_rfree;
 	atomic_add(event->rmem_len, &sk->sk_rmem_alloc);
 	/*
-	 * This mimics the behavior of
-	 * sk_stream_set_owner_r
+	 * This mimics the behavior of skb_set_owner_r
 	 */
 	sk->sk_forward_alloc -= event->rmem_len;
 }
net/sctp/protocol.c +1 -1
diff -pruN net-2.6.25-t12t19m-p3/net/sctp/protocol.c net-2.6.25-t12t19m-p4/net/sctp/protocol.c
@@ -1109,7 +1109,7 @@ SCTP_STATIC __init int sctp_init(void)
 	sysctl_sctp_rmem[1] = (1500 *(sizeof(struct sk_buff) + 1));
 	sysctl_sctp_rmem[2] = max(sysctl_sctp_rmem[1], max_share);
 
-	sysctl_sctp_wmem[0] = SK_STREAM_MEM_QUANTUM;
+	sysctl_sctp_wmem[0] = SK_MEM_QUANTUM;
 	sysctl_sctp_wmem[1] = 16*1024;
 	sysctl_sctp_wmem[2] = max(64*1024, max_share);
 
net/sctp/sm_statefuns.c +1 -1
diff -pruN net-2.6.25-t12t19m-p3/net/sctp/sm_statefuns.c net-2.6.25-t12t19m-p4/net/sctp/sm_statefuns.c
@@ -5866,7 +5866,7 @@ static int sctp_eat_data(const struct sc
 	/*
 	 * Also try to renege to limit our memory usage in the event that
 	 * we are under memory pressure
-	 * If we can't renege, don't worry about it, the sk_stream_rmem_schedule
+	 * If we can't renege, don't worry about it, the sk_rmem_schedule
 	 * in sctp_ulpevent_make_rcvmsg will drop the frame if we grow our
 	 * memory usage too much
 	 */
net/sctp/socket.c +6 -5
diff -pruN net-2.6.25-t12t19m-p3/net/sctp/socket.c net-2.6.25-t12t19m-p4/net/sctp/socket.c
@@ -174,7 +174,8 @@ static inline void sctp_set_owner_w(stru
 				sizeof(struct sctp_chunk);
 
 	atomic_add(sizeof(struct sctp_chunk), &sk->sk_wmem_alloc);
-	sk_charge_skb(sk, chunk->skb);
+	sk->sk_wmem_queued += chunk->skb->truesize;
+	sk_mem_charge(sk, chunk->skb->truesize);
 }
 
 /* Verify that this is a valid address. */
@@ -6035,10 +6036,10 @@ static void sctp_wfree(struct sk_buff *s
 	atomic_sub(sizeof(struct sctp_chunk), &sk->sk_wmem_alloc);
 
 	/*
-	 * This undoes what is done via sk_charge_skb
+	 * This undoes what is done via sctp_set_owner_w and sk_mem_charge
 	 */
 	sk->sk_wmem_queued   -= skb->truesize;
-	sk->sk_forward_alloc += skb->truesize;
+	sk_mem_uncharge(sk, skb->truesize);
 
 	sock_wfree(skb);
 	__sctp_write_space(asoc);
@@ -6059,9 +6060,9 @@ void sctp_sock_rfree(struct sk_buff *skb
 	atomic_sub(event->rmem_len, &sk->sk_rmem_alloc);
 
 	/*
-	 * Mimic the behavior of sk_stream_rfree
+	 * Mimic the behavior of sock_rfree
 	 */
-	sk->sk_forward_alloc += event->rmem_len;
+	sk_mem_uncharge(sk, event->rmem_len);
 }
 
 
net/sctp/ulpevent.c +1 -1
diff -pruN net-2.6.25-t12t19m-p3/net/sctp/ulpevent.c net-2.6.25-t12t19m-p4/net/sctp/ulpevent.c
@@ -700,7 +700,7 @@ struct sctp_ulpevent *sctp_ulpevent_make
 	if (rx_count >= asoc->base.sk->sk_rcvbuf) {
 
 		if ((asoc->base.sk->sk_userlocks & SOCK_RCVBUF_LOCK) ||
-		   (!sk_stream_rmem_schedule(asoc->base.sk, chunk->skb)))
+		    (!sk_rmem_schedule(asoc->base.sk, chunk->skb->truesize)))
 			goto fail;
 	}
 
net/sctp/ulpqueue.c +1 -1
diff -pruN net-2.6.25-t12t19m-p3/net/sctp/ulpqueue.c net-2.6.25-t12t19m-p4/net/sctp/ulpqueue.c
@@ -1046,7 +1046,7 @@ void sctp_ulpq_renege(struct sctp_ulpq *
 		sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
 	}
 
-	sk_stream_mem_reclaim(asoc->base.sk);
+	sk_mem_reclaim(asoc->base.sk);
 	return;
 }
 
Signature · 1 line
Hitachi Computer Products (America) Inc.
maintainer
#7 lore raw
In reply to Hideo AOKI

From: Hideo AOKI <[email protected]>
Date: Sun, 30 Dec 2007 03:47:33 -0500

Quoted 8 lines

Hello,

This patch set introduces new memory accounting interface. Current interface is written for stream protocols only. To enable memory accounting in other protocols (e.g. UDP), I enhanced the interface and updated TCP and SCTP memory accounting.

Quoted 8 lines

The patch set consists of the following 4 patches.

[1/4] introducing new memory accounting interface [2/4] adding memory accounting points to consolidate functions [3/4] updating TCP to use new interface
[4/4] updating SCTP to use new interface

The patch set was tested on net-2.6.25 tree.

I like this work very much and will add this to net-2.6.25 But I will have to combine it all into one patch.

You cannot have one patch which breaks the build in any way. All of the kernel must build properly after each patch in your patchset is applied.

Since patch 1 renames all of the sk_stream_*() functions, TCP and SCTP stop building.

We enforce this rule, otherwise when users try to use "git bisect" to find out where regressions are added, they will get stuck in places like this where the tree will not build due to such careless changesets.

Thank you.

#8 lore raw
Quoted 27 lines · David Miller

From: Hideo AOKI <[email protected]>
Date: Sun, 30 Dec 2007 03:47:33 -0500

Quoted 8 lines

Hello,

This patch set introduces new memory accounting interface. Current interface is written for stream protocols only. To enable memory accounting in other protocols (e.g. UDP), I enhanced the interface and updated TCP and SCTP memory accounting.

Quoted 8 lines

The patch set consists of the following 4 patches.

[1/4] introducing new memory accounting interface [2/4] adding memory accounting points to consolidate functions [3/4] updating TCP to use new interface
[4/4] updating SCTP to use new interface

The patch set was tested on net-2.6.25 tree.

I like this work very much and will add this to net-2.6.25 But I will have to combine it all into one patch.

You cannot have one patch which breaks the build in any way. All of the kernel must build properly after each patch in your patchset is applied.

Quoted 7 lines

Since patch 1 renames all of the sk_stream_*() functions, TCP and SCTP stop building.

We enforce this rule, otherwise when users try to use "git bisect" to find out where regressions are added, they will get stuck in places like this where the tree will not build due to such careless changesets.

Hi David

Could you add the following patch, because it apparently was lost during the battle :)

Thank you

[PATCH] use SK_MEM_QUANTUM_SHIFT in __sk_mem_reclaim()

Avoid an expensive divide (as done in commit 18030477e70a826b91608aee40a987bbd368fec6 but lost in commit 23821d2653111d20e75472c8c5003df1a55309a8)

net/core/sock.c +1 -1
index 8c184c4..3804e7d 100644
@@ -1476,7 +1476,7 @@ void __sk_mem_reclaim(struct sock *sk)
 {
 	struct proto *prot = sk->sk_prot;
 
-	atomic_sub(sk->sk_forward_alloc / SK_MEM_QUANTUM,
+	atomic_sub(sk->sk_forward_alloc >> SK_MEM_QUANTUM_SHIFT,
 		   prot->memory_allocated);
 	sk->sk_forward_alloc &= SK_MEM_QUANTUM - 1;
 
author
#9 lore raw
Quoted 5 lines · Eric Dumazet

Hi David

Could you add the following patch, because it apparently was lost during the battle :)

Quoted 7 lines

Thank you

[PATCH] use SK_MEM_QUANTUM_SHIFT in __sk_mem_reclaim()

Avoid an expensive divide (as done in commit 18030477e70a826b91608aee40a987bbd368fec6 but lost in commit 23821d2653111d20e75472c8c5003df1a55309a8)

Hello Eric,

Thank you for catching this. I'm sorry about the lost.

Best regards,
Hideo

Signature · 1 line
Hitachi Computer Products (America) Inc.
maintainer
#10 lore raw
In reply to Eric Dumazet

From: Eric Dumazet <[email protected]>
Date: Mon, 31 Dec 2007 16:17:37 +0100

Quoted 5 lines

Could you add the following patch, because it apparently was lost during the battle :)

Thank you

Quoted 8 lines

[PATCH] use SK_MEM_QUANTUM_SHIFT in __sk_mem_reclaim()

Avoid an expensive divide (as done in commit 18030477e70a826b91608aee40a987bbd368fec6 but lost in commit 23821d2653111d20e75472c8c5003df1a55309a8)

Sigh :-/

Thanks for catching this Eric, patch applied.

author
#11 lore raw
In reply to David Miller

Hello David,

Quoted 13 lines · David Miller

From: Hideo AOKI <[email protected]>
Date: Sun, 30 Dec 2007 03:47:33 -0500

Quoted 6 lines

The patch set consists of the following 4 patches.

[1/4] introducing new memory accounting interface [2/4] adding memory accounting points to consolidate functions [3/4] updating TCP to use new interface
[4/4] updating SCTP to use new interface

I like this work very much and will add this to net-2.6.25 But I will have to combine it all into one patch.

Quoted 6 lines

You cannot have one patch which breaks the build in any way. All of the kernel must build properly after each patch in your patchset is applied.

Since patch 1 renames all of the sk_stream_*() functions, TCP and SCTP stop building.

That's correct.

Quoted 4 lines

We enforce this rule, otherwise when users try to use "git bisect" to find out where regressions are added, they will get stuck in places like this where the tree will not build due to such careless changesets.

Thank you for your explanation. To be honest, I didn't know the rule exactly. I am so sorry for sending inconvenient patch set.

Best regards,
Hideo

Signature · 1 line
Hitachi Computer Products (America) Inc.