[PATCH 01/16] ARM: warnings in arch/arm/include/asm/uaccess.h

[PATCH 01/16] ARM: warnings in arch/arm/include/asm/uaccess.h

[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:

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 <arnd@arndb.de>
Cc: Greg Ungerer <gerg@uclinux.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/include/asm/uaccess.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 77bd79f..7e1f760 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -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)
-- 
1.7.10

Re: [PATCH 01/16] ARM: warnings in arch/arm/include/asm/uaccess.h

[Greg Ungerer]

On 06/10/12 00:55, Arnd Bergmann wrote:
> 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 <arnd@arndb.de>
> Cc: Greg Ungerer <gerg@uclinux.org>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>

Acked-by: Greg Ungerer <gerg@uclinux.org>

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

Regards
Greg


> ---
>   arch/arm/include/asm/uaccess.h |    4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
> index 77bd79f..7e1f760 100644
> --- a/arch/arm/include/asm/uaccess.h
> +++ b/arch/arm/include/asm/uaccess.h
> @@ -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)
>


-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

Re: [PATCH 01/16] ARM: warnings in arch/arm/include/asm/uaccess.h

[Arnd Bergmann]

On Monday 08 October 2012, Greg Ungerer wrote:
> On 06/10/12 00:55, Arnd Bergmann wrote:
> > 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 <arnd@arndb.de>
> > Cc: Greg Ungerer <gerg@uclinux.org>
> > Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> Acked-by: Greg Ungerer <gerg@uclinux.org>

Thanks!

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

Well, my idea was that I would prefer the patches go through some other tree
besides arm-soc since they are not really the main purpose for this tree.

The core ARM patches in particular should go through Russell's ARM tree.
I have a few more that come in during the merge window and plan to send
him a pull request for those.

	Arnd

[PATCH 02/16] ARM: binfmt_flat: unused variable 'persistent'

[Arnd Bergmann]

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 <arnd@arndb.de>
Cc: Greg Ungerer <gerg@uclinux.org>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Bryan Wu <bryan.wu@canonical.com>
---
 arch/arm/include/asm/flat.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/flat.h b/arch/arm/include/asm/flat.h
index 59426a4..e847d23 100644
--- a/arch/arm/include/asm/flat.h
+++ b/arch/arm/include/asm/flat.h
@@ -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
-- 
1.7.10

Re: [PATCH 02/16] ARM: binfmt_flat: unused variable 'persistent'

[Greg Ungerer]

On 06/10/12 00:55, Arnd Bergmann wrote:
> 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 <arnd@arndb.de>
> Cc: Greg Ungerer <gerg@uclinux.org>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Bryan Wu <bryan.wu@canonical.com>

Acked-by: Greg Ungerer <gerg@uclinux.org>

I can pick this one up to, and push to the arm-soc tree.

Regards
Greg


> ---
>   arch/arm/include/asm/flat.h |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/flat.h b/arch/arm/include/asm/flat.h
> index 59426a4..e847d23 100644
> --- a/arch/arm/include/asm/flat.h
> +++ b/arch/arm/include/asm/flat.h
> @@ -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
>


-- 
------------------------------------------------------------------------
Greg Ungerer  --  Principal Engineer        EMAIL:     gerg@snapgear.com
SnapGear Group, McAfee                      PHONE:       +61 7 3435 2888
8 Gardner Close                             FAX:         +61 7 3217 5323
Milton, QLD, 4064, Australia                WEB: http://www.SnapGear.com

[PATCH 03/16] SCSI: ARM: ncr5380/oak uses no interrupts

[Arnd Bergmann]

The ncr5380 driver is included by multiple board specific
drivers, which may or may not use the interrupt handler.
The oak variant doesn't, and should set the DONT_USE_INTR
macro.

Without this patch, building rpc_defconfig results in:

drivers/scsi/arm/../NCR5380.c:1160:20: warning: 'oakscsi_intr' defined but not used [-Wunused-function]

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/arm/oak.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/arm/oak.c b/drivers/scsi/arm/oak.c
index d25f944..fc6a5aa 100644
--- a/drivers/scsi/arm/oak.c
+++ b/drivers/scsi/arm/oak.c
@@ -21,6 +21,7 @@
 /*#define PSEUDO_DMA*/
 
 #define OAKSCSI_PUBLIC_RELEASE 1
+#define DONT_USE_INTR
 
 #define priv(host)			((struct NCR5380_hostdata *)(host)->hostdata)
 #define NCR5380_local_declare()		void __iomem *_base
-- 
1.7.10

[PATCH 04/16] SCSI: ARM: make fas216_dumpinfo function conditional

[Arnd Bergmann]

The fas216_dumpinfo function is only used by __fas216_checkmagic,
which is conditionally compiled, so we should put both functions
inside of the same #ifdef.

Without this patch, building rpc_defconfig results in:

drivers/scsi/arm/fas216.c:182:13: warning: 'fas216_dumpinfo' defined but not used [-Wunused-function]

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-scsi@vger.kernel.org
---
 drivers/scsi/arm/fas216.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/arm/fas216.c b/drivers/scsi/arm/fas216.c
index 6206a66..737554c 100644
--- a/drivers/scsi/arm/fas216.c
+++ b/drivers/scsi/arm/fas216.c
@@ -179,6 +179,7 @@ static void print_SCp(struct scsi_pointer *SCp, const char *prefix, const char *
 		SCp->buffers_residual, suffix);
 }
 
+#ifdef CHECK_STRUCTURE
 static void fas216_dumpinfo(FAS216_Info *info)
 {
 	static int used = 0;
@@ -223,7 +224,6 @@ static void fas216_dumpinfo(FAS216_Info *info)
 		info->internal_done, info->magic_end);
 }
 
-#ifdef CHECK_STRUCTURE
 static void __fas216_checkmagic(FAS216_Info *info, const char *func)
 {
 	int corruption = 0;
-- 
1.7.10

[PATCH 05/16] vfs: bogus warnings in fs/namei.c

[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.

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 <arnd@arndb.de>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org
Cc: Jan Kara <jack@suse.cz>
---
 fs/namei.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index dd1ed1b..62a1725 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -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);
-- 
1.7.10

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Jan Kara]

On Fri 05-10-12 16:55:19, Arnd Bergmann wrote:
> 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.

								Honza

> 
> 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 <arnd@arndb.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: linux-fsdevel@vger.kernel.org
> Cc: Jan Kara <jack@suse.cz>
> ---
>  fs/namei.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index dd1ed1b..62a1725 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -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);
> -- 
> 1.7.10
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Arnd Bergmann]

On Monday 08 October 2012, Jan Kara wrote:
> On Fri 05-10-12 16:55:19, Arnd Bergmann wrote:
> > 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.

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.

	Arnd

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Arnd Bergmann]

On Tuesday 09 October 2012, Arnd Bergmann wrote:
> On Monday 08 October 2012, Jan Kara wrote:
> > On Fri 05-10-12 16:55:19, Arnd Bergmann wrote:
> > > 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.
> 
> 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'.

	Arnd

diff --git a/fs/namei.c b/fs/namei.c
index aa30d19..c3612a5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -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;

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Jan Kara]

On Tue 09-10-12 13:07:19, Arnd Bergmann wrote:
> On Tuesday 09 October 2012, Arnd Bergmann wrote:
> > On Monday 08 October 2012, Jan Kara wrote:
> > > On Fri 05-10-12 16:55:19, Arnd Bergmann wrote:
> > > > 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.
> > 
> > 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'.
  Hum, dumb compiler... I like this patch better and since the extra
initialization is on error path only, I don't think it matters. But
whatever Al likes better.

									Honza

> diff --git a/fs/namei.c b/fs/namei.c
> index aa30d19..c3612a5 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -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;

Re: [PATCH 05/16] vfs: bogus warnings in fs/namei.c

[Al Viro]

On Tue, Oct 09, 2012 at 01:07:19PM +0000, Arnd Bergmann wrote:

> 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'.

I can live with that, provided that you give it sane commit message and
your s-o-b.

[PATCH v2] vfs: bogus warnings in fs/namei.c

[Arnd Bergmann]

The follow_link() function always initializes its *p argument,
or returns an error, but when building with 'gcc -s', the compiler
gets confused by the __always_inline attribute to the function
and can no longer detect where the cookie was initialized.

The solution is to always initialize the pointer from follow_link,
even in the error path. When building with -O2, this has zero impact
on generated code and adds a single instruction in the error path
for a -Os build on ARM.

Without this patch, building with gcc-4.6 through gcc-4.8 and
CONFIG_CC_OPTIMIZE_FOR_SIZE 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 <arnd@arndb.de>

diff --git a/fs/namei.c b/fs/namei.c
index 6d47fac..c1f18e4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -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;

[PATCH 06/16] mm/slob: use min_t() to compare ARCH_SLAB_MINALIGN

[Arnd Bergmann]

The definition of ARCH_SLAB_MINALIGN is architecture dependent
and can be either of type size_t or int. Comparing that value
with ARCH_KMALLOC_MINALIGN can cause harmless warnings on
platforms where they are different. Since both are always
small positive integer numbers, using the size_t type to compare
them is safe and gets rid of the warning.

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]

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
---
 mm/slob.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/slob.c b/mm/slob.c
index 45d4ca7..497c55e 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -428,7 +428,7 @@ out:
 void *__kmalloc_node(size_t size, gfp_t gfp, int node)
 {
 	unsigned int *m;
-	int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
+	int align = max_t(size_t, ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
 	void *ret;
 
 	gfp &= gfp_allowed_mask;
@@ -481,7 +481,7 @@ void kfree(const void *block)
 
 	sp = virt_to_page(block);
 	if (PageSlab(sp)) {
-		int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
+		int align = max_t(size_t, ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
 		unsigned int *m = (unsigned int *)(block - align);
 		slob_free(m, *m + align);
 	} else
@@ -500,7 +500,7 @@ size_t ksize(const void *block)
 
 	sp = virt_to_page(block);
 	if (PageSlab(sp)) {
-		int align = max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
+		int align = max_t(size_t, ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MINALIGN);
 		unsigned int *m = (unsigned int *)(block - align);
 		return SLOB_UNITS(*m) * SLOB_UNIT;
 	} else
-- 
1.7.10

Re: [PATCH 06/16] mm/slob: use min_t() to compare ARCH_SLAB_MINALIGN

[Christoph Lameter]

On Fri, 5 Oct 2012, Arnd Bergmann wrote:

> 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]

Acked-by: Christoph Lameter <cl@linux.com>

Re: [PATCH 06/16] mm/slob: use min_t() to compare ARCH_SLAB_MINALIGN

[Pekka Enberg]

On Fri, 5 Oct 2012, Christoph Lameter wrote:

> On Fri, 5 Oct 2012, Arnd Bergmann wrote:
> 
> > 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]
> 
> Acked-by: Christoph Lameter <cl@linux.com>

Applied, thanks!

[PATCH 07/16] cgroup: fix warning when building without any subsys

[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);

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 <arnd@arndb.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Ben Blum <bblum@andrew.cmu.edu>
---
 include/linux/cgroup.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index c90eaa8..26b99df 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -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;
-- 
1.7.10

Re: [PATCH 07/16] cgroup: fix warning when building without any subsys

[Ben Blum]

On Fri, Oct 05, 2012 at 04:55:21PM +0200, Arnd Bergmann wrote:
> 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 <arnd@arndb.de>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Li Zefan <lizefan@huawei.com>
> Cc: Ben Blum <bblum@andrew.cmu.edu>
> ---
>  include/linux/cgroup.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index c90eaa8..26b99df 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -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;
> -- 
> 1.7.10
> 
> 

Oops. Thank you!

Acked-by: Ben Blum <bblum@andrew.cmu.edu>

Re: [PATCH 07/16] cgroup: fix warning when building without any subsys

[Tejun Heo]

On Fri, Oct 05, 2012 at 04:55:21PM +0200, Arnd Bergmann wrote:
> 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?

Thanks.

-- 
tejun

Re: [PATCH 07/16] cgroup: fix warning when building without any subsys

[Arnd Bergmann]

On Saturday 06 October 2012, Tejun Heo wrote:
> On Fri, Oct 05, 2012 at 04:55:21PM +0200, Arnd Bergmann wrote:
> > 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?

Ah, you are right. I can confirm the problem is gone, and I'll drop
the patch.

	Arnd

[PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages

[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.

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 <arnd@arndb.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Simon Horman <horms@verge.net.au>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: netfilter-devel@vger.kernel.org
Cc: netfilter@vger.kernel.org
Cc: coreteam@netfilter.org
---
 net/netfilter/ipvs/ip_vs_ctl.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index f51013c..f3a66c3 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -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]
-- 
1.7.10

Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages

[Julian Anastasov]

	Hello,

On Fri, 5 Oct 2012, Arnd Bergmann wrote:

> 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:

	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.

> 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.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: Simon Horman <horms@verge.net.au>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: netfilter-devel@vger.kernel.org
> Cc: netfilter@vger.kernel.org
> Cc: coreteam@netfilter.org
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index f51013c..f3a66c3 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -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]
> -- 
> 1.7.10

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages

[Arnd Bergmann]

On Friday 05 October 2012, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Fri, 5 Oct 2012, Arnd Bergmann wrote:
> 
> > 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:
> 
> 	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

> > 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.

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 <arnd@arndb.de>

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 2770f85..3995d2e 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -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
+	memset(u, 0, sizeof (*u));
 
 #ifdef CONFIG_IP_VS_PROTO_TCP
 	pd = ip_vs_proto_data_get(net, IPPROTO_TCP);
@@ -2768,7 +2767,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;

Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages

[Julian Anastasov]

	Hello,

On Sat, 6 Oct 2012, Arnd Bergmann wrote:

> On Friday 05 October 2012, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Fri, 5 Oct 2012, Arnd Bergmann wrote:
> > 
> > > 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:
> > 
> > 	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

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

> > > 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.
> 
> 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 <arnd@arndb.de>
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 2770f85..3995d2e 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -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 <ja@ssi.bg>

> +	memset(u, 0, sizeof (*u));
>  
>  #ifdef CONFIG_IP_VS_PROTO_TCP
>  	pd = ip_vs_proto_data_get(net, IPPROTO_TCP);
> @@ -2768,7 +2767,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;

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages

[Arnd Bergmann]

On Saturday 06 October 2012, Julian Anastasov wrote:
> On Sat, 6 Oct 2012, Arnd Bergmann wrote:
> > > 	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
> 
> 	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.

> > --- a/net/netfilter/ipvs/ip_vs_ctl.c
> > +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> > @@ -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 <ja@ssi.bg>

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 <arnd@arndb.de>
Acked-by: Julian Anastasov <ja@ssi.bg>
---
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 2770f85..c4ee437 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -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;

Re: [PATCH 08/16] ipvs: fix ip_vs_set_timeout debug messages

[Simon Horman]

On Sat, Oct 06, 2012 at 09:54:15AM +0000, Arnd Bergmann wrote:
> On Saturday 06 October 2012, Julian Anastasov wrote:
> > On Sat, 6 Oct 2012, Arnd Bergmann wrote:
> > > > 	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
> > 
> > 	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.
> 
> > > --- a/net/netfilter/ipvs/ip_vs_ctl.c
> > > +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> > > @@ -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 <ja@ssi.bg>
> 
> 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

Hi,

sorry for being a bit slow, it was a long weekend here.
This patch looks reasonable and I think it is appropriate for stable.
I'll see about getting it merged accordingly.

> 
> 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 <arnd@arndb.de>
> Acked-by: Julian Anastasov <ja@ssi.bg>
> ---
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 2770f85..c4ee437 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -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;
> 

[PATCH 09/16] USB: EHCI: mark ehci_orion_conf_mbus_windows __devinit

[Arnd Bergmann]

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 <arnd@arndb.de>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/host/ehci-orion.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c
index 8892d36..1f5dd5e 100644
--- a/drivers/usb/host/ehci-orion.c
+++ b/drivers/usb/host/ehci-orion.c
@@ -160,7 +160,7 @@ static const struct hc_driver ehci_orion_hc_driver = {
 	.clear_tt_buffer_complete = ehci_clear_tt_buffer_complete,
 };
 
-static void __init
+static void __devinit
 ehci_orion_conf_mbus_windows(struct usb_hcd *hcd,
 			     const struct mbus_dram_target_info *dram)
 {
-- 
1.7.10

Re: [PATCH 09/16] USB: EHCI: mark ehci_orion_conf_mbus_windows __devinit

[Alan Stern]

On Fri, 5 Oct 2012, Arnd Bergmann wrote:

> 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 <arnd@arndb.de>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org

Acked-by: Alan Stern <stern@rowland.harvard.edu>

[PATCH 10/16] clk: don't mark clkdev_add_table as init

[Arnd Bergmann]

s3c2440_clk_add is a subsys_interface method and calls clkdev_add_table,
which means we might be calling it after the __init section is
discarded.

Without this patch, building mini2440_defconfig results in:

WARNING: vmlinux.o(.text+0x9848): Section mismatch in reference from the function s3c2440_clk_add() to the function .init.text:clkdev_add_table()
The function s3c2440_clk_add() references
the function __init clkdev_add_table().
This is often because s3c2440_clk_add lacks a __init
annotation or the annotation of clkdev_add_table is wrong.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Mike Turquette <mturquette@linaro.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Ben Dooks <ben-linux@fluff.org>
---
 drivers/clk/clkdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 442a313..6956857 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -179,7 +179,7 @@ void clkdev_add(struct clk_lookup *cl)
 }
 EXPORT_SYMBOL(clkdev_add);
 
-void __init clkdev_add_table(struct clk_lookup *cl, size_t num)
+void clkdev_add_table(struct clk_lookup *cl, size_t num)
 {
 	mutex_lock(&clocks_mutex);
 	while (num--) {
-- 
1.7.10

[PATCH 11/16] pcmcia: sharpsl: don't discard sharpsl_pcmcia_ops

[Arnd Bergmann]

The sharpsl_pcmcia_ops structure gets passed into
sa11xx_drv_pcmcia_probe, where it gets accessed at run-time,
unlike all other pcmcia drivers that pass their structures
into platform_device_add_data, which makes a copy.

This means the gcc warning is valid and the structure
must not be marked as __initdata.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Pavel Machek <pavel@suse.cz>
Cc: linux-pcmcia@lists.infradead.org
Cc: Jochen Friedrich <jochen@scram.de>
Cc: stable@vger.kernel.org
---
 drivers/pcmcia/pxa2xx_sharpsl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pcmcia/pxa2xx_sharpsl.c b/drivers/pcmcia/pxa2xx_sharpsl.c
index b066273..7dd879c 100644
--- a/drivers/pcmcia/pxa2xx_sharpsl.c
+++ b/drivers/pcmcia/pxa2xx_sharpsl.c
@@ -194,7 +194,7 @@ static void sharpsl_pcmcia_socket_suspend(struct soc_pcmcia_socket *skt)
 	sharpsl_pcmcia_init_reset(skt);
 }
 
-static struct pcmcia_low_level sharpsl_pcmcia_ops __initdata = {
+static struct pcmcia_low_level sharpsl_pcmcia_ops = {
 	.owner                  = THIS_MODULE,
 	.hw_init                = sharpsl_pcmcia_hw_init,
 	.socket_state           = sharpsl_pcmcia_socket_state,
-- 
1.7.10

[PATCH 12/16] video: mark nuc900fb_map_video_memory as __devinit

[Arnd Bergmann]

nuc900fb_map_video_memory is called by an devinit function
that may be called at run-time, but the function itself is
marked __init and will be discarded after boot.

To avoid calling into a function that may have been overwritten,
mark nuc900fb_map_video_memory itself as __devinit.

Without this patch, building nuc950_defconfig results in:

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

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Wan ZongShun <mcuos.com@gmail.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
 drivers/video/nuc900fb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/nuc900fb.c b/drivers/video/nuc900fb.c
index e10f551..b31b12b 100644
--- a/drivers/video/nuc900fb.c
+++ b/drivers/video/nuc900fb.c
@@ -387,7 +387,7 @@ static int nuc900fb_init_registers(struct fb_info *info)
  *    The buffer should be a non-cached, non-buffered, memory region
  *    to allow palette and pixel writes without flushing the cache.
  */
-static int __init nuc900fb_map_video_memory(struct fb_info *info)
+static int __devinit nuc900fb_map_video_memory(struct fb_info *info)
 {
 	struct nuc900fb_info *fbi = info->par;
 	dma_addr_t map_dma;
-- 
1.7.10

[PATCH 13/16] ARM: be really quiet when building with 'make -s'

[Arnd Bergmann]

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 <arnd@arndb.de>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Michal Marek <mmarek@suse.cz>
---
 arch/arm/boot/Makefile  |   10 +++++-----
 arch/arm/tools/Makefile |    2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
index c877087..89680f2 100644
--- a/arch/arm/boot/Makefile
+++ b/arch/arm/boot/Makefile
@@ -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:
diff --git a/arch/arm/tools/Makefile b/arch/arm/tools/Makefile
index 635cb18..cd60a81 100644
--- a/arch/arm/tools/Makefile
+++ b/arch/arm/tools/Makefile
@@ -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; }
-- 
1.7.10

Re: [PATCH 13/16] ARM: be really quiet when building with 'make -s'

[Nicolas Pitre]

On Fri, 5 Oct 2012, Arnd Bergmann wrote:

> 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 <arnd@arndb.de>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Michal Marek <mmarek@suse.cz>

Acked-by: Nicolas Pitre <nico@linaro.org>


> ---
>  arch/arm/boot/Makefile  |   10 +++++-----
>  arch/arm/tools/Makefile |    2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
> index c877087..89680f2 100644
> --- a/arch/arm/boot/Makefile
> +++ b/arch/arm/boot/Makefile
> @@ -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:
> diff --git a/arch/arm/tools/Makefile b/arch/arm/tools/Makefile
> index 635cb18..cd60a81 100644
> --- a/arch/arm/tools/Makefile
> +++ b/arch/arm/tools/Makefile
> @@ -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; }
> -- 
> 1.7.10
> 

[PATCH 14/16] ARM: pxa: armcore: fix PCI PIO warnings

[Arnd Bergmann]

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 <arnd@arndb.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Krzysztof Halasa <khc@pm.waw.pl>
Cc: Mike Rapoport <mike@compulab.co.il>
Cc: Igor Grinberg <grinberg@compulab.co.il>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Eric Miao <eric.y.miao@gmail.com>
---
 arch/arm/common/it8152.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm/common/it8152.c b/arch/arm/common/it8152.c
index c4110d1..001f491 100644
--- a/arch/arm/common/it8152.c
+++ b/arch/arm/common/it8152.c
@@ -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");
-- 
1.7.10

Re: [PATCH 14/16] ARM: pxa: armcore: fix PCI PIO warnings

[Igor Grinberg]

On 10/05/12 16:55, Arnd Bergmann wrote:
> 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 <arnd@arndb.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Krzysztof Halasa <khc@pm.waw.pl>
> Cc: Mike Rapoport <mike@compulab.co.il>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
> Cc: Eric Miao <eric.y.miao@gmail.com>

Acked-by: Igor Grinberg <grinberg@compulab.co.il>

Thanks!

> ---
>  arch/arm/common/it8152.c |   12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/common/it8152.c b/arch/arm/common/it8152.c
> index c4110d1..001f491 100644
> --- a/arch/arm/common/it8152.c
> +++ b/arch/arm/common/it8152.c
> @@ -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");

-- 
Regards,
Igor.

[PATCH 15/16] spi/s3c64xx: use correct dma_transfer_direction type

[Arnd Bergmann]

There is a subtle difference between dma_transfer_direction and
dma_data_direction: the former is used by the dmaengine framework,
while the latter is used by the dma-mapping API. Although the
purpose is comparable, the actual values are different and must
not be mixed. In this case, the driver just wants to use
dma_transfer_direction.

Without this patch, building s3c6400_defconfig results in:

drivers/spi/spi-s3c64xx.c: In function 's3c64xx_spi_dmacb':
drivers/spi/spi-s3c64xx.c:239:21: warning: comparison between
	'enum dma_data_direction' and 'enum dma_transfer_direction' [-Wenum-compare]

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: spi-devel-general@lists.sourceforge.net
---
 drivers/spi/spi-s3c64xx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index d1c8441f..2e44dd6 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -132,7 +132,7 @@
 
 struct s3c64xx_spi_dma_data {
 	unsigned		ch;
-	enum dma_data_direction direction;
+	enum dma_transfer_direction direction;
 	enum dma_ch	dmach;
 	struct property		*dma_prop;
 };
-- 
1.7.10

[PATCH 16/16] ARM: pass -marm to gcc by default for both C and assembler

[Arnd Bergmann]

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 <arnd@arndb.de>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/Makefile |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index a051dfb..4391427 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -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__
 
-- 
1.7.10

Re: [PATCH 16/16] ARM: pass -marm to gcc by default for both C and assembler

[Nicolas Pitre]

On Fri, 5 Oct 2012, Arnd Bergmann wrote:

> 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 <arnd@arndb.de>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Dave Martin <dave.martin@linaro.org>

I don't think you need "call cc-option" with -marm.  Those gcc versions 
that might not accept -marm have not been able to compile the kernel for 
a long long time now.  Otherwise...

Acked-by: Nicolas Pitre <nico@linaro.org>

> ---
>  arch/arm/Makefile |   13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index a051dfb..4391427 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -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__
>  
> -- 
> 1.7.10
>