* Re: [PATCH] hwpoison: fix uninitialized warning
2009-09-15 21:19 [PATCH] hwpoison: fix uninitialized warning Hugh Dickins
@ 2009-09-15 23:59 ` Wu Fengguang
2009-09-16 0:12 ` Minchan Kim
2009-09-16 0:23 ` Wu Fengguang
2 siblings, 0 replies; 6+ messages in thread
From: Wu Fengguang @ 2009-09-15 23:59 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andi Kleen, Andrew Morton, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
On Wed, Sep 16, 2009 at 05:19:07AM +0800, Hugh Dickins wrote:
> Fix mmotm build warning, presumably also in linux-next:
> mm/memory.c: In function `do_swap_page':
> mm/memory.c:2498: warning: `pte' may be used uninitialized in this function
Thanks for the fix!
Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> ---
> I've only noticed this warning on one machine, the powerpc: certainly it
> needs CONFIG_MIGRATION or CONFIG_MEMORY_FAILURE to see it, but I thought
> I had one of those set on other machines - just musing in case it's being
> masked elsewhere by some other bug...
>
> mm/memory.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- mmotm/mm/memory.c 2009-09-14 16:34:37.000000000 +0100
> +++ linux/mm/memory.c 2009-09-15 22:00:48.000000000 +0100
> @@ -2495,7 +2495,7 @@ static int do_swap_page(struct mm_struct
> } else if (is_hwpoison_entry(entry)) {
> ret = VM_FAULT_HWPOISON;
> } else {
> - print_bad_pte(vma, address, pte, NULL);
> + print_bad_pte(vma, address, orig_pte, NULL);
> ret = VM_FAULT_OOM;
> }
> goto out;
--
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] 6+ messages in thread* Re: [PATCH] hwpoison: fix uninitialized warning
2009-09-15 21:19 [PATCH] hwpoison: fix uninitialized warning Hugh Dickins
2009-09-15 23:59 ` Wu Fengguang
@ 2009-09-16 0:12 ` Minchan Kim
2009-09-16 0:23 ` Wu Fengguang
2 siblings, 0 replies; 6+ messages in thread
From: Minchan Kim @ 2009-09-16 0:12 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andi Kleen, Wu Fengguang, Andrew Morton, linux-kernel, linux-mm
On Wed, Sep 16, 2009 at 6:19 AM, Hugh Dickins
<hugh.dickins@tiscali.co.uk> wrote:
> Fix mmotm build warning, presumably also in linux-next:
> mm/memory.c: In function `do_swap_page':
> mm/memory.c:2498: warning: `pte' may be used uninitialized in this function
>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
--
Kind regards,
Minchan Kim
--
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] 6+ messages in thread
* Re: [PATCH] hwpoison: fix uninitialized warning
2009-09-15 21:19 [PATCH] hwpoison: fix uninitialized warning Hugh Dickins
2009-09-15 23:59 ` Wu Fengguang
2009-09-16 0:12 ` Minchan Kim
@ 2009-09-16 0:23 ` Wu Fengguang
2009-09-16 0:51 ` Hugh Dickins
2 siblings, 1 reply; 6+ messages in thread
From: Wu Fengguang @ 2009-09-16 0:23 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andi Kleen, Andrew Morton, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
On Wed, Sep 16, 2009 at 05:19:07AM +0800, Hugh Dickins wrote:
> Fix mmotm build warning, presumably also in linux-next:
> mm/memory.c: In function `do_swap_page':
> mm/memory.c:2498: warning: `pte' may be used uninitialized in this function
>
> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> ---
> I've only noticed this warning on one machine, the powerpc: certainly it
> needs CONFIG_MIGRATION or CONFIG_MEMORY_FAILURE to see it, but I thought
> I had one of those set on other machines - just musing in case it's being
> masked elsewhere by some other bug...
>
> mm/memory.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- mmotm/mm/memory.c 2009-09-14 16:34:37.000000000 +0100
> +++ linux/mm/memory.c 2009-09-15 22:00:48.000000000 +0100
> @@ -2495,7 +2495,7 @@ static int do_swap_page(struct mm_struct
> } else if (is_hwpoison_entry(entry)) {
> ret = VM_FAULT_HWPOISON;
> } else {
> - print_bad_pte(vma, address, pte, NULL);
> + print_bad_pte(vma, address, orig_pte, NULL);
> ret = VM_FAULT_OOM;
> }
The lines was introduced in this patch:
entry = pte_to_swp_entry(orig_pte);
- if (is_migration_entry(entry)) {
- migration_entry_wait(mm, pmd, address);
+ if (unlikely(non_swap_entry(entry))) {
+ if (is_migration_entry(entry)) {
+ migration_entry_wait(mm, pmd, address);
+ } else if (is_hwpoison_entry(entry)) {
+ ret = VM_FAULT_HWPOISON;
+ } else {
+ print_bad_pte(vma, address, pte, NULL);
+ ret = VM_FAULT_OOM;
+ }
goto out;
}
Given that currently there are only two types of non swap entries:
migration/hwpoison, the last 'else' block is in fact dead code..
Thanks,
Fengguang
--
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] 6+ messages in thread* Re: [PATCH] hwpoison: fix uninitialized warning
2009-09-16 0:23 ` Wu Fengguang
@ 2009-09-16 0:51 ` Hugh Dickins
2009-09-16 1:08 ` Wu Fengguang
0 siblings, 1 reply; 6+ messages in thread
From: Hugh Dickins @ 2009-09-16 0:51 UTC (permalink / raw)
To: Wu Fengguang
Cc: Andi Kleen, Andrew Morton, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
On Wed, 16 Sep 2009, Wu Fengguang wrote:
> On Wed, Sep 16, 2009 at 05:19:07AM +0800, Hugh Dickins wrote:
> > Fix mmotm build warning, presumably also in linux-next:
> > mm/memory.c: In function `do_swap_page':
> > mm/memory.c:2498: warning: `pte' may be used uninitialized in this function
> >
> > Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> > ---
> > I've only noticed this warning on one machine, the powerpc: certainly it
> > needs CONFIG_MIGRATION or CONFIG_MEMORY_FAILURE to see it, but I thought
> > I had one of those set on other machines - just musing in case it's being
> > masked elsewhere by some other bug...
> The lines was introduced in this patch:
>
> entry = pte_to_swp_entry(orig_pte);
> - if (is_migration_entry(entry)) {
> - migration_entry_wait(mm, pmd, address);
> + if (unlikely(non_swap_entry(entry))) {
> + if (is_migration_entry(entry)) {
> + migration_entry_wait(mm, pmd, address);
> + } else if (is_hwpoison_entry(entry)) {
> + ret = VM_FAULT_HWPOISON;
> + } else {
> + print_bad_pte(vma, address, pte, NULL);
> + ret = VM_FAULT_OOM;
> + }
> goto out;
> }
>
> Given that currently there are only two types of non swap entries:
> migration/hwpoison, the last 'else' block is in fact dead code..
Ah, yes, I think it is dead code on x86 (32 and 64), where the
swp_entry_t is well packed. But not dead code on ppc64, which has
#define __swp_type(entry) (((entry).val >> 1) & 0x3f)
which is allowing swap types up to 63, when in fact the highest
we use is 31: that leaves space for 32 more non_swap_entry types.
So the compiler was absolutely right to complain about the
uninitialized variable on ppc64, but not on x86. It's a little
surprising that ppc64 allows 64 swap types, but nothing wrong.
Thanks,
Hugh
--
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] 6+ messages in thread* Re: [PATCH] hwpoison: fix uninitialized warning
2009-09-16 0:51 ` Hugh Dickins
@ 2009-09-16 1:08 ` Wu Fengguang
0 siblings, 0 replies; 6+ messages in thread
From: Wu Fengguang @ 2009-09-16 1:08 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andi Kleen, Andrew Morton, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
On Wed, Sep 16, 2009 at 08:51:06AM +0800, Hugh Dickins wrote:
> On Wed, 16 Sep 2009, Wu Fengguang wrote:
> > On Wed, Sep 16, 2009 at 05:19:07AM +0800, Hugh Dickins wrote:
> > > Fix mmotm build warning, presumably also in linux-next:
> > > mm/memory.c: In function `do_swap_page':
> > > mm/memory.c:2498: warning: `pte' may be used uninitialized in this function
> > >
> > > Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
> > > ---
> > > I've only noticed this warning on one machine, the powerpc: certainly it
> > > needs CONFIG_MIGRATION or CONFIG_MEMORY_FAILURE to see it, but I thought
> > > I had one of those set on other machines - just musing in case it's being
> > > masked elsewhere by some other bug...
>
> > The lines was introduced in this patch:
> >
> > entry = pte_to_swp_entry(orig_pte);
> > - if (is_migration_entry(entry)) {
> > - migration_entry_wait(mm, pmd, address);
> > + if (unlikely(non_swap_entry(entry))) {
> > + if (is_migration_entry(entry)) {
> > + migration_entry_wait(mm, pmd, address);
> > + } else if (is_hwpoison_entry(entry)) {
> > + ret = VM_FAULT_HWPOISON;
> > + } else {
> > + print_bad_pte(vma, address, pte, NULL);
> > + ret = VM_FAULT_OOM;
> > + }
> > goto out;
> > }
> >
> > Given that currently there are only two types of non swap entries:
> > migration/hwpoison, the last 'else' block is in fact dead code..
>
> Ah, yes, I think it is dead code on x86 (32 and 64), where the
> swp_entry_t is well packed. But not dead code on ppc64, which has
>
> #define __swp_type(entry) (((entry).val >> 1) & 0x3f)
>
> which is allowing swap types up to 63, when in fact the highest
> we use is 31: that leaves space for 32 more non_swap_entry types.
>
> So the compiler was absolutely right to complain about the
> uninitialized variable on ppc64, but not on x86. It's a little
> surprising that ppc64 allows 64 swap types, but nothing wrong.
Ah I know. It seems that gcc is smart enough to remove that dead code
and hence the warning message in x86 :)
Thanks,
Fengguang
--
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] 6+ messages in thread