Subject: Re: linux-next: build warnings after merge of the akpm-current tree

From: Stephen Rothwell <[email-masked]>
Date: Wed, 7 Oct 2020 09:03:27 +1100

Hi all,

On Tue, 6 Oct 2020 16:01:38 -0400 Peter Xu <[email-masked]> wrote:
>
> On Tue, Oct 06, 2020 at 11:05:16PM +1100, Stephen Rothwell wrote:
> > 
> > After merging the akpm-current tree, today's linux-next build (sparc
> > defconfig) produced this warning:
> > 
> > mm/memory.c: In function 'copy_present_page':
> > mm/memory.c:800:20: warning: unused variable 'dst_mm' [-Wunused-variable]
> >   struct mm_struct *dst_mm = dst_vma->vm_mm;
> >                     ^~~~~~
> > mm/memory.c: In function 'copy_present_pte':
> > mm/memory.c:889:20: warning: unused variable 'dst_mm' [-Wunused-variable]
> >   struct mm_struct *dst_mm = dst_vma->vm_mm;
> >                     ^~~~~~
> > 
> > Maybe introduced by commit
> > 
> >   7e6cdccef3df ("mm-remove-src-dst-mm-parameter-in-copy_page_range-v2")  
> 
> Yes it is.  The mm pointer is only used by set_pte_at(), while I just noticed
> that some of the archs do not use the mm pointer at all, hence this warning.
> 
> The required change attached; this is quite special that we only referenced the
> mm once in each of the function, so that temp variable can actually be avoided.
> Ideally there should be some way to only define the variable on archs that need
> this mm pointer (e.g., when set_pte_at() or some similar function is called
> multiple times in some function, it should still be helpful to introduce a
> local variable to keep dst_vma->vm_mm).  However I don't know a good way to do
> this...
> 
> Thanks,
> 
> ------------8<------------
> diff --git a/mm/memory.c b/mm/memory.c
> index 8ade87e8600a..d9b16136014c 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -798,7 +798,6 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
>                   pte_t *dst_pte, pte_t *src_pte, unsigned long addr, int *rss,
>                   struct page **prealloc, pte_t pte, struct page *page)
>  {
> -       struct mm_struct *dst_mm = dst_vma->vm_mm;
>         struct mm_struct *src_mm = src_vma->vm_mm;
>         struct page *new_page;
> 
> @@ -874,7 +873,7 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
>         /* All done, just insert the new page copy in the child */
>         pte = mk_pte(new_page, dst_vma->vm_page_prot);
>         pte = maybe_mkwrite(pte_mkdirty(pte), dst_vma);
> -       set_pte_at(dst_mm, addr, dst_pte, pte);
> +       set_pte_at(dst_vma->vm_mm, addr, dst_pte, pte);
>         return 0;
>  }
> 
> @@ -887,7 +886,6 @@ copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>                  pte_t *dst_pte, pte_t *src_pte, unsigned long addr, int *rss,
>                  struct page **prealloc)
>  {
> -       struct mm_struct *dst_mm = dst_vma->vm_mm;
>         struct mm_struct *src_mm = src_vma->vm_mm;
>         unsigned long vm_flags = src_vma->vm_flags;
>         pte_t pte = *src_pte;
> @@ -932,7 +930,7 @@ copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
>         if (!(vm_flags & VM_UFFD_WP))
>                 pte = pte_clear_uffd_wp(pte);
> 
> -       set_pte_at(dst_mm, addr, dst_pte, pte);
> +       set_pte_at(dst_vma->vm_mm, addr, dst_pte, pte);
>         return 0;
>  }

Thanks, I have applied that to linux-next today.

-- 
Cheers,
Stephen Rothwell