Re: [PATCH v5 18/26] hugetlb/userfaultfd: Take care of UFFDIO_COPY_MODE_WP

[kernel test robot]

Hi Peter,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kselftest/next]
[also build test ERROR on linus/master v5.14-rc2 next-20210720]
[cannot apply to hnaz-linux-mm/master asm-generic/master arm64/for-next/core linux/master tip/x86/core]
[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]

url:    https://github.com/0day-ci/linux/commits/Peter-Xu/userfaultfd-wp-Support-shmem-and-hugetlbfs/20210716-041947
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
config: s390-randconfig-r023-20210716 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 5d5b08761f944d5b9822d582378333cc4b36a0a7)
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
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/2ad11be7ccbb4fada15c5ec48a35630d450fc9ca
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Peter-Xu/userfaultfd-wp-Support-shmem-and-hugetlbfs/20210716-041947
        git checkout 2ad11be7ccbb4fada15c5ec48a35630d450fc9ca
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>

All errors (new ones prefixed by >>):

   In file included from mm/hugetlb.c:19:
   In file included from include/linux/memblock.h:14:
   In file included from arch/s390/include/asm/dma.h:5:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:36:59: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
                                                             ^
   include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16'
   #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
                                                        ^
   In file included from mm/hugetlb.c:19:
   In file included from include/linux/memblock.h:14:
   In file included from arch/s390/include/asm/dma.h:5:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32'
   #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
                                                        ^
   In file included from mm/hugetlb.c:19:
   In file included from include/linux/memblock.h:14:
   In file included from arch/s390/include/asm/dma.h:5:
   In file included from arch/s390/include/asm/io.h:75:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   mm/hugetlb.c:5063:29: error: implicit declaration of function 'huge_pte_uffd_wp' [-Werror,-Wimplicit-function-declaration]
           if (userfaultfd_wp(vma) && huge_pte_uffd_wp(huge_ptep_get(ptep)) &&
                                      ^
>> mm/hugetlb.c:5301:14: error: implicit declaration of function 'huge_pte_mkuffd_wp' [-Werror,-Wimplicit-function-declaration]
                   _dst_pte = huge_pte_mkuffd_wp(_dst_pte);
                              ^
   mm/hugetlb.c:5301:14: note: did you mean 'pte_mkuffd_wp'?
   include/asm-generic/pgtable_uffd.h:18:30: note: 'pte_mkuffd_wp' declared here
   static __always_inline pte_t pte_mkuffd_wp(pte_t pte)
                                ^
>> mm/hugetlb.c:5301:12: error: assigning to 'pte_t' from incompatible type 'int'
                   _dst_pte = huge_pte_mkuffd_wp(_dst_pte);
                            ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   12 warnings and 3 errors generated.


vim +/huge_pte_mkuffd_wp +5301 mm/hugetlb.c

  5132	
  5133	#ifdef CONFIG_USERFAULTFD
  5134	/*
  5135	 * Used by userfaultfd UFFDIO_COPY.  Based on mcopy_atomic_pte with
  5136	 * modifications for huge pages.
  5137	 */
  5138	int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm,
  5139				    pte_t *dst_pte,
  5140				    struct vm_area_struct *dst_vma,
  5141				    unsigned long dst_addr,
  5142				    unsigned long src_addr,
  5143				    enum mcopy_atomic_mode mode,
  5144				    struct page **pagep,
  5145				    bool wp_copy)
  5146	{
  5147		bool is_continue = (mode == MCOPY_ATOMIC_CONTINUE);
  5148		struct hstate *h = hstate_vma(dst_vma);
  5149		struct address_space *mapping = dst_vma->vm_file->f_mapping;
  5150		pgoff_t idx = vma_hugecache_offset(h, dst_vma, dst_addr);
  5151		unsigned long size;
  5152		int vm_shared = dst_vma->vm_flags & VM_SHARED;
  5153		pte_t _dst_pte;
  5154		spinlock_t *ptl;
  5155		int ret = -ENOMEM;
  5156		struct page *page;
  5157		int writable;
  5158	
  5159		if (is_continue) {
  5160			ret = -EFAULT;
  5161			page = find_lock_page(mapping, idx);
  5162			if (!page)
  5163				goto out;
  5164		} else if (!*pagep) {
  5165			/* If a page already exists, then it's UFFDIO_COPY for
  5166			 * a non-missing case. Return -EEXIST.
  5167			 */
  5168			if (vm_shared &&
  5169			    hugetlbfs_pagecache_present(h, dst_vma, dst_addr)) {
  5170				ret = -EEXIST;
  5171				goto out;
  5172			}
  5173	
  5174			page = alloc_huge_page(dst_vma, dst_addr, 0);
  5175			if (IS_ERR(page)) {
  5176				ret = -ENOMEM;
  5177				goto out;
  5178			}
  5179	
  5180			ret = copy_huge_page_from_user(page,
  5181							(const void __user *) src_addr,
  5182							pages_per_huge_page(h), false);
  5183	
  5184			/* fallback to copy_from_user outside mmap_lock */
  5185			if (unlikely(ret)) {
  5186				ret = -ENOENT;
  5187				/* Free the allocated page which may have
  5188				 * consumed a reservation.
  5189				 */
  5190				restore_reserve_on_error(h, dst_vma, dst_addr, page);
  5191				put_page(page);
  5192	
  5193				/* Allocate a temporary page to hold the copied
  5194				 * contents.
  5195				 */
  5196				page = alloc_huge_page_vma(h, dst_vma, dst_addr);
  5197				if (!page) {
  5198					ret = -ENOMEM;
  5199					goto out;
  5200				}
  5201				*pagep = page;
  5202				/* Set the outparam pagep and return to the caller to
  5203				 * copy the contents outside the lock. Don't free the
  5204				 * page.
  5205				 */
  5206				goto out;
  5207			}
  5208		} else {
  5209			if (vm_shared &&
  5210			    hugetlbfs_pagecache_present(h, dst_vma, dst_addr)) {
  5211				put_page(*pagep);
  5212				ret = -EEXIST;
  5213				*pagep = NULL;
  5214				goto out;
  5215			}
  5216	
  5217			page = alloc_huge_page(dst_vma, dst_addr, 0);
  5218			if (IS_ERR(page)) {
  5219				ret = -ENOMEM;
  5220				*pagep = NULL;
  5221				goto out;
  5222			}
  5223			copy_huge_page(page, *pagep);
  5224			put_page(*pagep);
  5225			*pagep = NULL;
  5226		}
  5227	
  5228		/*
  5229		 * The memory barrier inside __SetPageUptodate makes sure that
  5230		 * preceding stores to the page contents become visible before
  5231		 * the set_pte_at() write.
  5232		 */
  5233		__SetPageUptodate(page);
  5234	
  5235		/* Add shared, newly allocated pages to the page cache. */
  5236		if (vm_shared && !is_continue) {
  5237			size = i_size_read(mapping->host) >> huge_page_shift(h);
  5238			ret = -EFAULT;
  5239			if (idx >= size)
  5240				goto out_release_nounlock;
  5241	
  5242			/*
  5243			 * Serialization between remove_inode_hugepages() and
  5244			 * huge_add_to_page_cache() below happens through the
  5245			 * hugetlb_fault_mutex_table that here must be hold by
  5246			 * the caller.
  5247			 */
  5248			ret = huge_add_to_page_cache(page, mapping, idx);
  5249			if (ret)
  5250				goto out_release_nounlock;
  5251		}
  5252	
  5253		ptl = huge_pte_lockptr(h, dst_mm, dst_pte);
  5254		spin_lock(ptl);
  5255	
  5256		/*
  5257		 * Recheck the i_size after holding PT lock to make sure not
  5258		 * to leave any page mapped (as page_mapped()) beyond the end
  5259		 * of the i_size (remove_inode_hugepages() is strict about
  5260		 * enforcing that). If we bail out here, we'll also leave a
  5261		 * page in the radix tree in the vm_shared case beyond the end
  5262		 * of the i_size, but remove_inode_hugepages() will take care
  5263		 * of it as soon as we drop the hugetlb_fault_mutex_table.
  5264		 */
  5265		size = i_size_read(mapping->host) >> huge_page_shift(h);
  5266		ret = -EFAULT;
  5267		if (idx >= size)
  5268			goto out_release_unlock;
  5269	
  5270		ret = -EEXIST;
  5271		if (!huge_pte_none(huge_ptep_get(dst_pte)))
  5272			goto out_release_unlock;
  5273	
  5274		if (vm_shared) {
  5275			page_dup_rmap(page, true);
  5276		} else {
  5277			ClearHPageRestoreReserve(page);
  5278			hugepage_add_new_anon_rmap(page, dst_vma, dst_addr);
  5279		}
  5280	
  5281		/*
  5282		 * For either: (1) CONTINUE on a non-shared VMA, or (2) UFFDIO_COPY
  5283		 * with wp flag set, don't set pte write bit.
  5284		 */
  5285		if (wp_copy || (is_continue && !vm_shared))
  5286			writable = 0;
  5287		else
  5288			writable = dst_vma->vm_flags & VM_WRITE;
  5289	
  5290		_dst_pte = make_huge_pte(dst_vma, page, writable);
  5291		/*
  5292		 * Always mark UFFDIO_COPY page dirty; note that this may not be
  5293		 * extremely important for hugetlbfs for now since swapping is not
  5294		 * supported, but we should still be clear in that this page cannot be
  5295		 * thrown away at will, even if write bit not set.
  5296		 */
  5297		_dst_pte = huge_pte_mkdirty(_dst_pte);
  5298		_dst_pte = pte_mkyoung(_dst_pte);
  5299	
  5300		if (wp_copy)
> 5301			_dst_pte = huge_pte_mkuffd_wp(_dst_pte);
  5302	
  5303		set_huge_pte_at(dst_mm, dst_addr, dst_pte, _dst_pte);
  5304	
  5305		(void)huge_ptep_set_access_flags(dst_vma, dst_addr, dst_pte, _dst_pte,
  5306						dst_vma->vm_flags & VM_WRITE);
  5307		hugetlb_count_add(pages_per_huge_page(h), dst_mm);
  5308	
  5309		/* No need to invalidate - it was non-present before */
  5310		update_mmu_cache(dst_vma, dst_addr, dst_pte);
  5311	
  5312		spin_unlock(ptl);
  5313		if (!is_continue)
  5314			SetHPageMigratable(page);
  5315		if (vm_shared || is_continue)
  5316			unlock_page(page);
  5317		ret = 0;
  5318	out:
  5319		return ret;
  5320	out_release_unlock:
  5321		spin_unlock(ptl);
  5322		if (vm_shared || is_continue)
  5323			unlock_page(page);
  5324	out_release_nounlock:
  5325		restore_reserve_on_error(h, dst_vma, dst_addr, page);
  5326		put_page(page);
  5327		goto out;
  5328	}
  5329	#endif /* CONFIG_USERFAULTFD */
  5330	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip