Hi all,
On Tue, 6 Oct 2020 16:01:38 -0400 Peter Xu <peterx@xxxxxxxxxx> 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
Attachment:
pgpm9ThPANMqM.pgp
Description: OpenPGP digital signature
