[PATCH 4/5] drm/radeon: add userptr flag to register MMU notifier v3

Jerome Glisse j.glisse at gmail.com
Wed Aug 6 08:16:30 PDT 2014


On Tue, Aug 05, 2014 at 06:05:31PM +0200, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
> 
> Whenever userspace mapping related to our userptr change
> we wait for it to become idle and unmap it from GTT.
> 
> v2: rebased, fix mutex unlock in error path
> v3: improve commit message

Why in hell do you not register the mmu_notifier on first userptr bo ?

> 
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  drivers/gpu/drm/Kconfig                |   1 +
>  drivers/gpu/drm/radeon/Makefile        |   2 +-
>  drivers/gpu/drm/radeon/radeon.h        |  12 ++
>  drivers/gpu/drm/radeon/radeon_device.c |   2 +
>  drivers/gpu/drm/radeon/radeon_gem.c    |   9 +-
>  drivers/gpu/drm/radeon/radeon_mn.c     | 272 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/radeon/radeon_object.c |   1 +
>  include/uapi/drm/radeon_drm.h          |   1 +
>  8 files changed, 298 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/radeon/radeon_mn.c
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 9b2eedc..2745284 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -115,6 +115,7 @@ config DRM_RADEON
>  	select HWMON
>  	select BACKLIGHT_CLASS_DEVICE
>  	select INTERVAL_TREE
> +	select MMU_NOTIFIER
>  	help
>  	  Choose this option if you have an ATI Radeon graphics card.  There
>  	  are both PCI and AGP versions.  You don't need to choose this to
> diff --git a/drivers/gpu/drm/radeon/Makefile b/drivers/gpu/drm/radeon/Makefile
> index 0013ad0..c7fa1ae 100644
> --- a/drivers/gpu/drm/radeon/Makefile
> +++ b/drivers/gpu/drm/radeon/Makefile
> @@ -80,7 +80,7 @@ radeon-y += radeon_device.o radeon_asic.o radeon_kms.o \
>  	r600_dpm.o rs780_dpm.o rv6xx_dpm.o rv770_dpm.o rv730_dpm.o rv740_dpm.o \
>  	rv770_smc.o cypress_dpm.o btc_dpm.o sumo_dpm.o sumo_smc.o trinity_dpm.o \
>  	trinity_smc.o ni_dpm.o si_smc.o si_dpm.o kv_smc.o kv_dpm.o ci_smc.o \
> -	ci_dpm.o dce6_afmt.o radeon_vm.o radeon_ucode.o radeon_ib.o
> +	ci_dpm.o dce6_afmt.o radeon_vm.o radeon_ucode.o radeon_ib.o radeon_mn.o
>  
>  # add async DMA block
>  radeon-y += \
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 3c6999e..511191f 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -65,6 +65,7 @@
>  #include <linux/list.h>
>  #include <linux/kref.h>
>  #include <linux/interval_tree.h>
> +#include <linux/hashtable.h>
>  
>  #include <ttm/ttm_bo_api.h>
>  #include <ttm/ttm_bo_driver.h>
> @@ -487,6 +488,9 @@ struct radeon_bo {
>  
>  	struct ttm_bo_kmap_obj		dma_buf_vmap;
>  	pid_t				pid;
> +
> +	struct radeon_mn		*mn;
> +	struct interval_tree_node	mn_it;
>  };
>  #define gem_to_radeon_bo(gobj) container_of((gobj), struct radeon_bo, gem_base)
>  
> @@ -1725,6 +1729,11 @@ void radeon_test_ring_sync(struct radeon_device *rdev,
>  			   struct radeon_ring *cpB);
>  void radeon_test_syncing(struct radeon_device *rdev);
>  
> +/*
> + * MMU Notifier
> + */
> +int radeon_mn_register(struct radeon_bo *bo, unsigned long addr);
> +void radeon_mn_unregister(struct radeon_bo *bo);
>  
>  /*
>   * Debugfs
> @@ -2372,6 +2381,9 @@ struct radeon_device {
>  	/* tracking pinned memory */
>  	u64 vram_pin_size;
>  	u64 gart_pin_size;
> +
> +	struct mutex	mn_lock;
> +	DECLARE_HASHTABLE(mn_hash, 7);
>  };
>  
>  bool radeon_is_px(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index c8ea050..c58f84f 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1270,6 +1270,8 @@ int radeon_device_init(struct radeon_device *rdev,
>  	init_rwsem(&rdev->pm.mclk_lock);
>  	init_rwsem(&rdev->exclusive_lock);
>  	init_waitqueue_head(&rdev->irq.vblank_queue);
> +	mutex_init(&rdev->mn_lock);
> +	hash_init(rdev->mn_hash);
>  	r = radeon_gem_init(rdev);
>  	if (r)
>  		return r;
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 4506560..2a6fbf1 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -291,7 +291,8 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
>  
>  	/* reject unknown flag values */
>  	if (args->flags & ~(RADEON_GEM_USERPTR_READONLY |
> -	    RADEON_GEM_USERPTR_ANONONLY | RADEON_GEM_USERPTR_VALIDATE))
> +	    RADEON_GEM_USERPTR_ANONONLY | RADEON_GEM_USERPTR_VALIDATE |
> +	    RADEON_GEM_USERPTR_REGISTER))
>  		return -EINVAL;
>  
>  	/* readonly pages not tested on older hardware */
> @@ -312,6 +313,12 @@ int radeon_gem_userptr_ioctl(struct drm_device *dev, void *data,
>  	if (r)
>  		goto release_object;
>  
> +	if (args->flags & RADEON_GEM_USERPTR_REGISTER) {
> +		r = radeon_mn_register(bo, args->addr);
> +		if (r)
> +			goto release_object;
> +	}
> +
>  	if (args->flags & RADEON_GEM_USERPTR_VALIDATE) {
>  		down_read(&current->mm->mmap_sem);
>  		r = radeon_bo_reserve(bo, true);
> diff --git a/drivers/gpu/drm/radeon/radeon_mn.c b/drivers/gpu/drm/radeon/radeon_mn.c
> new file mode 100644
> index 0000000..0157bc2
> --- /dev/null
> +++ b/drivers/gpu/drm/radeon/radeon_mn.c
> @@ -0,0 +1,272 @@
> +/*
> + * Copyright 2014 Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
> + * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
> + * USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + */
> +/*
> + * Authors:
> + *    Christian König <christian.koenig at amd.com>
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/module.h>
> +#include <linux/mmu_notifier.h>
> +#include <drm/drmP.h>
> +#include <drm/drm.h>
> +
> +#include "radeon.h"
> +
> +struct radeon_mn {
> +	/* constant after initialisation */
> +	struct radeon_device	*rdev;
> +	struct mm_struct	*mm;
> +	struct mmu_notifier	mn;
> +
> +	/* only used on destruction */
> +	struct work_struct	work;
> +
> +	/* protected by rdev->mn_lock */
> +	struct hlist_node	node;
> +
> +	/* objects protected by lock */
> +	struct mutex		lock;
> +	struct rb_root		objects;
> +};
> +
> +/**
> + * radeon_mn_destroy - destroy the rmn
> + *
> + * @work: previously sheduled work item
> + *
> + * Lazy destroys the notifier from a work item
> + */
> +static void radeon_mn_destroy(struct work_struct *work)
> +{
> +	struct radeon_mn *rmn = container_of(work, struct radeon_mn, work);
> +	struct radeon_device *rdev = rmn->rdev;
> +	struct radeon_bo *bo, *next;
> +
> +	mutex_lock(&rdev->mn_lock);
> +	mutex_lock(&rmn->lock);
> +	hash_del(&rmn->node);
> +	rbtree_postorder_for_each_entry_safe(bo, next, &rmn->objects, mn_it.rb) {
> +		interval_tree_remove(&bo->mn_it, &rmn->objects);
> +		bo->mn = NULL;
> +	}
> +	mutex_unlock(&rmn->lock);
> +	mutex_unlock(&rdev->mn_lock);
> +	mmu_notifier_unregister(&rmn->mn, rmn->mm);
> +	kfree(rmn);
> +}
> +
> +/**
> + * radeon_mn_release - callback to notify about mm destruction
> + *
> + * @mn: our notifier
> + * @mn: the mm this callback is about
> + *
> + * Shedule a work item to lazy destroy our notifier.
> + */
> +static void radeon_mn_release(struct mmu_notifier *mn,
> +			      struct mm_struct *mm)
> +{
> +	struct radeon_mn *rmn = container_of(mn, struct radeon_mn, mn);
> +	INIT_WORK(&rmn->work, radeon_mn_destroy);
> +	schedule_work(&rmn->work);
> +}
> +
> +/**
> + * radeon_mn_invalidate_range_start - callback to notify about mm change
> + *
> + * @mn: our notifier
> + * @mn: the mm this callback is about
> + * @start: start of updated range
> + * @end: end of updated range
> + *
> + * We block for all BOs between start and end to be idle and
> + * unmap them by move them into system domain again.
> + */
> +static void radeon_mn_invalidate_range_start(struct mmu_notifier *mn,
> +					     struct mm_struct *mm,
> +					     unsigned long start,
> +					     unsigned long end)
> +{
> +	struct radeon_mn *rmn = container_of(mn, struct radeon_mn, mn);
> +	struct interval_tree_node *it;
> +
> +	/* notification is exclusive, but interval is inclusive */
> +	end -= 1;
> +
> +	mutex_lock(&rmn->lock);
> +
> +	it = interval_tree_iter_first(&rmn->objects, start, end);
> +	while (it) {
> +		struct radeon_bo *bo;
> +		int r;
> +
> +		bo = container_of(it, struct radeon_bo, mn_it);
> +		it = interval_tree_iter_next(it, start, end);
> +
> +		r = radeon_bo_reserve(bo, true);
> +		if (r) {
> +			DRM_ERROR("(%d) failed to reserve user bo\n", r);
> +			continue;
> +		}
> +
> +		if (bo->tbo.sync_obj) {
> +			r = radeon_fence_wait(bo->tbo.sync_obj, false);
> +			if (r)
> +				DRM_ERROR("(%d) failed to wait for user bo\n", r);
> +		}
> +
> +		radeon_ttm_placement_from_domain(bo, RADEON_GEM_DOMAIN_CPU);
> +		r = ttm_bo_validate(&bo->tbo, &bo->placement, false, false);
> +		if (r)
> +			DRM_ERROR("(%d) failed to validate user bo\n", r);
> +
> +		radeon_bo_unreserve(bo);
> +	}
> +	
> +	mutex_unlock(&rmn->lock);
> +}
> +
> +static const struct mmu_notifier_ops radeon_mn_ops = {
> +	.release = radeon_mn_release,
> +	.invalidate_range_start = radeon_mn_invalidate_range_start,
> +};
> +
> +/**
> + * radeon_mn_get - create notifier context
> + *
> + * @rdev: radeon device pointer
> + *
> + * Creates a notifier context for current->mm.
> + */
> +static struct radeon_mn *radeon_mn_get(struct radeon_device *rdev)
> +{
> +	struct mm_struct *mm = current->mm;
> +	struct radeon_mn *rmn;
> +	int r;
> +
> +	down_write(&mm->mmap_sem);
> +	mutex_lock(&rdev->mn_lock);
> +
> +	hash_for_each_possible(rdev->mn_hash, rmn, node, (unsigned long)mm)
> +		if (rmn->mm == mm)
> +			goto release_locks;
> +
> +	rmn = kzalloc(sizeof(*rmn), GFP_KERNEL);
> +	if (!rmn) {
> +		rmn = ERR_PTR(-ENOMEM);
> +		goto release_locks;
> +	}
> +
> +	rmn->rdev = rdev;
> +	rmn->mm = mm;
> +	rmn->mn.ops = &radeon_mn_ops;
> +	mutex_init(&rmn->lock);
> +	rmn->objects = RB_ROOT;
> +	
> +	r = __mmu_notifier_register(&rmn->mn, mm);
> +	if (r)
> +		goto free_rmn;
> +
> +	hash_add(rdev->mn_hash, &rmn->node, (unsigned long)mm);
> +
> +release_locks:
> +	mutex_unlock(&rdev->mn_lock);
> +	up_write(&mm->mmap_sem);
> +
> +	return rmn;
> +
> +free_rmn:
> +	mutex_unlock(&rdev->mn_lock);
> +	up_write(&mm->mmap_sem);
> +	kfree(rmn);
> +
> +	return ERR_PTR(r);
> +}
> +
> +/**
> + * radeon_mn_register - register a BO for notifier updates
> + *
> + * @bo: radeon buffer object
> + * @addr: userptr addr we should monitor
> + *
> + * Registers an MMU notifier for the given BO at the specified address.
> + * Returns 0 on success, -ERRNO if anything goes wrong.
> + */
> +int radeon_mn_register(struct radeon_bo *bo, unsigned long addr)
> +{
> +	unsigned long end = addr + radeon_bo_size(bo) - 1;
> +	struct radeon_device *rdev = bo->rdev;
> +	struct radeon_mn *rmn;
> +	struct interval_tree_node *it;
> +
> +	rmn = radeon_mn_get(rdev);
> +	if (IS_ERR(rmn))
> +		return PTR_ERR(rmn);
> +
> +	mutex_lock(&rmn->lock);
> +
> +	it = interval_tree_iter_first(&rmn->objects, addr, end);
> +	if (it) {
> +		mutex_unlock(&rmn->lock);
> +		return -EEXIST;
> +	}
> +
> +	bo->mn = rmn;
> +	bo->mn_it.start = addr;
> +	bo->mn_it.last = end;
> +	interval_tree_insert(&bo->mn_it, &rmn->objects);
> +
> +	mutex_unlock(&rmn->lock);
> +
> +	return 0;
> +}
> +
> +/**
> + * radeon_mn_unregister - unregister a BO for notifier updates
> + *
> + * @bo: radeon buffer object
> + *
> + * Remove any registration of MMU notifier updates from the buffer object.
> + */
> +void radeon_mn_unregister(struct radeon_bo *bo)
> +{
> +	struct radeon_device *rdev = bo->rdev;
> +	struct radeon_mn *rmn;
> +
> +	mutex_lock(&rdev->mn_lock);
> +	rmn = bo->mn;
> +	if (rmn == NULL) {
> +		mutex_unlock(&rdev->mn_lock);
> +		return;
> +	}
> +
> +	mutex_lock(&rmn->lock);
> +	interval_tree_remove(&bo->mn_it, &rmn->objects);
> +	bo->mn = NULL;
> +	mutex_unlock(&rmn->lock);
> +	mutex_unlock(&rdev->mn_lock);
> +}
> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> index c73c1e3..2875238 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -75,6 +75,7 @@ static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo)
>  	bo = container_of(tbo, struct radeon_bo, tbo);
>  
>  	radeon_update_memory_usage(bo, bo->tbo.mem.mem_type, -1);
> +	radeon_mn_unregister(bo);
>  
>  	mutex_lock(&bo->rdev->gem.mutex);
>  	list_del_init(&bo->list);
> diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h
> index 5dc61c2..c77495f 100644
> --- a/include/uapi/drm/radeon_drm.h
> +++ b/include/uapi/drm/radeon_drm.h
> @@ -818,6 +818,7 @@ struct drm_radeon_gem_create {
>  #define RADEON_GEM_USERPTR_READONLY	(1 << 0)
>  #define RADEON_GEM_USERPTR_ANONONLY	(1 << 1)
>  #define RADEON_GEM_USERPTR_VALIDATE	(1 << 2)
> +#define RADEON_GEM_USERPTR_REGISTER	(1 << 3)
>  
>  struct drm_radeon_gem_userptr {
>  	uint64_t		addr;
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel


More information about the dri-devel mailing list