Warning message when compiling ioremap.c

Warning message when compiling ioremap.c

[Claudio Scordino]

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

Hi,

       I'm not skilled with MM at all, so sorry if I'm saying something
stupid.

When compiling Linux (latest kernel from Linus' git) on ARM, I noticed 
the following warning:

CC      arch/arm/mm/ioremap.o
arch/arm/mm/ioremap.c: In function '__arm_ioremap_pfn':
arch/arm/mm/ioremap.c:83: warning: control may reach end of non-void
function 'remap_area_pte' being inlined

According to the message in the printk, we go to "bad" when the page
already exists.

So, I'm wondering if we shouldn't return a -EEXIST (see the patch
attached). This would remove that annoying warning message during
compilation...

Is it a good/bad idea ?

Regards,

              Claudio





[-- Attachment #2: 0001-Fix-compilation-warning-related-to-return-value-in-r.patch --]
[-- Type: text/x-diff, Size: 820 bytes --]

>From 6ebc8f240bb8ca1cd640994ddb9280fb450f3f04 Mon Sep 17 00:00:00 2001
From: Claudio Scordino <claudio@evidence.eu.com>
Date: Mon, 1 Sep 2008 11:30:55 +0200
Subject: [PATCH 1/1] Fix compilation warning related to return value in remap_area_pte(...).


Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
 arch/arm/mm/ioremap.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index b81dbf9..1e59a03 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -64,6 +64,7 @@ static int remap_area_pte(pmd_t *pmd, unsigned long addr, unsigned long end,
  bad:
 	printk(KERN_CRIT "remap_area_pte: page already exists\n");
 	BUG();
+	return -EEXIST;
 }
 
 static inline int remap_area_pmd(pgd_t *pgd, unsigned long addr,
-- 
1.5.4.3

Re: Warning message when compiling ioremap.c

[Luiz Fernando N. Capitulino]

Em Tue, 02 Sep 2008 09:37:14 +0200
Claudio Scordino <claudio@evidence.eu.com> escreveu:

| Hi,
| 
|        I'm not skilled with MM at all, so sorry if I'm saying something
| stupid.
| 
| When compiling Linux (latest kernel from Linus' git) on ARM, I noticed 
| the following warning:
| 
| CC      arch/arm/mm/ioremap.o
| arch/arm/mm/ioremap.c: In function '__arm_ioremap_pfn':
| arch/arm/mm/ioremap.c:83: warning: control may reach end of non-void
| function 'remap_area_pte' being inlined
| 
| According to the message in the printk, we go to "bad" when the page
| already exists.

 You see that right before the return you have added there is a
BUG() macro? That macro will call panic(), this means that this
function will never return if it reaches that point.

 If all you want is to silent gcc, you should remove the goto and
move the bad label contents there.

 This is minor, but I see no need for the goto.

-- 
Luiz Fernando N. Capitulino

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

Re: Warning message when compiling ioremap.c

[Claudio Scordino]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

Luiz Fernando N. Capitulino ha scritto:
> Em Tue, 02 Sep 2008 09:37:14 +0200
> Claudio Scordino <claudio@evidence.eu.com> escreveu:
> 
> | Hi,
> | 
> |        I'm not skilled with MM at all, so sorry if I'm saying something
> | stupid.
> | 
> | When compiling Linux (latest kernel from Linus' git) on ARM, I noticed 
> | the following warning:
> | 
> | CC      arch/arm/mm/ioremap.o
> | arch/arm/mm/ioremap.c: In function '__arm_ioremap_pfn':
> | arch/arm/mm/ioremap.c:83: warning: control may reach end of non-void
> | function 'remap_area_pte' being inlined
> | 
> | According to the message in the printk, we go to "bad" when the page
> | already exists.
> 
>  You see that right before the return you have added there is a
> BUG() macro? That macro will call panic(), this means that this
> function will never return if it reaches that point.

Well, probably BUG() doesn't call panic() always. For instance, in
arch/arm/include/asm/bug.h in case CONFIG_DEBUG_BUGVERBOSE is defined
(and it might be), BUG just causes an oops (by dereferencing a NULL
pointer). If I'm not wrong this doesn't always mean a panic...

However, in any case, the handler of pagefault eventually calls
do_exit(), so you're right: what follows BUG() won't be executed.

>  If all you want is to silent gcc, you should remove the goto and
> move the bad label contents there.
> 
>  This is minor, but I see no need for the goto.

Yes, it's obviously minor. But I don't like having meaningless
warnings during compilation: they just confuse output, and people may
miss some important warning message...

The need for the goto exists only if BUG() can return, and it doesn't, 
so we can safely remove it as you suggested.

Who's in charge of maintaining this piece of code? Should the patch in 
attachment be submitted to some specific person?

Many thanks,

         Claudio

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-compilation-warning-in-remap_area_pte.patch --]
[-- Type: text/x-diff; name="0001-Fix-compilation-warning-in-remap_area_pte.patch", Size: 0 bytes --]

Re: Warning message when compiling ioremap.c

[Phil Blundell]

On Mon, 2008-09-08 at 15:11 +0200, Claudio Scordino wrote: 
> The need for the goto exists only if BUG() can return, and it doesn't, 
> so we can safely remove it as you suggested.

The structure of the original code (with the goto) is arranged so that
the code path is a straight line for the normal, non-bad case.  If you
want to remove the goto, you should wrap the condition in unlikely() so
as not to introduce another branch.

> Who's in charge of maintaining this piece of code? Should the patch
> in attachment be submitted to some specific person?

I guess you should send it to the linux-arm-kernel list.

p.


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