Re: [PATCH v3 4/5] mm: userfaultfd: don't separate addr + len arguments

From: Axel Rasmussen
Date: Wed Mar 08 2023 - 13:48:56 EST

Next message: Bjorn Helgaas: "Re: [PATCH] PCI: imx6: install the fault handler only if we are really running on a compatible device"
Previous message: Bart Van Assche: "Re: [RFC PATCH v1 2/4] ufs: mcq: Add supporting functions for mcq abort"
In reply to: kernel test robot: "Re: [PATCH v3 4/5] mm: userfaultfd: don't separate addr + len arguments"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Mar 8, 2023 at 1:52 AM kernel test robot <lkp@xxxxxxxxx> wrote:
>
> Hi Axel,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v6.3-rc1]
> [cannot apply to akpm-mm/mm-everything next-20230308]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Axel-Rasmussen/mm-userfaultfd-rename-functions-for-clarity-consistency/20230307-065203
> patch link: https://lore.kernel.org/r/20230306225024.264858-5-axelrasmussen%40google.com
> patch subject: [PATCH v3 4/5] mm: userfaultfd: don't separate addr + len arguments
> config: x86_64-randconfig-a011-20230306 (https://download.01.org/0day-ci/archive/20230308/202303081703.nwxAgIVH-lkp@xxxxxxxxx/config)
> compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/cee642b93be3ae01c7cc737c0176cbc16074a25a
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Axel-Rasmussen/mm-userfaultfd-rename-functions-for-clarity-consistency/20230307-065203
> git checkout cee642b93be3ae01c7cc737c0176cbc16074a25a
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
> | Link: https://lore.kernel.org/oe-kbuild-all/202303081703.nwxAgIVH-lkp@xxxxxxxxx/
>
> All errors (new ones prefixed by >>):
>
> >> mm/userfaultfd.c:577:52: error: passing 'const struct uffdio_range *' to parameter of incompatible type 'struct uffdio_range'
> return mfill_atomic_hugetlb(dst_vma, src_start, dst, flags);
> ^~~
> mm/userfaultfd.c:463:29: note: passing argument to parameter 'dst' here
> struct uffdio_range dst,
> ^
> 1 error generated.

Whoops. :) I admittedly didn't test with !CONFIG_HUGETLB_PAGE.

The next version of this series will drop this patch as per discussion
though, so the issue is moot.

>
>
> vim +577 mm/userfaultfd.c
>
> 508
> 509 static __always_inline ssize_t mfill_atomic(struct mm_struct *dst_mm,
> 510 unsigned long src_start,
> 511 const struct uffdio_range *dst,
> 512 atomic_t *mmap_changing,
> 513 uffd_flags_t flags)
> 514 {
> 515 struct vm_area_struct *dst_vma;
> 516 ssize_t err;
> 517 pmd_t *dst_pmd;
> 518 unsigned long src_addr, dst_addr;
> 519 long copied;
> 520 struct page *page;
> 521
> 522 /*
> 523 * Sanitize the command parameters:
> 524 */
> 525 BUG_ON(dst->start & ~PAGE_MASK);
> 526 BUG_ON(dst->len & ~PAGE_MASK);
> 527
> 528 /* Does the address range wrap, or is the span zero-sized? */
> 529 BUG_ON(src_start + dst->len <= src_start);
> 530 BUG_ON(dst->start + dst->len <= dst->start);
> 531
> 532 src_addr = src_start;
> 533 dst_addr = dst->start;
> 534 copied = 0;
> 535 page = NULL;
> 536 retry:
> 537 mmap_read_lock(dst_mm);
> 538
> 539 /*
> 540 * If memory mappings are changing because of non-cooperative
> 541 * operation (e.g. mremap) running in parallel, bail out and
> 542 * request the user to retry later
> 543 */
> 544 err = -EAGAIN;
> 545 if (mmap_changing && atomic_read(mmap_changing))
> 546 goto out_unlock;
> 547
> 548 /*
> 549 * Make sure the vma is not shared, that the dst range is
> 550 * both valid and fully within a single existing vma.
> 551 */
> 552 err = -ENOENT;
> 553 dst_vma = find_dst_vma(dst_mm, dst);
> 554 if (!dst_vma)
> 555 goto out_unlock;
> 556
> 557 err = -EINVAL;
> 558 /*
> 559 * shmem_zero_setup is invoked in mmap for MAP_ANONYMOUS|MAP_SHARED but
> 560 * it will overwrite vm_ops, so vma_is_anonymous must return false.
> 561 */
> 562 if (WARN_ON_ONCE(vma_is_anonymous(dst_vma) &&
> 563 dst_vma->vm_flags & VM_SHARED))
> 564 goto out_unlock;
> 565
> 566 /*
> 567 * validate 'mode' now that we know the dst_vma: don't allow
> 568 * a wrprotect copy if the userfaultfd didn't register as WP.
> 569 */
> 570 if ((flags & MFILL_ATOMIC_WP) && !(dst_vma->vm_flags & VM_UFFD_WP))
> 571 goto out_unlock;
> 572
> 573 /*
> 574 * If this is a HUGETLB vma, pass off to appropriate routine
> 575 */
> 576 if (is_vm_hugetlb_page(dst_vma))
> > 577 return mfill_atomic_hugetlb(dst_vma, src_start, dst, flags);
> 578
> 579 if (!vma_is_anonymous(dst_vma) && !vma_is_shmem(dst_vma))
> 580 goto out_unlock;
> 581 if (!vma_is_shmem(dst_vma) &&
> 582 (flags & MFILL_ATOMIC_MODE_MASK) == MFILL_ATOMIC_CONTINUE)
> 583 goto out_unlock;
> 584
> 585 /*
> 586 * Ensure the dst_vma has a anon_vma or this page
> 587 * would get a NULL anon_vma when moved in the
> 588 * dst_vma.
> 589 */
> 590 err = -ENOMEM;
> 591 if (!(dst_vma->vm_flags & VM_SHARED) &&
> 592 unlikely(anon_vma_prepare(dst_vma)))
> 593 goto out_unlock;
> 594
> 595 while (src_addr < src_start + dst->len) {
> 596 pmd_t dst_pmdval;
> 597
> 598 BUG_ON(dst_addr >= dst->start + dst->len);
> 599
> 600 dst_pmd = mm_alloc_pmd(dst_mm, dst_addr);
> 601 if (unlikely(!dst_pmd)) {
> 602 err = -ENOMEM;
> 603 break;
> 604 }
> 605
> 606 dst_pmdval = pmdp_get_lockless(dst_pmd);
> 607 /*
> 608 * If the dst_pmd is mapped as THP don't
> 609 * override it and just be strict.
> 610 */
> 611 if (unlikely(pmd_trans_huge(dst_pmdval))) {
> 612 err = -EEXIST;
> 613 break;
> 614 }
> 615 if (unlikely(pmd_none(dst_pmdval)) &&
> 616 unlikely(__pte_alloc(dst_mm, dst_pmd))) {
> 617 err = -ENOMEM;
> 618 break;
> 619 }
> 620 /* If an huge pmd materialized from under us fail */
> 621 if (unlikely(pmd_trans_huge(*dst_pmd))) {
> 622 err = -EFAULT;
> 623 break;
> 624 }
> 625
> 626 BUG_ON(pmd_none(*dst_pmd));
> 627 BUG_ON(pmd_trans_huge(*dst_pmd));
> 628
> 629 err = mfill_atomic_pte(dst_pmd, dst_vma, dst_addr,
> 630 src_addr, &page, flags);
> 631 cond_resched();
> 632
> 633 if (unlikely(err == -ENOENT)) {
> 634 void *page_kaddr;
> 635
> 636 mmap_read_unlock(dst_mm);
> 637 BUG_ON(!page);
> 638
> 639 page_kaddr = kmap_local_page(page);
> 640 err = copy_from_user(page_kaddr,
> 641 (const void __user *) src_addr,
> 642 PAGE_SIZE);
> 643 kunmap_local(page_kaddr);
> 644 if (unlikely(err)) {
> 645 err = -EFAULT;
> 646 goto out;
> 647 }
> 648 flush_dcache_page(page);
> 649 goto retry;
> 650 } else
> 651 BUG_ON(page);
> 652
> 653 if (!err) {
> 654 dst_addr += PAGE_SIZE;
> 655 src_addr += PAGE_SIZE;
> 656 copied += PAGE_SIZE;
> 657
> 658 if (fatal_signal_pending(current))
> 659 err = -EINTR;
> 660 }
> 661 if (err)
> 662 break;
> 663 }
> 664
> 665 out_unlock:
> 666 mmap_read_unlock(dst_mm);
> 667 out:
> 668 if (page)
> 669 put_page(page);
> 670 BUG_ON(copied < 0);
> 671 BUG_ON(err > 0);
> 672 BUG_ON(!copied && !err);
> 673 return copied ? copied : err;
> 674 }
> 675
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests

Next message: Bjorn Helgaas: "Re: [PATCH] PCI: imx6: install the fault handler only if we are really running on a compatible device"
Previous message: Bart Van Assche: "Re: [RFC PATCH v1 2/4] ufs: mcq: Add supporting functions for mcq abort"
In reply to: kernel test robot: "Re: [PATCH v3 4/5] mm: userfaultfd: don't separate addr + len arguments"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]