[PATCH 2/2] gpu/radeon: use HMM mirror for userptr buffer object.

kbuild test robot lkp at intel.com
Mon Sep 10 18:31:24 UTC 2018


Hi Jérôme,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/jglisse-redhat-com/Getting-rid-of-GUP-and-use-HMM-for-user-ptr-features/20180911-020741
config: x86_64-randconfig-x017-201836 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/radeon/radeon_mn.c:43:20: error: field 'mirror' has incomplete type
     struct hmm_mirror mirror;
                       ^~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_mn_destroy':
   drivers/gpu/drm/radeon/radeon_mn.c:90:2: error: implicit declaration of function 'hmm_mirror_unregister'; did you mean 'drm_dp_aux_unregister'? [-Werror=implicit-function-declaration]
     hmm_mirror_unregister(&rmn->mirror);
     ^~~~~~~~~~~~~~~~~~~~~
     drm_dp_aux_unregister
   In file included from include/linux/firmware.h:6:0,
                    from drivers/gpu/drm/radeon/radeon_mn.c:31:
   drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_mirror_release':
   include/linux/kernel.h:997:32: error: dereferencing pointer to incomplete type 'struct hmm_mirror'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                                   ^~~~~~
   include/linux/compiler.h:335:18: note: in definition of macro '__compiletime_assert'
      int __cond = !(condition);    \
                     ^~~~~~~~~
   include/linux/compiler.h:358:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:997:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^~~~~~~~~~~~~~~~
   include/linux/kernel.h:997:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^~~~~~~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c:103:26: note: in expansion of macro 'container_of'
     struct radeon_mn *rmn = container_of(mirror, struct radeon_mn, mirror);
                             ^~~~~~~~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c: At top level:
   drivers/gpu/drm/radeon/radeon_mn.c:119:24: warning: 'struct hmm_update' declared inside parameter list will not be visible outside of this definition or declaration
              const struct hmm_update *update)
                           ^~~~~~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_sync_cpu_device_pagetables':
   drivers/gpu/drm/radeon/radeon_mn.c:128:14: error: dereferencing pointer to incomplete type 'const struct hmm_update'
     end = update->end - 1;
                 ^~
   drivers/gpu/drm/radeon/radeon_mn.c: At top level:
   drivers/gpu/drm/radeon/radeon_mn.c:183:21: error: variable 'radeon_mirror_ops' has initializer but incomplete type
    static const struct hmm_mirror_ops radeon_mirror_ops = {
                        ^~~~~~~~~~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c:184:3: error: 'const struct hmm_mirror_ops' has no member named 'sync_cpu_device_pagetables'
     .sync_cpu_device_pagetables = &radeon_sync_cpu_device_pagetables,
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c:184:32: warning: excess elements in struct initializer
     .sync_cpu_device_pagetables = &radeon_sync_cpu_device_pagetables,
                                   ^
   drivers/gpu/drm/radeon/radeon_mn.c:184:32: note: (near initialization for 'radeon_mirror_ops')
   drivers/gpu/drm/radeon/radeon_mn.c:185:3: error: 'const struct hmm_mirror_ops' has no member named 'release'
     .release = &radeon_mirror_release,
      ^~~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c:185:13: warning: excess elements in struct initializer
     .release = &radeon_mirror_release,
                ^
   drivers/gpu/drm/radeon/radeon_mn.c:185:13: note: (near initialization for 'radeon_mirror_ops')
   drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_mn_get':
   drivers/gpu/drm/radeon/radeon_mn.c:224:6: error: implicit declaration of function 'hmm_mirror_register'; did you mean 'drm_dp_aux_register'? [-Werror=implicit-function-declaration]
     r = hmm_mirror_register(&new->mirror, mm);
         ^~~~~~~~~~~~~~~~~~~
         drm_dp_aux_register
   drivers/gpu/drm/radeon/radeon_mn.c: In function 'radeon_mn_bo_map':
>> drivers/gpu/drm/radeon/radeon_mn.c:373:43: error: 'HMM_PFN_FLAG_MAX' undeclared (first use in this function); did you mean 'TTM_PL_FLAG_VRAM'?
     static const uint64_t radeon_range_flags[HMM_PFN_FLAG_MAX] = {
                                              ^~~~~~~~~~~~~~~~
                                              TTM_PL_FLAG_VRAM
   drivers/gpu/drm/radeon/radeon_mn.c:373:43: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/radeon/radeon_mn.c:378:44: error: 'HMM_PFN_VALUE_MAX' undeclared (first use in this function); did you mean 'HMM_PFN_FLAG_MAX'?
     static const uint64_t radeon_range_values[HMM_PFN_VALUE_MAX] = {
                                               ^~~~~~~~~~~~~~~~~
                                               HMM_PFN_FLAG_MAX
>> drivers/gpu/drm/radeon/radeon_mn.c:389:19: error: storage size of 'range' isn't known
     struct hmm_range range;
                      ^~~~~
>> drivers/gpu/drm/radeon/radeon_mn.c:421:31: error: 'HMM_PFN_VALID' undeclared (first use in this function); did you mean 'HMM_PFN_VALUE_MAX'?
      range.pfns[i] = range.flags[HMM_PFN_VALID];
                                  ^~~~~~~~~~~~~
                                  HMM_PFN_VALUE_MAX
>> drivers/gpu/drm/radeon/radeon_mn.c:422:40: error: 'HMM_PFN_WRITE' undeclared (first use in this function); did you mean 'HMM_PFN_VALID'?
      range.pfns[i] |= write ? range.flags[HMM_PFN_WRITE] : 0;
                                           ^~~~~~~~~~~~~
                                           HMM_PFN_VALID
>> drivers/gpu/drm/radeon/radeon_mn.c:425:8: error: implicit declaration of function 'hmm_vma_fault'; did you mean 'filemap_fault'? [-Werror=implicit-function-declaration]
     ret = hmm_vma_fault(&range, true);
           ^~~~~~~~~~~~~
           filemap_fault
>> drivers/gpu/drm/radeon/radeon_mn.c:430:23: error: implicit declaration of function 'hmm_pfn_to_page'; did you mean '__pfn_to_page'? [-Werror=implicit-function-declaration]
      struct page *page = hmm_pfn_to_page(&range, range.pfns[i]);
                          ^~~~~~~~~~~~~~~
                          __pfn_to_page
>> drivers/gpu/drm/radeon/radeon_mn.c:446:4: error: implicit declaration of function 'hmm_vma_range_done'; did you mean 'drm_vma_node_size'? [-Werror=implicit-function-declaration]
       hmm_vma_range_done(&range);
       ^~~~~~~~~~~~~~~~~~
       drm_vma_node_size
   drivers/gpu/drm/radeon/radeon_mn.c:389:19: warning: unused variable 'range' [-Wunused-variable]
     struct hmm_range range;
                      ^~~~~
   drivers/gpu/drm/radeon/radeon_mn.c:378:24: warning: unused variable 'radeon_range_values' [-Wunused-variable]
     static const uint64_t radeon_range_values[HMM_PFN_VALUE_MAX] = {
                           ^~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c:373:24: warning: unused variable 'radeon_range_flags' [-Wunused-variable]
     static const uint64_t radeon_range_flags[HMM_PFN_FLAG_MAX] = {
                           ^~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/radeon/radeon_mn.c: At top level:
   drivers/gpu/drm/radeon/radeon_mn.c:183:36: error: storage size of 'radeon_mirror_ops' isn't known
    static const struct hmm_mirror_ops radeon_mirror_ops = {
                                       ^~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +373 drivers/gpu/drm/radeon/radeon_mn.c

   182	
   183	static const struct hmm_mirror_ops radeon_mirror_ops = {
   184		.sync_cpu_device_pagetables = &radeon_sync_cpu_device_pagetables,
 > 185		.release = &radeon_mirror_release,
   186	};
   187	
   188	/**
   189	 * radeon_mn_get - create notifier context
   190	 *
   191	 * @rdev: radeon device pointer
   192	 *
   193	 * Creates a notifier context for current->mm.
   194	 */
   195	static struct radeon_mn *radeon_mn_get(struct radeon_device *rdev)
   196	{
   197		struct mm_struct *mm = current->mm;
   198		struct radeon_mn *rmn, *new;
   199		int r;
   200	
   201		mutex_lock(&rdev->mn_lock);
   202		hash_for_each_possible(rdev->mn_hash, rmn, node, (unsigned long)mm) {
   203			if (rmn->mm == mm) {
   204				mutex_unlock(&rdev->mn_lock);
   205				return rmn;
   206			}
   207		}
   208		mutex_unlock(&rdev->mn_lock);
   209	
   210		new = kzalloc(sizeof(*rmn), GFP_KERNEL);
   211		if (!new) {
   212			return ERR_PTR(-ENOMEM);
   213		}
   214		new->mm = mm;
   215		new->rdev = rdev;
   216		mutex_init(&new->lock);
   217		new->objects = RB_ROOT_CACHED;
   218		new->mirror.ops = &radeon_mirror_ops;
   219	
   220		if (down_write_killable(&mm->mmap_sem)) {
   221			kfree(new);
   222			return ERR_PTR(-EINTR);
   223		}
   224		r = hmm_mirror_register(&new->mirror, mm);
   225		up_write(&mm->mmap_sem);
   226		if (r) {
   227			kfree(new);
   228			return ERR_PTR(r);
   229		}
   230	
   231		mutex_lock(&rdev->mn_lock);
   232		/* Check again in case some other thread raced with us ... */
   233		hash_for_each_possible(rdev->mn_hash, rmn, node, (unsigned long)mm) {
   234			if (rmn->mm == mm) {
   235				mutex_unlock(&rdev->mn_lock);
   236				hmm_mirror_unregister(&new->mirror);
   237				kfree(new);
   238				return rmn;
   239			}
   240		}
   241		hash_add(rdev->mn_hash, &new->node, (unsigned long)mm);
   242		mutex_unlock(&rdev->mn_lock);
   243	
   244		return new;
   245	}
   246	
   247	/**
   248	 * radeon_mn_register - register a BO for notifier updates
   249	 *
   250	 * @bo: radeon buffer object
   251	 * @addr: userptr addr we should monitor
   252	 *
   253	 * Registers an MMU notifier for the given BO at the specified address.
   254	 * Returns 0 on success, -ERRNO if anything goes wrong.
   255	 */
   256	int radeon_mn_register(struct radeon_bo *bo, unsigned long addr)
   257	{
   258		unsigned long end = addr + radeon_bo_size(bo) - 1;
   259		struct radeon_device *rdev = bo->rdev;
   260		struct radeon_mn *rmn;
   261		struct radeon_mn_node *node = NULL;
   262		struct list_head bos;
   263		struct interval_tree_node *it;
   264	
   265		bo->userptr = addr;
   266		bo->pfns = kvmalloc_array(bo->tbo.num_pages, sizeof(uint64_t),
   267					  GFP_KERNEL | __GFP_ZERO);
   268		if (bo->pfns == NULL)
   269			return -ENOMEM;
   270	
   271		rmn = radeon_mn_get(rdev);
   272		if (IS_ERR(rmn)) {
   273			kvfree(bo->pfns);
   274			bo->pfns = NULL;
   275			return PTR_ERR(rmn);
   276		}
   277	
   278		INIT_LIST_HEAD(&bos);
   279	
   280		mutex_lock(&rmn->lock);
   281	
   282		while ((it = interval_tree_iter_first(&rmn->objects, addr, end))) {
   283			kfree(node);
   284			node = container_of(it, struct radeon_mn_node, it);
   285			interval_tree_remove(&node->it, &rmn->objects);
   286			addr = min(it->start, addr);
   287			end = max(it->last, end);
   288			list_splice(&node->bos, &bos);
   289		}
   290	
   291		if (!node) {
   292			node = kmalloc(sizeof(struct radeon_mn_node), GFP_KERNEL);
   293			if (!node) {
   294				mutex_unlock(&rmn->lock);
   295				kvfree(bo->pfns);
   296				bo->pfns = NULL;
   297				return -ENOMEM;
   298			}
   299		}
   300	
   301		bo->mn = rmn;
   302	
   303		node->it.start = addr;
   304		node->it.last = end;
   305		INIT_LIST_HEAD(&node->bos);
   306		list_splice(&bos, &node->bos);
   307		list_add(&bo->mn_list, &node->bos);
   308	
   309		interval_tree_insert(&node->it, &rmn->objects);
   310	
   311		mutex_unlock(&rmn->lock);
   312	
   313		return 0;
   314	}
   315	
   316	/**
   317	 * radeon_mn_unregister - unregister a BO for notifier updates
   318	 *
   319	 * @bo: radeon buffer object
   320	 *
   321	 * Remove any registration of MMU notifier updates from the buffer object.
   322	 */
   323	void radeon_mn_unregister(struct radeon_bo *bo)
   324	{
   325		struct radeon_device *rdev = bo->rdev;
   326		struct radeon_mn *rmn;
   327		struct list_head *head;
   328	
   329		mutex_lock(&rdev->mn_lock);
   330		rmn = bo->mn;
   331		if (rmn == NULL) {
   332			mutex_unlock(&rdev->mn_lock);
   333			return;
   334		}
   335	
   336		mutex_lock(&rmn->lock);
   337		/* save the next list entry for later */
   338		head = bo->mn_list.next;
   339	
   340		bo->mn = NULL;
   341		list_del(&bo->mn_list);
   342	
   343		if (list_empty(head)) {
   344			struct radeon_mn_node *node;
   345			node = container_of(head, struct radeon_mn_node, bos);
   346			interval_tree_remove(&node->it, &rmn->objects);
   347			kfree(node);
   348		}
   349	
   350		mutex_unlock(&rmn->lock);
   351		mutex_unlock(&rdev->mn_lock);
   352	
   353		kvfree(bo->pfns);
   354		bo->pfns = NULL;
   355	}
   356	
   357	/**
   358	 * radeon_mn_bo_map - map range of virtual address as buffer object
   359	 *
   360	 * @bo: radeon buffer object
   361	 * @ttm: ttm_tt object in which holds mirroring result
   362	 * @write: can GPU write to the range ?
   363	 * Returns: 0 on success, error code otherwise
   364	 *
   365	 * Use HMM to mirror a range of virtual address as a buffer object mapped into
   366	 * GPU address space (thus allowing transparent GPU access to this range). It
   367	 * does not pin pages for range but rely on HMM and underlying synchronizations
   368	 * to make sure that both CPU and GPU points to same physical memory for the
   369	 * range.
   370	 */
   371	int radeon_mn_bo_map(struct radeon_bo *bo, struct ttm_dma_tt *dma, bool write)
   372	{
 > 373		static const uint64_t radeon_range_flags[HMM_PFN_FLAG_MAX] = {
   374			(1 << 0), /* HMM_PFN_VALID */
   375			(1 << 1), /* HMM_PFN_WRITE */
   376			0 /* HMM_PFN_DEVICE_PRIVATE */
   377		};
 > 378		static const uint64_t radeon_range_values[HMM_PFN_VALUE_MAX] = {
   379			0xfffffffffffffffeUL, /* HMM_PFN_ERROR */
   380			0, /* HMM_PFN_NONE */
   381			0xfffffffffffffffcUL /* HMM_PFN_SPECIAL */
   382		};
   383	
   384		unsigned long i, npages = bo->tbo.num_pages;
   385		enum dma_data_direction direction = write ?
   386			DMA_BIDIRECTIONAL : DMA_TO_DEVICE;
   387		struct radeon_device *rdev = bo->rdev;
   388		struct ttm_tt *ttm = &dma->ttm;
 > 389		struct hmm_range range;
   390		struct radeon_mn *rmn;
   391		int ret;
   392	
   393		/*
   394		 * FIXME This whole protection shouldn't be needed as we should only
   395		 * reach that code with a valid reserved bo that can not under go a
   396		 * concurrent radeon_mn_unregister().
   397		 */
   398		mutex_lock(&rdev->mn_lock);
   399		if (bo->mn == NULL) {
   400			mutex_unlock(&rdev->mn_lock);
   401			return -EINVAL;
   402		}
   403		rmn = bo->mn;
   404		mutex_unlock(&rdev->mn_lock);
   405	
   406		range.pfn_shift = 12;
   407		range.pfns = bo->pfns;
   408		range.start = bo->userptr;
   409		range.flags = radeon_range_flags;
   410		range.values = radeon_range_values;
   411		range.end = bo->userptr + radeon_bo_size(bo);
   412	
   413		range.vma = find_vma(rmn->mm, bo->userptr);
   414		if (!range.vma || range.vma->vm_file || range.vma->vm_end < range.end)
   415			return -EPERM;
   416	
   417		memset(ttm->pages, 0, sizeof(void*) * npages);
   418	
   419	again:
   420		for (i = 0; i < npages; ++i) {
 > 421			range.pfns[i] = range.flags[HMM_PFN_VALID];
 > 422			range.pfns[i] |= write ? range.flags[HMM_PFN_WRITE] : 0;
   423		}
   424	
 > 425		ret = hmm_vma_fault(&range, true);
   426		if (ret)
   427			goto err_unmap;
   428	
   429		for (i = 0; i < npages; ++i) {
 > 430			struct page *page = hmm_pfn_to_page(&range, range.pfns[i]);
   431	
   432			if (page == NULL)
   433				goto again;
   434	
   435			if (ttm->pages[i] == page)
   436				continue;
   437	
   438			if (ttm->pages[i])
   439				dma_unmap_page(rdev->dev, dma->dma_address[i],
   440					       PAGE_SIZE, direction);
   441			ttm->pages[i] = page;
   442	
   443			dma->dma_address[i] = dma_map_page(rdev->dev, page, 0,
   444							   PAGE_SIZE, direction);
   445			if (dma_mapping_error(rdev->dev, dma->dma_address[i])) {
 > 446				hmm_vma_range_done(&range);
   447				ttm->pages[i] = NULL;
   448				ret = -ENOMEM;
   449				goto err_unmap;
   450			}
   451		}
   452	
   453		/*
   454		 * Taking rmn->lock is not necessary here as we are protected from any
   455		 * concurrent invalidation through ttm object reservation. Involved
   456		 * functions: radeon_sync_cpu_device_pagetables()
   457		 *            radeon_bo_list_validate()
   458		 *            radeon_gem_userptr_ioctl()
   459		 */
   460		if (!hmm_vma_range_done(&range))
   461			goto again;
   462	
   463		return 0;
   464	
   465	err_unmap:
   466		radeon_mn_bo_unmap(bo, dma, write);
   467		return ret;
   468	}
   469	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 30009 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180911/c1756150/attachment-0001.gz>


More information about the dri-devel mailing list