Re: [PATCH] mm: Fix compile warning of mmotm-2012-09-06-16-46

[Andrew Morton <akpm@linux-foundation.org>]

On Fri,  7 Sep 2012 09:57:10 +0900
Minchan Kim <minchan@kernel.org> wrote:

> When I compiled today, I met following warning.
> Correct it.
> 
> mm/memory.c: In function ___copy_page_range___:
> include/linux/mmu_notifier.h:235:38: warning: ___mmun_end___ may be used uninitialized in this function [-Wuninitialized]
> mm/memory.c:1043:16: note: ___mmun_end___ was declared here
> include/linux/mmu_notifier.h:235:38: warning: ___mmun_start___ may be used uninitialized in this function [-Wuninitialized]
> mm/memory.c:1042:16: note: ___mmun_start___ was declared here
>   LD      mm/built-in.o
> 
> Cc: Sagi Grimberg <sagig@mellanox.com>
> Cc: Haggai Eran <haggaie@mellanox.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  mm/memory.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 10e9b38..d000449 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1039,8 +1039,8 @@ int copy_page_range(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>  	unsigned long next;
>  	unsigned long addr = vma->vm_start;
>  	unsigned long end = vma->vm_end;
> -	unsigned long mmun_start;	/* For mmu_notifiers */
> -	unsigned long mmun_end;		/* For mmu_notifiers */
> +	unsigned long uninitialized_var(mmun_start);	/* For mmu_notifiers */
> +	unsigned long uninitialized_var(mmun_end);	/* For mmu_notifiers */
>  	int ret;
>  

Well yes, but a) uninitialized_var is a bit ugly and has some potential
to hide real bugs and b) it's not completely obvious that
is_cow_mapping() is stable across those two calls.

I think a better approach is this:

--- a/mm/memory.c~mm-move-all-mmu-notifier-invocations-to-be-done-outside-the-pt-lock-fix-fix
+++ a/mm/memory.c
@@ -1041,6 +1041,7 @@ int copy_page_range(struct mm_struct *ds
 	unsigned long end = vma->vm_end;
 	unsigned long mmun_start;	/* For mmu_notifiers */
 	unsigned long mmun_end;		/* For mmu_notifiers */
+	bool is_cow;
 	int ret;
 
 	/*
@@ -1074,7 +1075,8 @@ int copy_page_range(struct mm_struct *ds
 	 * parent mm. And a permission downgrade will only happen if
 	 * is_cow_mapping() returns true.
 	 */
-	if (is_cow_mapping(vma->vm_flags)) {
+	is_cow = is_cow_mapping(vma->vm_flags);
+	if (is_cow) {
 		mmun_start = addr;
 		mmun_end   = end;
 		mmu_notifier_invalidate_range_start(src_mm, mmun_start,
@@ -1095,7 +1097,7 @@ int copy_page_range(struct mm_struct *ds
 		}
 	} while (dst_pgd++, src_pgd++, addr = next, addr != end);
 
-	if (is_cow_mapping(vma->vm_flags))
+	if (is_cow)
 		mmu_notifier_invalidate_range_end(src_mm, mmun_start, mmun_end);
 	return ret;
 }

Unfortunately, my (old) versions of gcc are so stupid that they *still*
generate the warning even when the code is as obviously correct as this :(

Can you please test it with your compiler?

We should use this patch anyway - it makes memory.o text 13 bytes smaller.

is_cow_mapping() should return bool.

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