Re: [PATCH] drm/ttm: Use RCU for ttm_mem_type_manager.move fence

[Christian König <christian.koenig@xxxxxxx>]

Am 29.08.2016 um 18:57 schrieb Chris Wilson:
> The ttm_mem_type_manager.move tracks the fence for the last migration on
> the memory manager. Currently it is accessed under its own spinlock to
> ensure that the fence doesn't disappear from underneath it. We can
> translate the reader to acquire a reference to the fence using
> fence_get_rcu_safe() which ensures that the fence cannot be reallocated
> as the reference is acquired.
>
> Suggested-by: Christian König <christian.koenig@xxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Christian König <christian.koenig@xxxxxxx>

Reviewed-by: Christian König <christian.koenig@xxxxxxx>.

> ---
>   drivers/gpu/drm/ttm/ttm_bo.c      | 24 ++++++++++++++++--------
>   drivers/gpu/drm/ttm/ttm_bo_util.c | 12 ++++++++----
>   include/drm/ttm/ttm_bo_driver.h   |  2 +-
>   3 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 42c074a9c955..422d9b39d8ae 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -797,9 +797,9 @@ static int ttm_bo_add_move_fence(struct ttm_buffer_object *bo,
>   	struct fence *fence;
>   	int ret;
>   
> -	spin_lock(&man->move_lock);
> -	fence = fence_get(man->move);
> -	spin_unlock(&man->move_lock);
> +	rcu_read_lock();
> +	fence = fence_get_rcu_safe(&man->move);
> +	rcu_read_unlock();
>   
>   	if (fence) {
>   		reservation_object_add_shared_fence(bo->resv, fence);
> @@ -1310,9 +1310,9 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev,
>   	}
>   	spin_unlock(&glob->lru_lock);
>   
> -	spin_lock(&man->move_lock);
> -	fence = fence_get(man->move);
> -	spin_unlock(&man->move_lock);
> +	rcu_read_lock();
> +	fence = fence_get_rcu_safe(&man->move);
> +	rcu_read_unlock();
>   
>   	if (fence) {
>   		ret = fence_wait(fence, false);
> @@ -1332,6 +1332,7 @@ static int ttm_bo_force_list_clean(struct ttm_bo_device *bdev,
>   int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type)
>   {
>   	struct ttm_mem_type_manager *man;
> +	struct fence *last_move;
>   	int ret = -EINVAL;
>   
>   	if (mem_type >= TTM_NUM_MEM_TYPES) {
> @@ -1345,7 +1346,14 @@ int ttm_bo_clean_mm(struct ttm_bo_device *bdev, unsigned mem_type)
>   		       mem_type);
>   		return ret;
>   	}
> -	fence_put(man->move);
> +
> +	/* The locking here is overkill; but harmless and documentary */
> +	spin_lock(&man->move_lock);
> +	last_move = rcu_dereference_protected(man->move,
> +					      spin_is_locked(&man->move_lock));
> +	rcu_assign_pointer(man->move, NULL);
> +	spin_unlock(&man->move_lock);
> +	fence_put(last_move);
>   
>   	man->use_type = false;
>   	man->has_type = false;
> @@ -1410,7 +1418,7 @@ int ttm_bo_init_mm(struct ttm_bo_device *bdev, unsigned type,
>   	man->size = p_size;
>   
>   	INIT_LIST_HEAD(&man->lru);
> -	man->move = NULL;
> +	RCU_INIT_POINTER(man->move, NULL);
>   
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index f157a9efd220..cd675d503ee4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -755,6 +755,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>   		ttm_bo_unref(&ghost_obj);
>   
>   	} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +		struct fence *last_move;
>   
>   		/**
>   		 * BO doesn't have a TTM we need to bind/unbind. Just remember
> @@ -762,11 +763,14 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>   		 */
>   
>   		spin_lock(&from->move_lock);
> -		if (!from->move || fence_is_later(fence, from->move)) {
> -			fence_put(from->move);
> -			from->move = fence_get(fence);
> -		}
> +		last_move = rcu_dereference_protected(from->move,
> +						      spin_is_locked(&from->move_lock));
> +		if (!last_move || fence_is_later(fence, last_move))
> +			rcu_assign_pointer(from->move, fence_get(fence));
> +		else
> +			last_move = NULL;
>   		spin_unlock(&from->move_lock);
> +		fence_put(last_move);
>   
>   		ttm_bo_free_old_node(bo);
>   
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 99c6d01d24f2..508d2f428d25 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -305,7 +305,7 @@ struct ttm_mem_type_manager {
>   	/*
>   	 * Protected by @move_lock.
>   	 */
> -	struct fence *move;
> +	struct fence __rcu *move;
>   };
>   
>   /**


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel