2e685cad5790 build warning

2e685cad5790 build warning

[Johannes Weiner]

Hi Eric,

commit 2e685cad57906e19add7189b5ff49dfb6aaa21d3
Author: Eric W. Biederman <ebiederm@xmission.com>
Date:   Sat Oct 19 16:26:19 2013 -0700

    tcp_memcontrol: Kill struct tcp_memcontrol
    
    Replace the pointers in struct cg_proto with actual data fields and kill
    struct tcp_memcontrol as it is not fully redundant.
    
    This removes a confusing, unnecessary layer of abstraction.
    
    Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

triggers a build warning because it removed the only reference to a
function but not the function itself:

linux/net/ipv4/tcp_memcontrol.c:9:13: warning: ‘memcg_tcp_enter_memory_pressure’ defined but not used [-Wunused-function]
 static void memcg_tcp_enter_memory_pressure(struct sock *sk)

I can not see from the changelog why this function is no longer used,
or who is supposed to now set cg_proto->memory_pressure which you
still initialize etc.  Either way, the current state does not seem to
make much sense.  The author would be the best person to double check
such changes, but he wasn't copied on your patch, so I copied him now.

Apologies if this has been brought up before, I could not find any
reference on LKML of either this patch or a report of this warning.

Thanks!

Re: 2e685cad5790 build warning

[Johannes Weiner]

[ botched linux-kernel address in the first try ]

On Wed, Dec 04, 2013 at 05:29:43PM -0500, Johannes Weiner wrote:
> Hi Eric,
> 
> commit 2e685cad57906e19add7189b5ff49dfb6aaa21d3
> Author: Eric W. Biederman <ebiederm@xmission.com>
> Date:   Sat Oct 19 16:26:19 2013 -0700
> 
>     tcp_memcontrol: Kill struct tcp_memcontrol
>     
>     Replace the pointers in struct cg_proto with actual data fields and kill
>     struct tcp_memcontrol as it is not fully redundant.
>     
>     This removes a confusing, unnecessary layer of abstraction.
>     
>     Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> triggers a build warning because it removed the only reference to a
> function but not the function itself:
> 
> linux/net/ipv4/tcp_memcontrol.c:9:13: warning: ‘memcg_tcp_enter_memory_pressure’ defined but not used [-Wunused-function]
>  static void memcg_tcp_enter_memory_pressure(struct sock *sk)
> 
> I can not see from the changelog why this function is no longer used,
> or who is supposed to now set cg_proto->memory_pressure which you
> still initialize etc.  Either way, the current state does not seem to
> make much sense.  The author would be the best person to double check
> such changes, but he wasn't copied on your patch, so I copied him now.
> 
> Apologies if this has been brought up before, I could not find any
> reference on LKML of either this patch or a report of this warning.
> 
> Thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: 2e685cad5790 build warning

[David Miller]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: Text/Plain; charset=iso-8859-7, Size: 843 bytes --]

From: Johannes Weiner <hannes@cmpxchg.org>
Date: Wed, 4 Dec 2013 17:29:43 -0500

> linux/net/ipv4/tcp_memcontrol.c:9:13: warning: ¡memcg_tcp_enter_memory_pressure¢ defined but not used [-Wunused-function]
>  static void memcg_tcp_enter_memory_pressure(struct sock *sk)

Also, amusingly, although static there is an EXPORT_SYMBOL() for it.

> 
> I can not see from the changelog why this function is no longer used,
> or who is supposed to now set cg_proto->memory_pressure which you
> still initialize etc.  Either way, the current state does not seem to
> make much sense.  The author would be the best person to double check
> such changes, but he wasn't copied on your patch, so I copied him now.

Eric, please resolve this.
N‹§²æìr¸›zǧu©ž²Æ {\b­†éì¹»\x1c®&Þ–)îÆi¢žØ^n‡r¶‰šŽŠÝ¢j$½§$¢¸\x05¢¹¨­è§~Š'.)îÄÃ,yèm¶ŸÿÃ\f%Š{±šj+ƒðèž×¦j)Z†·Ÿ

[PATCH] tcp_memcontrol: Cleanup/fix cg_proto->memory_pressure handling.

[Eric W. Biederman]

kill memcg_tcp_enter_memory_pressure.  The only function of
memcg_tcp_enter_memory_pressure was to reduce deal with the
unnecessary abstraction that was tcp_memcontrol.  Now that struct
tcp_memcontrol is gone remove this unnecessary function, the
unnecessary function pointer, and modify sk_enter_memory_pressure to
set this field directly, just as sk_leave_memory_pressure cleas this
field directly.

This fixes a small bug I intruduced when killing struct tcp_memcontrol
that caused memcg_tcp_enter_memory_pressure to never be called and
thus failed to ever set cg_proto->memory_pressure.

Remove the cg_proto enter_memory_pressure function as it now serves
no useful purpose.

Don't test cg_proto->memory_presser in sk_leave_memory_pressure before
clearing it.  The test was originally there to ensure that the pointer
was non-NULL.  Now that cg_proto is not a pointer the pointer does not
matter.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 include/net/sock.h        |    6 ++----
 net/ipv4/tcp_memcontrol.c |    7 -------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index e3a18ff0c38b..2ef3c3eca47a 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1035,7 +1035,6 @@ enum cg_proto_flags {
 };
 
 struct cg_proto {
-	void			(*enter_memory_pressure)(struct sock *sk);
 	struct res_counter	memory_allocated;	/* Current allocated memory. */
 	struct percpu_counter	sockets_allocated;	/* Current number of sockets. */
 	int			memory_pressure;
@@ -1155,8 +1154,7 @@ static inline void sk_leave_memory_pressure(struct sock *sk)
 		struct proto *prot = sk->sk_prot;
 
 		for (; cg_proto; cg_proto = parent_cg_proto(prot, cg_proto))
-			if (cg_proto->memory_pressure)
-				cg_proto->memory_pressure = 0;
+			cg_proto->memory_pressure = 0;
 	}
 
 }
@@ -1171,7 +1169,7 @@ static inline void sk_enter_memory_pressure(struct sock *sk)
 		struct proto *prot = sk->sk_prot;
 
 		for (; cg_proto; cg_proto = parent_cg_proto(prot, cg_proto))
-			cg_proto->enter_memory_pressure(sk);
+			cg_proto->memory_pressure = 1;
 	}
 
 	sk->sk_prot->enter_memory_pressure(sk);
diff --git a/net/ipv4/tcp_memcontrol.c b/net/ipv4/tcp_memcontrol.c
index 03e9154f7e68..2c39f8f0dddf 100644
--- a/net/ipv4/tcp_memcontrol.c
+++ b/net/ipv4/tcp_memcontrol.c
@@ -6,13 +6,6 @@
 #include <linux/memcontrol.h>
 #include <linux/module.h>
 
-static void memcg_tcp_enter_memory_pressure(struct sock *sk)
-{
-	if (sk->sk_cgrp->memory_pressure)
-		sk->sk_cgrp->memory_pressure = 1;
-}
-EXPORT_SYMBOL(memcg_tcp_enter_memory_pressure);
-
 int tcp_init_cgroup(struct mem_cgroup *memcg, struct cgroup_subsys *ss)
 {
 	/*
-- 
1.7.5.4

Re: [PATCH] tcp_memcontrol: Cleanup/fix cg_proto->memory_pressure handling.

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Wed, 04 Dec 2013 20:12:04 -0800

> 
> kill memcg_tcp_enter_memory_pressure.  The only function of
> memcg_tcp_enter_memory_pressure was to reduce deal with the
> unnecessary abstraction that was tcp_memcontrol.  Now that struct
> tcp_memcontrol is gone remove this unnecessary function, the
> unnecessary function pointer, and modify sk_enter_memory_pressure to
> set this field directly, just as sk_leave_memory_pressure cleas this
> field directly.
> 
> This fixes a small bug I intruduced when killing struct tcp_memcontrol
> that caused memcg_tcp_enter_memory_pressure to never be called and
> thus failed to ever set cg_proto->memory_pressure.
> 
> Remove the cg_proto enter_memory_pressure function as it now serves
> no useful purpose.
> 
> Don't test cg_proto->memory_presser in sk_leave_memory_pressure before
> clearing it.  The test was originally there to ensure that the pointer
> was non-NULL.  Now that cg_proto is not a pointer the pointer does not
> matter.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Applied.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>