On Mon, Sep 10, 2012 at 10:14:07AM +0900, Minchan Kim wrote:
> Hi Andrew,
>
> On Fri, Sep 07, 2012 at 01:06:05PM -0700, Andrew Morton wrote:
> > On Fri, 7 Sep 2012 09:57:10 +0900
> > Minchan Kim <minchan@xxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > Cc: Haggai Eran <haggaie@xxxxxxxxxxxx>
> > > Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> > > ---
> > > 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?
>
> My compiler is stupidl, too. It still emit the samw warning when I apply
> your patch. In addition, I can't see any benefit of text size.
barrios@bbox:~/linux-mmotm$ gcc --version
gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3
--
Kind regards,
Minchan Kim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>