ARM: mostly harmless gcc warnings

On NOMMU ARM, the __addr_ok() and __range_ok() macros do not evaluate their arguments, which may lead to harmless build warnings in some code where the variables are not used otherwise. Adding a cast to void gets rid of the warning and does not make any semantic changes.

Without this patch, building at91x40_defconfig results in:

fs/read_write.c: In function 'rw_copy_check_uvector':
fs/read_write.c:684:9: warning: unused variable 'buf' [-Wunused-variable]

Signed-off-by Arnd Bergmann <[email protected]>

Cc Greg Ungerer <[email protected]>

Cc Russell King <[email protected]>

---
  arch/arm/include/asm/uaccess.h |    4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/include/asm/uaccess.h +2 -2

index 77bd79f..7e1f760 100644
@@ -200,8 +200,8 @@ extern int __put_user_8(void *, unsigned long long);
  #define USER_DS			KERNEL_DS
 
  #define segment_eq(a,b)		(1)
-#define __addr_ok(addr)		(1)
-#define __range_ok(addr,size)	(0)
+#define __addr_ok(addr)		((void)(addr),1)
+#define __range_ok(addr,size)	((void)(addr),0)
  #define get_fs()		(KERNEL_DS)
 
  static inline void set_fs(mm_segment_t fs)

Quoted 7 lines · Arnd Bergmann

On NOMMU ARM, the __addr_ok() and __range_ok() macros do not evaluate their arguments, which may lead to harmless build warnings in some code where the variables are not used otherwise. Adding a cast to void gets rid of the warning and does not make any semantic changes.

Without this patch, building at91x40_defconfig results in:

Quoted 6 lines

fs/read_write.c: In function 'rw_copy_check_uvector':
fs/read_write.c:684:9: warning: unused variable 'buf' [-Wunused-variable]

Signed-off-by Arnd Bergmann <[email protected]>

Cc Greg Ungerer <[email protected]>

Cc Russell King <[email protected]>

Acked-by Greg Ungerer <[email protected]>

I can pick it up and push to the arm-soc tree.

The flat_get_addr_from_rp() macro does not use the 'persistent' argument on ARM, causing a harmless compiler warning. A cast to void removes that warning.

Without this patch, building at91x40_defconfig results in:

fs/binfmt_flat.c: In function 'load_flat_file':
fs/binfmt_flat.c:746:17: warning: unused variable 'persistent' [-Wunused-variable]

Signed-off-by Arnd Bergmann <[email protected]>

Cc Greg Ungerer <[email protected]>

Cc Russell King <[email protected]>

Cc Bryan Wu <[email protected]>

---
  arch/arm/include/asm/flat.h |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/include/asm/flat.h +1 -1

index 59426a4..e847d23 100644
@@ -8,7 +8,7 @@
  #define	flat_argvp_envp_on_stack()		1
  #define	flat_old_ram_flag(flags)		(flags)
  #define	flat_reloc_valid(reloc, size)		((reloc) <= (size))
-#define	flat_get_addr_from_rp(rp, relval, flags, persistent) get_unaligned(rp)
+#define	flat_get_addr_from_rp(rp, relval, flags, persistent) ((void)persistent,get_unaligned(rp))
  #define	flat_put_addr_at_rp(rp, val, relval)	put_unaligned(val,rp)
  #define	flat_get_relocate_addr(rel)		(rel)
  #define	flat_set_persistent(relval, p)		0

The follow_link() function always initializes its *p argument, or returns an error, but not all versions of gcc figure this out, so we have to work around this using the uninitialized_var() macro.

Without this patch, building with arm-linux-gnueabi-gcc-4.6 results in:

fs/namei.c: In function 'link_path_walk':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized] fs/namei.c:1544:9: note: 'cookie' was declared here
fs/namei.c: In function 'path_lookupat':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized] fs/namei.c:1934:10: note: 'cookie' was declared here
fs/namei.c: In function 'path_openat':
fs/namei.c:649:24: warning: 'cookie' may be used uninitialized in this function [-Wuninitialized] fs/namei.c:2899:9: note: 'cookie' was declared here

Signed-off-by Arnd Bergmann <[email protected]>

Cc Al Viro <[email protected]>

Cc linux-fsdevel at vger.kernel.org

Cc Jan Kara <[email protected]>

---
 fs/namei.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

fs/namei.c +3 -3

index dd1ed1b..62a1725 100644
@@ -1543,7 +1543,7 @@ static inline int nested_symlink(struct path *path, struct nameidata *nd)
 
 	do {
 		struct path link = *path;
-		void *cookie;
+		void *uninitialized_var(cookie);
 
 		res = follow_link(&link, nd, &cookie);
 		if (res)

@@ -1933,7 +1933,7 @@ static int path_lookupat(int dfd, const char *name,
 	if (!err && !(flags & LOOKUP_PARENT)) {
 		err = lookup_last(nd, &path);
 		while (err > 0) {
-			void *cookie;
+			void *uninitialized_var(cookie);
 			struct path link = path;
 			err = may_follow_link(&link, nd);
 			if (unlikely(err))

@@ -2902,7 +2902,7 @@ static struct file *path_openat(int dfd, const char *pathname,
 	error = do_last(nd, &path, file, op, &opened, pathname);
 	while (unlikely(error > 0)) { /* trailing symlink */
 		struct path link = path;
-		void *cookie;
+		void *uninitialized_var(cookie);
 		if (!(nd->flags & LOOKUP_FOLLOW)) {
 			path_put_conditional(&path, nd);
 			path_put(&nd->path);

Signature · 1 line

1.7.10

Quoted 4 lines · Arnd Bergmann

The follow_link() function always initializes its *p argument, or returns an error, but not all versions of gcc figure this out, so we have to work around this using the uninitialized_var() macro.

Well, I'm somewhat sceptical to this approach. I agree that bogus
warnings are not nice but later when the code is changed and possibly real use without initialization is added, we won't notice it. Without changing anything, we'd at least have a chance of catching it with gcc versions which were clever enough to not warn with the original code. Or alternatively if we unconditionally initialized the variable that would get rid of the warning and made the code more future-proof (that's what I usually end up doing)... I don't really care that much about the chosen solution, Al is the one to decide. But I wanted to point out there are downsides to your solution.

Quoted 5 lines · Jan Kara

Quoted 4 lines · Arnd Bergmann

The follow_link() function always initializes its *p argument, or returns an error, but not all versions of gcc figure this out, so we have to work around this using the uninitialized_var() macro.

Quoted 10 lines

Well, I'm somewhat sceptical to this approach. I agree that bogus
warnings are not nice but later when the code is changed and possibly real use without initialization is added, we won't notice it. Without changing anything, we'd at least have a chance of catching it with gcc versions which were clever enough to not warn with the original code. Or alternatively if we unconditionally initialized the variable that would get rid of the warning and made the code more future-proof (that's what I usually end up doing)... I don't really care that much about the chosen solution, Al is the one to decide. But I wanted to point out there are downsides to your solution.

I'll drop the patch for now and won't send it from my tree then. I agree that uninitialized_var() is not ideal, but none of the alternatives seemed better.

With my latest compiler, I don't actually see the warnings any more, so maybe someone fixed gcc instead, or this went away after another change. I'll let you know if it comes back so we can discuss about a better fix then.

Quoted 17 lines · Arnd Bergmann

Quoted 5 lines · Jan Kara

Quoted 4 lines · Arnd Bergmann

The follow_link() function always initializes its *p argument, or returns an error, but not all versions of gcc figure this out, so we have to work around this using the uninitialized_var() macro.

Quoted 10 lines

Well, I'm somewhat sceptical to this approach. I agree that bogus
warnings are not nice but later when the code is changed and possibly real use without initialization is added, we won't notice it. Without changing anything, we'd at least have a chance of catching it with gcc versions which were clever enough to not warn with the original code. Or alternatively if we unconditionally initialized the variable that would get rid of the warning and made the code more future-proof (that's what I usually end up doing)... I don't really care that much about the chosen solution, Al is the one to decide. But I wanted to point out there are downsides to your solution.

Quoted 8 lines

I'll drop the patch for now and won't send it from my tree then. I agree that uninitialized_var() is not ideal, but none of the alternatives seemed better.

With my latest compiler, I don't actually see the warnings any more, so maybe someone fixed gcc instead, or this went away after another change. I'll let you know if it comes back so we can discuss about a better fix then.

Update: I could actually reproduce the problem now, but it only happens when building with 'gcc -s' (i.e. CONFIG_CC_OPTIMIZE_FOR_SIZE). It does happen with both gcc-4.6 and with gcc-4.8, and on both x86-64 and ARM. An alternative patch that would also make it go away is the variant below, but I think that's even worse than the first version I suggested because it makes the binary output slightly worse by adding an unnecessary initialization when building with 'make -s'.

fs/namei.c +1 -0

index aa30d19..c3612a5 100644
@@ -810,6 +810,7 @@ follow_link(struct path *link, struct nameidata *nd, void **p)
 	return error;
 
 out_put_nd_path:
+	*p = NULL;
 	path_put(&nd->path);
 	path_put(link);
 	return error;

Update: I could actually reproduce the problem now, but it only happens when building with 'gcc -s' (i.e. CONFIG_CC_OPTIMIZE_FOR_SIZE). It does happen with both gcc-4.6 and with gcc-4.8, and on both x86-64 and ARM. An alternative patch that would also make it go away is the variant below, but I think that's even worse than the first version I suggested because it makes the binary output slightly worse by adding an unnecessary initialization when building with 'make -s'.

Without this patch, building ARM collie_defconfig results in:

mm/slob.c: In function '__kmalloc_node':
mm/slob.c:431:152: warning: comparison of distinct pointer types lacks a cast [enabled by default] mm/slob.c: In function 'kfree':
mm/slob.c:484:153: warning: comparison of distinct pointer types lacks a cast [enabled by default] mm/slob.c: In function 'ksize':
mm/slob.c:503:153: warning: comparison of distinct pointer types lacks a cast [enabled by default]

Quoted 2 lines · Arnd Bergmann

Without this patch, building ARM collie_defconfig results in:

Quoted 6 lines

mm/slob.c: In function '__kmalloc_node':
mm/slob.c:431:152: warning: comparison of distinct pointer types lacks a cast [enabled by default] mm/slob.c: In function 'kfree':
mm/slob.c:484:153: warning: comparison of distinct pointer types lacks a cast [enabled by default] mm/slob.c: In function 'ksize':
mm/slob.c:503:153: warning: comparison of distinct pointer types lacks a cast [enabled by default]

Acked-by Christoph Lameter <[email protected]>

In a configuration where the base cgroup support is enabled but every single cgroup subsys is turned off, CGROUP_BUILTIN_SUBSYS_COUNT is zero, which causes the sanity check code in cgroup_load_subsys to trigger:

       BUG_ON(ss->subsys_id >= CGROUP_BUILTIN_SUBSYS_COUNT);
       BUG_ON(subsys[ss->subsys_id] != ss);

Gcc first confirms that subsys_id cannot be 0 or larger and consequently warns in the second line.

kernel/cgroup.c: In function 'cgroup_load_subsys':
kernel/cgroup.c:4326:38: warning: array subscript is below array bounds [-Warray-bounds]

Since the subsys_id can never be less than zero, we can just change the type to an unsigned int, which makes the warning go away.

Found by building ARM cns3420vb_defconfig.

Signed-off-by Arnd Bergmann <[email protected]>

Cc Tejun Heo <[email protected]>

Cc Li Zefan <[email protected]>

Cc Ben Blum <[email protected]>

---
 include/linux/cgroup.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/cgroup.h +1 -1

index c90eaa8..26b99df 100644
@@ -475,7 +475,7 @@ struct cgroup_subsys {
 	void (*post_clone)(struct cgroup *cgrp);
 	void (*bind)(struct cgroup *root);
 
-	int subsys_id;
+	unsigned int subsys_id;
 	int active;
 	int disabled;
 	int early_init;

Signature · 1 line

1.7.10

In a configuration where the base cgroup support is enabled but every single cgroup subsys is turned off, CGROUP_BUILTIN_SUBSYS_COUNT is zero, which causes the sanity check code in cgroup_load_subsys to trigger:

       BUG_ON(ss->subsys_id >= CGROUP_BUILTIN_SUBSYS_COUNT);
       BUG_ON(subsys[ss->subsys_id] != ss);

Quoted 7 lines · Arnd Bergmann

In a configuration where the base cgroup support is enabled but every single cgroup subsys is turned off, CGROUP_BUILTIN_SUBSYS_COUNT is zero, which causes the sanity check code in cgroup_load_subsys to trigger:

       BUG_ON(ss->subsys_id >= CGROUP_BUILTIN_SUBSYS_COUNT);
       BUG_ON(subsys[ss->subsys_id] != ss);

CGROUP_BUILTIN_SUBSYS_COUNT is already gone in mainline. I suppose this is no longer necessary?

The ip_vs_set_timeout function sets timeouts for TCP and UDP, which can be enabled independently at compile time. The debug message always prints both timeouts that are passed into the function, but if one is disabled, the message will show uninitialized data.

This splits the debug message into two separte IP_VS_DBG statements that are in the same #ifdef section to ensure we only print the text about what is actually going on.

Without this patch, building ARM ixp4xx_defconfig results in:

net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd':
net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here

Signed-off-by Arnd Bergmann <[email protected]>

Cc David S. Miller <[email protected]>

Cc [email protected]

Cc Simon Horman <[email protected]>

Cc Julian Anastasov <[email protected]>

Cc [email protected]

Cc [email protected]

Cc [email protected]

---
 net/netfilter/ipvs/ip_vs_ctl.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

net/netfilter/ipvs/ip_vs_ctl.c +6 -4

index f51013c..f3a66c3 100644
@@ -2237,12 +2237,11 @@ static int ip_vs_set_timeout(struct net *net, struct ip_vs_timeout_user *u)
 	struct ip_vs_proto_data *pd;
 #endif
 
-	IP_VS_DBG(2, "Setting timeout tcp:%d tcpfin:%d udp:%d\n",
+#ifdef CONFIG_IP_VS_PROTO_TCP
+	IP_VS_DBG(2, "Setting timeout tcp:%d tcpfin:%d\n",
 		  u->tcp_timeout,
-		  u->tcp_fin_timeout,
-		  u->udp_timeout);
+		  u->tcp_fin_timeout);
 
-#ifdef CONFIG_IP_VS_PROTO_TCP
 	if (u->tcp_timeout) {
 		pd = ip_vs_proto_data_get(net, IPPROTO_TCP);
 		pd->timeout_table[IP_VS_TCP_S_ESTABLISHED]

@@ -2257,6 +2256,9 @@ static int ip_vs_set_timeout(struct net *net, struct ip_vs_timeout_user *u)
 #endif
 
 #ifdef CONFIG_IP_VS_PROTO_UDP
+	IP_VS_DBG(2, "Setting timeout udp:%d\n",
+		  u->udp_timeout);
+
 	if (u->udp_timeout) {
 		pd = ip_vs_proto_data_get(net, IPPROTO_UDP);
 		pd->timeout_table[IP_VS_UDP_S_NORMAL]

Signature · 1 line

1.7.10

Hello,

Quoted 9 lines · Arnd Bergmann

The ip_vs_set_timeout function sets timeouts for TCP and UDP, which can be enabled independently at compile time. The debug message always prints both timeouts that are passed into the function, but if one is disabled, the message will show uninitialized data.

This splits the debug message into two separte IP_VS_DBG statements that are in the same #ifdef section to ensure we only print the text about what is actually going on.

Quoted 1 line

Without this patch, building ARM ixp4xx_defconfig results in:

Are there any CONFIG_IP_VS_PROTO_xxx options in this
default config? It is a waste of memory if IPVS is compiled without any protocols.

Quoted 7 lines

net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd':
net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here

There are many __ip_vs_get_timeouts callers but
just one calls memset(&t, 0, sizeof(t)) before that, problem only for ip_vs_genl_set_config and ip_vs_set_timeout.

To be safe, can we move this memset into
__ip_vs_get_timeouts instead of playing games with defines?:

	memset(t, 0, sizeof(*t));

	This debug message will be more precise in showing the

changed values if we replace the __ip_vs_get_timeouts call in ip_vs_genl_set_config with memset(&t, 0, sizeof(t)). Then we will see 0 for values that are not changed/supported.

Quoted 14 lines · Julian Anastasov

Hello,

Quoted 9 lines · Arnd Bergmann

The ip_vs_set_timeout function sets timeouts for TCP and UDP, which can be enabled independently at compile time. The debug message always prints both timeouts that are passed into the function, but if one is disabled, the message will show uninitialized data.

This splits the debug message into two separte IP_VS_DBG statements that are in the same #ifdef section to ensure we only print the text about what is actually going on.

Quoted 5 lines

Quoted 1 line

Without this patch, building ARM ixp4xx_defconfig results in:

Are there any CONFIG_IP_VS_PROTO_xxx options in this
default config? It is a waste of memory if IPVS is compiled without any protocols.

They all appear to be turned off:

$ grep CONFIG_IP_VS obj-tmp/.config
CONFIG_IP_VS=m
CONFIG_IP_VS_DEBUG=y
CONFIG_IP_VS_TAB_BITS=12
# CONFIG_IP_VS_PROTO_TCP is not set
# CONFIG_IP_VS_PROTO_UDP is not set
# CONFIG_IP_VS_PROTO_AH_ESP is not set
# CONFIG_IP_VS_PROTO_ESP is not set
# CONFIG_IP_VS_PROTO_AH is not set
# CONFIG_IP_VS_PROTO_SCTP is not set
CONFIG_IP_VS_RR=m
CONFIG_IP_VS_WRR=m
CONFIG_IP_VS_LC=m

CONFIG_IP_VS_WLC=m
CONFIG_IP_VS_LBLC=m
CONFIG_IP_VS_LBLCR=m
CONFIG_IP_VS_DH=m
CONFIG_IP_VS_SH=m
# CONFIG_IP_VS_SED is not set
# CONFIG_IP_VS_NQ is not set
CONFIG_IP_VS_SH_TAB_BITS=8

Quoted 15 lines

Quoted 7 lines

net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd':
net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here

There are many __ip_vs_get_timeouts callers but
just one calls memset(&t, 0, sizeof(t)) before that, problem only for ip_vs_genl_set_config and ip_vs_set_timeout.

To be safe, can we move this memset into
__ip_vs_get_timeouts instead of playing games with defines?:

Quoted 6 lines

	memset(t, 0, sizeof(*t));

	This debug message will be more precise in showing the

changed values if we replace the __ip_vs_get_timeouts call in ip_vs_genl_set_config with memset(&t, 0, sizeof(t)). Then we will see 0 for values that are not changed/supported.

8<-----
ipvs: initialize returned data in do_ip_vs_get_ctl

As reported by a gcc warning, the do_ip_vs_get_ctl does not initalize all the members of the ip_vs_timeout_user structure it returns if at least one of the TCP or UDP protocols is disabled for ipvs.

This makes sure that the data is always initialized, before it is returned as a response to IPVS_CMD_GET_CONFIG or printed as a debug message in IPVS_CMD_SET_CONFIG.

Without this patch, building ARM ixp4xx_defconfig results in:

net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd':
net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here

Signed-off-by Arnd Bergmann <[email protected]>

net/netfilter/ipvs/ip_vs_ctl.c +0 -0

index 2770f85..3995d2e 100644

Quoted 6 lines · Arnd Bergmann

Quoted 3 lines

Are there any CONFIG_IP_VS_PROTO_xxx options in this
default config? It is a waste of memory if IPVS is compiled without any protocols.

They all appear to be turned off:

Quoted 10 lines

$ grep CONFIG_IP_VS obj-tmp/.config
CONFIG_IP_VS=m
CONFIG_IP_VS_DEBUG=y
CONFIG_IP_VS_TAB_BITS=12
# CONFIG_IP_VS_PROTO_TCP is not set
# CONFIG_IP_VS_PROTO_UDP is not set
# CONFIG_IP_VS_PROTO_AH_ESP is not set
# CONFIG_IP_VS_PROTO_ESP is not set
# CONFIG_IP_VS_PROTO_AH is not set
# CONFIG_IP_VS_PROTO_SCTP is not set

Something should be changed here, may be at least
TCP/UDP, who knows.

net/netfilter/ipvs/ip_vs_ctl.c __ip_vs_get_timeouts(struct net *net, struct ip_vs_timeout_user *u) · line 2590

Quoted 6 lines

net/netfilter/ipvs/ip_vs_ctl.c +0 -0

@@ -2590,6 +2588,7 @@ __ip_vs_get_timeouts(struct net *net, struct ip_vs_timeout_user *u)
 #if defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP)
 	struct ip_vs_proto_data *pd;
 #endif

That is what we want. If you plan another submission
you can add empty line before this memset and to replace the __ip_vs_get_timeouts call in ip_vs_genl_set_config with memset but they are cosmetic changes. Or may be Simon will take care about the coding style when applying the change.

Acked-by Julian Anastasov <[email protected]>

Quoted 18 lines · Julian Anastasov

Quoted 6 lines · Arnd Bergmann

Quoted 3 lines

Are there any CONFIG_IP_VS_PROTO_xxx options in this
default config? It is a waste of memory if IPVS is compiled without any protocols.

They all appear to be turned off:

Quoted 10 lines

$ grep CONFIG_IP_VS obj-tmp/.config
CONFIG_IP_VS=m
CONFIG_IP_VS_DEBUG=y
CONFIG_IP_VS_TAB_BITS=12
# CONFIG_IP_VS_PROTO_TCP is not set
# CONFIG_IP_VS_PROTO_UDP is not set
# CONFIG_IP_VS_PROTO_AH_ESP is not set
# CONFIG_IP_VS_PROTO_ESP is not set
# CONFIG_IP_VS_PROTO_AH is not set
# CONFIG_IP_VS_PROTO_SCTP is not set

Quoted 2 lines

Something should be changed here, may be at least
TCP/UDP, who knows.

I don't try to read too much into our defconfigs. We have 140 of them on ARM, and they are mainly useful to give a reasonable build coverage, but I wouldn't expect them to be actually used on that hardware.

I'll leave it up to Krzysztof to send a patch for this if he wants.

Quoted 13 lines

net/netfilter/ipvs/ip_vs_ctl.c __ip_vs_get_timeouts(struct net *net, struct ip_vs_timeout_user *u) · line 2590

Quoted 6 lines

net/netfilter/ipvs/ip_vs_ctl.c +0 -0

@@ -2590,6 +2588,7 @@ __ip_vs_get_timeouts(struct net *net, struct ip_vs_timeout_user *u)
 #if defined(CONFIG_IP_VS_PROTO_TCP) || defined(CONFIG_IP_VS_PROTO_UDP)
 	struct ip_vs_proto_data *pd;
 #endif

That is what we want. If you plan another submission
you can add empty line before this memset and to replace the __ip_vs_get_timeouts call in ip_vs_genl_set_config with memset but they are cosmetic changes. Or may be Simon will take care about the coding style when applying the change.

Quoted 1 line

Acked-by Julian Anastasov <[email protected]>

I'd prefer Simon to pick up the patch. He should also decide whether he wants to add it to stable. In theory, this is a small leak of kernel stack data to user space, but as you say in practice it should not happen because it only exists for silly configurations that nobody should be using.

AFAICT, removing the call to __ip_vs_get_timeouts in do_ip_vs_get_ctl would be a semantic change for the case where a user sends a IPVS_CMD_SET_CONFIG message without without the complete set of attributes inside it. The current behavior is to leave the timeouts alone, replacing the __ip_vs_get_timeouts with a memset would zero them. I left this part alone then.

Arnd

8<-----
ipvs: initialize returned data in do_ip_vs_get_ctl

As reported by a gcc warning, the do_ip_vs_get_ctl does not initalize all the members of the ip_vs_timeout_user structure it returns if at least one of the TCP or UDP protocols is disabled for ipvs.

This makes sure that the data is always initialized, before it is returned as a response to IPVS_CMD_GET_CONFIG or printed as a debug message in IPVS_CMD_SET_CONFIG.

Without this patch, building ARM ixp4xx_defconfig results in:

net/netfilter/ipvs/ip_vs_ctl.c: In function 'ip_vs_genl_set_cmd':
net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.udp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.udp_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_fin_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_fin_timeout' was declared here net/netfilter/ipvs/ip_vs_ctl.c:2238:47: warning: 't.tcp_timeout' may be used uninitialized in this function [-Wuninitialized] net/netfilter/ipvs/ip_vs_ctl.c:3322:28: note: 't.tcp_timeout' was declared here

Signed-off-by Arnd Bergmann <[email protected]>

Acked-by Julian Anastasov <[email protected]>

---

net/netfilter/ipvs/ip_vs_ctl.c +2 -1

index 2770f85..c4ee437 100644
@@ -2591,6 +2589,8 @@ __ip_vs_get_timeouts(struct net *net, struct ip_vs_timeout_user *u)
 	struct ip_vs_proto_data *pd;
 #endif
 
+	memset(u, 0, sizeof (*u));
+
 #ifdef CONFIG_IP_VS_PROTO_TCP
 	pd = ip_vs_proto_data_get(net, IPPROTO_TCP);
 	u->tcp_timeout = pd->timeout_table[IP_VS_TCP_S_ESTABLISHED] / HZ;

@@ -2768,7 +2768,6 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
 	{
 		struct ip_vs_timeout_user t;
 
-		memset(&t, 0, sizeof(t));
 		__ip_vs_get_timeouts(net, &t);
 		if (copy_to_user(user, &t, sizeof(t)) != 0)
 			ret = -EFAULT;

The __devinit section is going away soon, but while it's still there, we get a correct warning about ehci_orion_conf_mbus_windows being discarded before its caller, so it should be marked __devinit rather than __init.

Without this patch, building dove_defconfig results in:

WARNING: drivers/usb/host/built-in.o(.devinit.text+0x8a4): Section mismatch in reference from the function ehci_orion_drv_probe() to the function .init.text:ehci_orion_conf_mbus_windows() The function __devinit ehci_orion_drv_probe() references
a function __init ehci_orion_conf_mbus_windows().
If ehci_orion_conf_mbus_windows is only used by ehci_orion_drv_probe then
annotate ehci_orion_conf_mbus_windows with a matching annotation.

Signed-off-by Arnd Bergmann <[email protected]>

Cc Alan Stern <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc linux-usb at vger.kernel.org

Sometimes we want the kernel build process to only print messages on errors, e.g. in automated build testing. This uses the "kecho" macro that the build system provides to hide a few informational messages. Nothing changes for a regular "make" or "make V=1".

Without this patch, building any ARM kernel results in:

Kernel: arch/arm/boot/Image is ready
Kernel: arch/arm/boot/zImage is ready

Signed-off-by Arnd Bergmann <[email protected]>

Cc Russell King <[email protected]>

Cc Nicolas Pitre <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Michal Marek <[email protected]>

---
 arch/arm/boot/Makefile  |   10 +++++-----
 arch/arm/tools/Makefile |    2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

arch/arm/boot/Makefile +5 -5

index c877087..89680f2 100644
@@ -31,7 +31,7 @@ ifeq ($(CONFIG_XIP_KERNEL),y)
 
 $(obj)/xipImage: vmlinux FORCE
 	$(call if_changed,objcopy)
-	@echo '  Kernel: $@ is ready (physical address: $(CONFIG_XIP_PHYS_ADDR))'
+	$(kecho) '  Kernel: $@ is ready (physical address: $(CONFIG_XIP_PHYS_ADDR))'
 
 $(obj)/Image $(obj)/zImage: FORCE
 	@echo 'Kernel configured for XIP (CONFIG_XIP_KERNEL=y)'

@@ -46,14 +46,14 @@ $(obj)/xipImage: FORCE
 
 $(obj)/Image: vmlinux FORCE
 	$(call if_changed,objcopy)
-	@echo '  Kernel: $@ is ready'
+	$(kecho) '  Kernel: $@ is ready'
 
 $(obj)/compressed/vmlinux: $(obj)/Image FORCE
 	$(Q)$(MAKE) $(build)=$(obj)/compressed $@
 
 $(obj)/zImage:	$(obj)/compressed/vmlinux FORCE
 	$(call if_changed,objcopy)
-	@echo '  Kernel: $@ is ready'
+	$(kecho) '  Kernel: $@ is ready'
 
 endif
 

@@ -88,7 +88,7 @@ fi
 $(obj)/uImage:	$(obj)/zImage FORCE
 	@$(check_for_multiple_loadaddr)
 	$(call if_changed,uimage)
-	@echo '  Image $@ is ready'
+	$(kecho) '  Image $@ is ready'
 
 $(obj)/bootp/bootp: $(obj)/zImage initrd FORCE
 	$(Q)$(MAKE) $(build)=$(obj)/bootp $@

@@ -96,7 +96,7 @@ $(obj)/bootp/bootp: $(obj)/zImage initrd FORCE
 
 $(obj)/bootpImage: $(obj)/bootp/bootp FORCE
 	$(call if_changed,objcopy)
-	@echo '  Kernel: $@ is ready'
+	$(kecho) '  Kernel: $@ is ready'
 
 PHONY += initrd FORCE
 initrd:

arch/arm/tools/Makefile +1 -1

index 635cb18..cd60a81 100644
@@ -5,6 +5,6 @@
 #
 
 include/generated/mach-types.h: $(src)/gen-mach-types $(src)/mach-types
-	@echo '  Generating $@'
+	$(kecho) '  Generating $@'
 	@mkdir -p $(dir $@)
 	$(Q)$(AWK) -f $^ > $@ || { rm -f $@; /bin/false; }

Signature · 1 line

1.7.10

The it8152 PCI host used on the pxa/cm_x2xx machines uses the old-style I/O window registration. This should eventually get converted to pci_ioremap_io() but for now, let's cast the IT8152_IO_BASE constant to an integer type to get rid of the warnings.

Without this patch, building cm_x2xx_defconfig results in:

arch/arm/common/it8152.c: In function 'it8152_pci_setup':
arch/arm/common/it8152.c:287:18: warning: assignment makes integer from pointer without a cast [enabled by default] arch/arm/common/it8152.c:288:16: warning: assignment makes integer from pointer without a cast [enabled by default] arch/arm/common/it8152.c:291:17: warning: assignment makes integer from pointer without a cast [enabled by default]

Signed-off-by Arnd Bergmann <[email protected]>

Cc Bjorn Helgaas <[email protected]>

Cc Krzysztof Halasa <[email protected]>

Cc Mike Rapoport <[email protected]>

Cc Igor Grinberg <[email protected]>

Cc Haojian Zhuang <[email protected]>

Cc Eric Miao <[email protected]>

---
 arch/arm/common/it8152.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

arch/arm/common/it8152.c +9 -3

index c4110d1..001f491 100644
@@ -284,11 +284,17 @@ int dma_set_coherent_mask(struct device *dev, u64 mask)
 
 int __init it8152_pci_setup(int nr, struct pci_sys_data *sys)
 {
-	it8152_io.start = IT8152_IO_BASE + 0x12000;
-	it8152_io.end	= IT8152_IO_BASE + 0x12000 + 0x100000;
+	/*
+	 * FIXME: use pci_ioremap_io to remap the IO space here and
+	 * move over to the generic io.h implementation.
+	 * This requires solving the same problem for PXA PCMCIA
+	 * support.
+	 */
+	it8152_io.start = (unsigned long)IT8152_IO_BASE + 0x12000;
+	it8152_io.end	= (unsigned long)IT8152_IO_BASE + 0x12000 + 0x100000;
 
 	sys->mem_offset = 0x10000000;
-	sys->io_offset  = IT8152_IO_BASE;
+	sys->io_offset  = (unsigned long)IT8152_IO_BASE;
 
 	if (request_resource(&ioport_resource, &it8152_io)) {
 		printk(KERN_ERR "PCI: unable to allocate IO region\n");

The Linaro cross toolchain and probably others nowadays default to building in THUMB2 mode. When building a kernel for a CPU that does not support THUMB2, the compiler complains about incorrect flags. We can work around this by setting -marm for all non-T2 builds.

Without this patch, building assabet_defconfig results in:

usr/initramfs_data.S:1:0: warning: target CPU does not support THUMB instructions [enabled by default] arch/arm/nwfpe/entry.S:1:0: warning: target CPU does not support THUMB instructions [enabled by default] firmware/cis/PCMLM28.cis.gen.S:1:0: warning: target CPU does not support THUMB instructions [enabled by default] (and many more)

Signed-off-by Arnd Bergmann <[email protected]>

Cc Russell King <[email protected]>

Cc Dave Martin <[email protected]>

---
 arch/arm/Makefile |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

arch/arm/Makefile +7 -6

index a051dfb..4391427 100644
@@ -21,8 +21,6 @@ endif
 OBJCOPYFLAGS	:=-O binary -R .comment -S
 GZFLAGS		:=-9
 #KBUILD_CFLAGS	+=-pipe
-# Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb:
-KBUILD_CFLAGS	+=$(call cc-option,-marm,)
 
 # Never generate .eh_frame
 KBUILD_CFLAGS	+= $(call cc-option,-fno-dwarf2-cfi-asm)

@@ -105,17 +103,20 @@ endif
 ifeq ($(CONFIG_THUMB2_KERNEL),y)
 AFLAGS_AUTOIT	:=$(call as-option,-Wa$(comma)-mimplicit-it=always,-Wa$(comma)-mauto-it)
 AFLAGS_NOWARN	:=$(call as-option,-Wa$(comma)-mno-warn-deprecated,-Wa$(comma)-W)
-CFLAGS_THUMB2	:=-mthumb $(AFLAGS_AUTOIT) $(AFLAGS_NOWARN)
-AFLAGS_THUMB2	:=$(CFLAGS_THUMB2) -Wa$(comma)-mthumb
+CFLAGS_ISA	:=-mthumb $(AFLAGS_AUTOIT) $(AFLAGS_NOWARN)
+AFLAGS_ISA	:=$(CFLAGS_ISA) -Wa$(comma)-mthumb
 # Work around buggy relocation from gas if requested:
 ifeq ($(CONFIG_THUMB2_AVOID_R_ARM_THM_JUMP11),y)
 CFLAGS_MODULE	+=-fno-optimize-sibling-calls
 endif
+else
+CFLAGS_ISA	:=$(call cc-option,-marm,)
+AFLAGS_ISA	:=$(CFLAGS_ISA)
 endif
 
 # Need -Uarm for gcc < 3.x
-KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(CFLAGS_THUMB2) $(arch-y) $(tune-y) $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) -msoft-float -Uarm
-KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(AFLAGS_THUMB2) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float
+KBUILD_CFLAGS	+=$(CFLAGS_ABI) $(CFLAGS_ISA) $(arch-y) $(tune-y) $(call cc-option,-mshort-load-bytes,$(call cc-option,-malignment-traps,)) -msoft-float -Uarm
+KBUILD_AFLAGS	+=$(CFLAGS_ABI) $(AFLAGS_ISA) $(arch-y) $(tune-y) -include asm/unified.h -msoft-float
 
 CHECKFLAGS	+= -D__arm__
 

Signature · 1 line

1.7.10