diff mbox

drm/ttm: Use RCU for ttm_mem_type_manager.move fence

Message ID 20160829165711.27101-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Aug. 29, 2016, 4:57 p.m. UTC
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@amd.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Christian König <christian.koenig@amd.com>
---
 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(-)

Comments

Christian König Aug. 29, 2016, 5:09 p.m. UTC | #1
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@amd.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Christian König <christian.koenig@amd.com>

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

> ---
>   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;
>   };
>   
>   /**
diff mbox

Patch

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;
 };
 
 /**