* Remove warning in compilation of ioremap
@ 2008-09-09 9:13 Claudio Scordino
2008-09-09 9:58 ` Russell King - ARM Linux
2008-09-09 13:08 ` David Anders
0 siblings, 2 replies; 7+ messages in thread
From: Claudio Scordino @ 2008-09-09 9:13 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: linux-mm, Luiz Fernando N. Capitulino, Phil Blundell
[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]
Hi all.
[We already discussed this issue in linux-mm ML, but people suggested
to post to linux-arm-kernel...]
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
If you look at the code, the problem is in a path including a BUG().
AFAIK, on ARM the code following BUG() is never executed: it's a NULL
pointer dereference, so the handler of pagefault eventually calls
do_exit(). Therefore, we may want to remove the goto as shown in the
patch in attachment.
It's obviously a minor issue. But I don't like having meaningless
warnings during compilation: they just confuse output, and developers
may miss some important warning message...
The need for the goto exists only if BUG() can return. If it doesn't,
we can safely remove it as shown in the patch.
Is this possible ? Should we update this piece of code ? Who's in
charge of maintaining it ?
Many thanks,
Claudio
[-- Attachment #2: 0001-Fix-compilation-warning-in-remap_area_pte.patch --]
[-- Type: text/x-diff, Size: 1148 bytes --]
>From 08d2e6f14230bf2252c54f5421d92def5e70f6dc Mon Sep 17 00:00:00 2001
From: Claudio Scordino <claudio@evidence.eu.com>
Date: Mon, 8 Sep 2008 16:03:38 +0200
Subject: [PATCH 1/1] Fix compilation warning in remap_area_pte
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
arch/arm/mm/ioremap.c | 11 ++++-------
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index b81dbf9..bc6eca0 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -52,18 +52,15 @@ static int remap_area_pte(pmd_t *pmd, unsigned long addr, unsigned long end,
return -ENOMEM;
do {
- if (!pte_none(*pte))
- goto bad;
-
+ if (unlikely(!pte_none(*pte))){
+ printk(KERN_CRIT "%s: page already exists\n", __FUNCTION__);
+ BUG();
+ }
set_pte_ext(pte, pfn_pte(phys_addr >> PAGE_SHIFT, prot),
type->prot_pte_ext);
phys_addr += PAGE_SIZE;
} while (pte++, addr += PAGE_SIZE, addr != end);
return 0;
-
- bad:
- printk(KERN_CRIT "remap_area_pte: page already exists\n");
- BUG();
}
static inline int remap_area_pmd(pgd_t *pgd, unsigned long addr,
--
1.5.4.3
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Remove warning in compilation of ioremap
2008-09-09 9:13 Remove warning in compilation of ioremap Claudio Scordino
@ 2008-09-09 9:58 ` Russell King - ARM Linux
2008-09-09 13:49 ` Claudio Scordino
2008-09-09 13:08 ` David Anders
1 sibling, 1 reply; 7+ messages in thread
From: Russell King - ARM Linux @ 2008-09-09 9:58 UTC (permalink / raw)
To: Claudio Scordino
Cc: linux-arm-kernel, linux-mm, Phil Blundell, Luiz Fernando N. Capitulino
On Tue, Sep 09, 2008 at 11:13:12AM +0200, Claudio Scordino wrote:
> 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
>
> If you look at the code, the problem is in a path including a BUG().
>
> AFAIK, on ARM the code following BUG() is never executed: it's a NULL
> pointer dereference, so the handler of pagefault eventually calls
> do_exit(). Therefore, we may want to remove the goto as shown in the
> patch in attachment.
>
> It's obviously a minor issue. But I don't like having meaningless
> warnings during compilation: they just confuse output, and developers
> may miss some important warning message...
>
> The need for the goto exists only if BUG() can return. If it doesn't,
> we can safely remove it as shown in the patch.
NAK. See patch 5211/2 in the patch system.
--
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Remove warning in compilation of ioremap
2008-09-09 9:58 ` Russell King - ARM Linux
@ 2008-09-09 13:49 ` Claudio Scordino
0 siblings, 0 replies; 7+ messages in thread
From: Claudio Scordino @ 2008-09-09 13:49 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-arm-kernel, linux-mm, Phil Blundell, Luiz Fernando N. Capitulino
Russell King - ARM Linux ha scritto:
> On Tue, Sep 09, 2008 at 11:13:12AM +0200, Claudio Scordino wrote:
>> 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
>>
>> If you look at the code, the problem is in a path including a BUG().
>>
>> AFAIK, on ARM the code following BUG() is never executed: it's a NULL
>> pointer dereference, so the handler of pagefault eventually calls
>> do_exit(). Therefore, we may want to remove the goto as shown in the
>> patch in attachment.
>>
>> It's obviously a minor issue. But I don't like having meaningless
>> warnings during compilation: they just confuse output, and developers
>> may miss some important warning message...
>>
>> The need for the goto exists only if BUG() can return. If it doesn't,
>> we can safely remove it as shown in the patch.
>
> NAK. See patch 5211/2 in the patch system.
Seen.
Thanks,
Claudio
--
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Remove warning in compilation of ioremap
2008-09-09 9:13 Remove warning in compilation of ioremap Claudio Scordino
2008-09-09 9:58 ` Russell King - ARM Linux
@ 2008-09-09 13:08 ` David Anders
2008-09-09 13:55 ` Russell King - ARM Linux
1 sibling, 1 reply; 7+ messages in thread
From: David Anders @ 2008-09-09 13:08 UTC (permalink / raw)
To: linux-arm-kernel, Claudio Scordino
Cc: linux-mm, Phil Blundell, Luiz Fernando N. Capitulino
Claudio,
i hope you have a better time getting this fixed than i have, i've been submitting patches as far back as 2.6.16:
http://lists.arm.linux.org.uk/lurker/message/20070906.135142.6c5e4d6f.en.html
http://lists.arm.linux.org.uk/lurker/message/20070906.140649.79f143a0.en.html
2.6.23 was when i gave up.
Dave Anders
--- On Tue, 9/9/08, Claudio Scordino <claudio@evidence.eu.com> wrote:
> From: Claudio Scordino <claudio@evidence.eu.com>
> Subject: Remove warning in compilation of ioremap
> To: linux-arm-kernel@lists.arm.linux.org.uk
> Cc: linux-mm@kvack.org, "Phil Blundell" <philb@gnu.org>, "Luiz Fernando N. Capitulino" <lcapitulino@mandriva.com.br>
> Date: Tuesday, September 9, 2008, 4:13 AM
> Hi all.
>
> [We already discussed this issue in linux-mm ML, but people
> suggested
> to post to linux-arm-kernel...]
>
> 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
>
> If you look at the code, the problem is in a path including
> a BUG().
>
> AFAIK, on ARM the code following BUG() is never executed:
> it's a NULL
> pointer dereference, so the handler of pagefault eventually
> calls
> do_exit(). Therefore, we may want to remove the goto as
> shown in the
> patch in attachment.
>
> It's obviously a minor issue. But I don't like
> having meaningless
> warnings during compilation: they just confuse output, and
> developers
> may miss some important warning message...
>
> The need for the goto exists only if BUG() can return. If
> it doesn't,
> we can safely remove it as shown in the patch.
>
> Is this possible ? Should we update this piece of code ?
> Who's in
> charge of maintaining it ?
>
> Many thanks,
>
> Claudio
>
>
>
>
>
> >From 08d2e6f14230bf2252c54f5421d92def5e70f6dc Mon Sep
> 17 00:00:00 2001
> From: Claudio Scordino <claudio@evidence.eu.com>
> Date: Mon, 8 Sep 2008 16:03:38 +0200
> Subject: [PATCH 1/1] Fix compilation warning in
> remap_area_pte
>
>
> Signed-off-by: Claudio Scordino
> <claudio@evidence.eu.com>
> ---
> arch/arm/mm/ioremap.c | 11 ++++-------
> 1 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
> index b81dbf9..bc6eca0 100644
> --- a/arch/arm/mm/ioremap.c
> +++ b/arch/arm/mm/ioremap.c
> @@ -52,18 +52,15 @@ static int remap_area_pte(pmd_t *pmd,
> unsigned long addr, unsigned long end,
> return -ENOMEM;
>
> do {
> - if (!pte_none(*pte))
> - goto bad;
> -
> + if (unlikely(!pte_none(*pte))){
> + printk(KERN_CRIT "%s: page already
> exists\n", __FUNCTION__);
> + BUG();
> + }
> set_pte_ext(pte, pfn_pte(phys_addr >> PAGE_SHIFT,
> prot),
> type->prot_pte_ext);
> phys_addr += PAGE_SIZE;
> } while (pte++, addr += PAGE_SIZE, addr != end);
> return 0;
> -
> - bad:
> - printk(KERN_CRIT "remap_area_pte: page already
> exists\n");
> - BUG();
> }
>
> static inline int remap_area_pmd(pgd_t *pgd, unsigned long
> addr,
> --
> 1.5.4.3
>
>
>
> -------------------------------------------------------------------
> List admin:
> http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
> FAQ:
> http://www.arm.linux.org.uk/mailinglists/faq.php
> Etiquette:
> http://www.arm.linux.org.uk/mailinglists/etiquette.php
--
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>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Remove warning in compilation of ioremap
2008-09-09 13:08 ` David Anders
@ 2008-09-09 13:55 ` Russell King - ARM Linux
2008-09-09 14:59 ` David Anders
0 siblings, 1 reply; 7+ messages in thread
From: Russell King - ARM Linux @ 2008-09-09 13:55 UTC (permalink / raw)
To: David Anders
Cc: linux-arm-kernel, Claudio Scordino, linux-mm, Phil Blundell,
Luiz Fernando N. Capitulino
On Tue, Sep 09, 2008 at 06:08:21AM -0700, David Anders wrote:
> Claudio,
>
> i hope you have a better time getting this fixed than i have,
> i've been submitting patches as far back as 2.6.16:
>
> http://lists.arm.linux.org.uk/lurker/message/20070906.135142.6c5e4d6f.en.html
> http://lists.arm.linux.org.uk/lurker/message/20070906.140649.79f143a0.en.html
>
> 2.6.23 was when i gave up.
It's not like you were ignored, both of those messages contain replies
from Erik Mouw, both of which were positive.
Unfortunately, I didn't see it as a high priority so it got left a little
too long and I never got around to commenting about it (I thought it was
a complex way of fixing what was a trivial problem.)
Take a look at the difference between yours:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4563/1
Comapred with the one which has been merged:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=5211/2
Sorry.
--
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Remove warning in compilation of ioremap
2008-09-09 13:55 ` Russell King - ARM Linux
@ 2008-09-09 14:59 ` David Anders
2008-09-09 15:21 ` Russell King - ARM Linux
0 siblings, 1 reply; 7+ messages in thread
From: David Anders @ 2008-09-09 14:59 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-arm-kernel, Claudio Scordino, linux-mm, Phil Blundell,
Luiz Fernando N. Capitulino
Russell,
> > i hope you have a better time getting this fixed than
> i have,
> > i've been submitting patches as far back as
> 2.6.16:
> >
> >
> http://lists.arm.linux.org.uk/lurker/message/20070906.135142.6c5e4d6f.en.html
> >
> http://lists.arm.linux.org.uk/lurker/message/20070906.140649.79f143a0.en.html
> >
> > 2.6.23 was when i gave up.
>
> It's not like you were ignored, both of those messages
> contain replies
> from Erik Mouw, both of which were positive.
>
> Unfortunately, I didn't see it as a high priority so it
> got left a little
> too long and I never got around to commenting about it (I
> thought it was
> a complex way of fixing what was a trivial problem.)
>
water under the bridge, just happy it got fixed.
> Take a look at the difference between yours:
>
>
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=4563/1
>
> Comapred with the one which has been merged:
>
>
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=5211/2
>
> Sorry.
agreed there are differences between what got accepted and what i submited. however, i did not randomly choose the code used in that define. i went back to the mailing lists for x86,powerpc, parisc, mips and sh. the code format of BUG() had been discussed on each of these lists, with x86, powerpc, and parisc using the exact same code format as i submitted, with mips and sh using one almost identical.
from include/asm-x86/bug.h
#define BUG() \
do { \
asm volatile("ud2"); \
for(;;) ; \
} while(0)
what i submited:
+#define BUG() \
+ do { \
+ (*(int *)0 = 0); \
+ for(;;) ; \
+ } while(0)
i apologize if this is a rant, no disrespect for your work or the pressure you are under to maintain LAK, but shall we always wait until there is a 100% elegant solution to a known issue before fixing it?
Dave Anders
--
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>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Remove warning in compilation of ioremap
2008-09-09 14:59 ` David Anders
@ 2008-09-09 15:21 ` Russell King - ARM Linux
0 siblings, 0 replies; 7+ messages in thread
From: Russell King - ARM Linux @ 2008-09-09 15:21 UTC (permalink / raw)
To: David Anders
Cc: linux-mm, Phil Blundell, Luiz Fernando N. Capitulino, linux-arm-kernel
On Tue, Sep 09, 2008 at 07:59:27AM -0700, David Anders wrote:
> Russell,
>...
> i apologize if this is a rant, no disrespect for your work or the
> pressure you are under to maintain LAK, but shall we always wait
> until there is a 100% elegant solution to a known issue before
> fixing it?
How about realising that I'm not perfect, and sometimes need reminding
(*after* a reasonable period of about a week or so)?
Rather than just saying "I submitted this, got ignored and gave up" ?
It's a bit like putting a slip of paper in a secretaries tray from your
doctor requesting another appointment in 4 weeks time. You wait and
wait for notification, which never comes.
What do you do? Do you give up? Or do you wait a reasonable time
before telephoning the secretary to find out what's happening?
Guess what I did this morning? I telephoned the secretary to find
out what was happening. Result is I now have an appointment. I didn't
give up.
This is no different. People forget things. Things get mislayed.
People have to be constantly hastled to do anything. That's life.
--
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-09-09 15:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-09 9:13 Remove warning in compilation of ioremap Claudio Scordino
2008-09-09 9:58 ` Russell King - ARM Linux
2008-09-09 13:49 ` Claudio Scordino
2008-09-09 13:08 ` David Anders
2008-09-09 13:55 ` Russell King - ARM Linux
2008-09-09 14:59 ` David Anders
2008-09-09 15:21 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox