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 <[email protected]>
---
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;
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 <[email protected]>
> Signed-off-by: Hugh Dickins <[email protected]>
> ---
> 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;
On Wed, Sep 16, 2009 at 6:19 AM, Hugh Dickins
<[email protected]> 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 <[email protected]>
Reviewed-by: Minchan Kim <[email protected]>
--
Kind regards,
Minchan Kim
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> > ---
> > 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
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 <[email protected]>
> > > ---
> > > 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