diff mbox series

[1/6] dma-buf: move dma_resv_prune_unlocked into dma_resv.c

Message ID 20211028132630.2330-1-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series [1/6] dma-buf: move dma_resv_prune_unlocked into dma_resv.c | expand

Commit Message

Christian König Oct. 28, 2021, 1:26 p.m. UTC 
The i915 driver implements a prune function which is called when it is very
likely that the fences inside the dma_resv object can be removed because they
are all signaled.

Move that function into the dma-resv.c code since the behavior of pruning
fences is something internal to the object.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/dma-resv.c                   | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/Makefile                |  1 -
 drivers/gpu/drm/i915/dma_resv_utils.c        | 17 -----------------
 drivers/gpu/drm/i915/dma_resv_utils.h        | 13 -------------
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  3 +--
 drivers/gpu/drm/i915/gem/i915_gem_wait.c     |  3 +--
 include/linux/dma-resv.h                     |  1 +
 7 files changed, 21 insertions(+), 35 deletions(-)
 delete mode 100644 drivers/gpu/drm/i915/dma_resv_utils.c
 delete mode 100644 drivers/gpu/drm/i915/dma_resv_utils.h

Comments

Maarten Lankhorst Nov. 25, 2021, 9:31 a.m. UTC | #1 
On 28-10-2021 15:26, Christian König wrote:
> The i915 driver implements a prune function which is called when it is very
> likely that the fences inside the dma_resv object can be removed because they
> are all signaled.
>
> Move that function into the dma-resv.c code since the behavior of pruning
> fences is something internal to the object.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/dma-buf/dma-resv.c                   | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/Makefile                |  1 -
>  drivers/gpu/drm/i915/dma_resv_utils.c        | 17 -----------------
>  drivers/gpu/drm/i915/dma_resv_utils.h        | 13 -------------
>  drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  3 +--
>  drivers/gpu/drm/i915/gem/i915_gem_wait.c     |  3 +--
>  include/linux/dma-resv.h                     |  1 +
>  7 files changed, 21 insertions(+), 35 deletions(-)
>  delete mode 100644 drivers/gpu/drm/i915/dma_resv_utils.c
>  delete mode 100644 drivers/gpu/drm/i915/dma_resv_utils.h
>
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index ff3c0558b3b8..64d4f95778c4 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -324,6 +324,24 @@ void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence)
>  }
>  EXPORT_SYMBOL(dma_resv_add_excl_fence);
>  
> +/**
> + * dma_resv_prune_unlocked - try to remove signaled fences
> + * @obj: The dma_resv object to prune
> + *
> + * Try to lock the object, test if it is signaled and if yes then remove all the
> + * signaled fences.
> + */
> +void dma_resv_prune_unlocked(struct dma_resv *obj)
> +{
> +	if (!dma_resv_trylock(obj))
> +		return;
> +
> +	if (dma_resv_test_signaled(obj, true))
> +		dma_resv_add_excl_fence(obj, NULL);
> +	dma_resv_unlock(obj);
> +}
> +EXPORT_SYMBOL(dma_resv_prune_unlocked);
> +
>  /**
>   * dma_resv_iter_restart_unlocked - restart the unlocked iterator
>   * @cursor: The dma_resv_iter object to restart
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 660bb03de6fc..5c1af130cb6d 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -60,7 +60,6 @@ i915-y += i915_drv.o \
>  
>  # core library code
>  i915-y += \
> -	dma_resv_utils.o \
>  	i915_memcpy.o \
>  	i915_mm.o \
>  	i915_sw_fence.o \
> diff --git a/drivers/gpu/drm/i915/dma_resv_utils.c b/drivers/gpu/drm/i915/dma_resv_utils.c
> deleted file mode 100644
> index 7df91b7e4ca8..000000000000
> --- a/drivers/gpu/drm/i915/dma_resv_utils.c
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -// SPDX-License-Identifier: MIT
> -/*
> - * Copyright © 2020 Intel Corporation
> - */
> -
> -#include <linux/dma-resv.h>
> -
> -#include "dma_resv_utils.h"
> -
> -void dma_resv_prune(struct dma_resv *resv)
> -{
> -	if (dma_resv_trylock(resv)) {
> -		if (dma_resv_test_signaled(resv, true))
> -			dma_resv_add_excl_fence(resv, NULL);
> -		dma_resv_unlock(resv);
> -	}
> -}
> diff --git a/drivers/gpu/drm/i915/dma_resv_utils.h b/drivers/gpu/drm/i915/dma_resv_utils.h
> deleted file mode 100644
> index b9d8fb5f8367..000000000000
> --- a/drivers/gpu/drm/i915/dma_resv_utils.h
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -/* SPDX-License-Identifier: MIT */
> -/*
> - * Copyright © 2020 Intel Corporation
> - */
> -
> -#ifndef DMA_RESV_UTILS_H
> -#define DMA_RESV_UTILS_H
> -
> -struct dma_resv;
> -
> -void dma_resv_prune(struct dma_resv *resv);
> -
> -#endif /* DMA_RESV_UTILS_H */
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> index 5ab136ffdeb2..48029bbda682 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> @@ -15,7 +15,6 @@
>  
>  #include "gt/intel_gt_requests.h"
>  
> -#include "dma_resv_utils.h"
>  #include "i915_trace.h"
>  
>  static bool swap_available(void)
> @@ -229,7 +228,7 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
>  					i915_gem_object_unlock(obj);
>  			}
>  
> -			dma_resv_prune(obj->base.resv);
> +			dma_resv_prune_unlocked(obj->base.resv);
>  
>  			scanned += obj->base.size >> PAGE_SHIFT;
>  skip:
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
> index 569658c7859c..1915d203a72d 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
> @@ -10,7 +10,6 @@
>  
>  #include "gt/intel_engine.h"
>  
> -#include "dma_resv_utils.h"
>  #include "i915_gem_ioctls.h"
>  #include "i915_gem_object.h"
>  
> @@ -53,7 +52,7 @@ i915_gem_object_wait_reservation(struct dma_resv *resv,
>  	 * signaled.
>  	 */
>  	if (timeout > 0)
> -		dma_resv_prune(resv);
> +		dma_resv_prune_unlocked(resv);
>  
>  	return timeout;
>  }
> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
> index eebf04325b34..e0558429a5ee 100644
> --- a/include/linux/dma-resv.h
> +++ b/include/linux/dma-resv.h
> @@ -458,6 +458,7 @@ void dma_resv_fini(struct dma_resv *obj);
>  int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences);
>  void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence);
>  void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence);
> +void dma_resv_prune_unlocked(struct dma_resv *obj);
>  int dma_resv_get_fences(struct dma_resv *obj, struct dma_fence **pfence_excl,
>  			unsigned *pshared_count, struct dma_fence ***pshared);
>  int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src);

I don't mind adding a dma_resv_prune for locked case, but I don't think unlocked would have benefits.

Furthermore, I'm trying to remove the unlocked versions from i915. Could this be a prereq patch instead?

https://patchwork.freedesktop.org/patch/460722/?series=96115&rev=1

~Maarten

~Maarten
Christian König Nov. 25, 2021, 9:44 a.m. UTC | #2 
Am 25.11.21 um 10:31 schrieb Maarten Lankhorst:
> [SNIP]
>> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
>> index eebf04325b34..e0558429a5ee 100644
>> --- a/include/linux/dma-resv.h
>> +++ b/include/linux/dma-resv.h
>> @@ -458,6 +458,7 @@ void dma_resv_fini(struct dma_resv *obj);
>>   int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences);
>>   void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence);
>>   void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence);
>> +void dma_resv_prune_unlocked(struct dma_resv *obj);
>>   int dma_resv_get_fences(struct dma_resv *obj, struct dma_fence **pfence_excl,
>>   			unsigned *pshared_count, struct dma_fence ***pshared);
>>   int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src);
> I don't mind adding a dma_resv_prune for locked case, but I don't think unlocked would have benefits.
>
> Furthermore, I'm trying to remove the unlocked versions from i915. Could this be a prereq patch instead?
>
> https://patchwork.freedesktop.org/patch/460722/?series=96115&rev=1

Yeah, that works for me as well.

I was on the edge of dropping that from TTM as well since this is really 
just abusing the interface to save a few bytes of memory.

Feel free to add an Acked-by: Christian König <christian.koenig@amd.com> 
to the i915 patch if it helps to get that committed.

Regards,
Christian.

>
> ~Maarten
>
> ~Maarten
>
diff mbox series

Patch

diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index ff3c0558b3b8..64d4f95778c4 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -324,6 +324,24 @@  void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_resv_add_excl_fence);
 
+/**
+ * dma_resv_prune_unlocked - try to remove signaled fences
+ * @obj: The dma_resv object to prune
+ *
+ * Try to lock the object, test if it is signaled and if yes then remove all the
+ * signaled fences.
+ */
+void dma_resv_prune_unlocked(struct dma_resv *obj)
+{
+	if (!dma_resv_trylock(obj))
+		return;
+
+	if (dma_resv_test_signaled(obj, true))
+		dma_resv_add_excl_fence(obj, NULL);
+	dma_resv_unlock(obj);
+}
+EXPORT_SYMBOL(dma_resv_prune_unlocked);
+
 /**
  * dma_resv_iter_restart_unlocked - restart the unlocked iterator
  * @cursor: The dma_resv_iter object to restart
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 660bb03de6fc..5c1af130cb6d 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -60,7 +60,6 @@  i915-y += i915_drv.o \
 
 # core library code
 i915-y += \
-	dma_resv_utils.o \
 	i915_memcpy.o \
 	i915_mm.o \
 	i915_sw_fence.o \
diff --git a/drivers/gpu/drm/i915/dma_resv_utils.c b/drivers/gpu/drm/i915/dma_resv_utils.c
deleted file mode 100644
index 7df91b7e4ca8..000000000000
--- a/drivers/gpu/drm/i915/dma_resv_utils.c
+++ /dev/null
@@ -1,17 +0,0 @@ 
-// SPDX-License-Identifier: MIT
-/*
- * Copyright © 2020 Intel Corporation
- */
-
-#include <linux/dma-resv.h>
-
-#include "dma_resv_utils.h"
-
-void dma_resv_prune(struct dma_resv *resv)
-{
-	if (dma_resv_trylock(resv)) {
-		if (dma_resv_test_signaled(resv, true))
-			dma_resv_add_excl_fence(resv, NULL);
-		dma_resv_unlock(resv);
-	}
-}
diff --git a/drivers/gpu/drm/i915/dma_resv_utils.h b/drivers/gpu/drm/i915/dma_resv_utils.h
deleted file mode 100644
index b9d8fb5f8367..000000000000
--- a/drivers/gpu/drm/i915/dma_resv_utils.h
+++ /dev/null
@@ -1,13 +0,0 @@ 
-/* SPDX-License-Identifier: MIT */
-/*
- * Copyright © 2020 Intel Corporation
- */
-
-#ifndef DMA_RESV_UTILS_H
-#define DMA_RESV_UTILS_H
-
-struct dma_resv;
-
-void dma_resv_prune(struct dma_resv *resv);
-
-#endif /* DMA_RESV_UTILS_H */
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index 5ab136ffdeb2..48029bbda682 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -15,7 +15,6 @@ 
 
 #include "gt/intel_gt_requests.h"
 
-#include "dma_resv_utils.h"
 #include "i915_trace.h"
 
 static bool swap_available(void)
@@ -229,7 +228,7 @@  i915_gem_shrink(struct i915_gem_ww_ctx *ww,
 					i915_gem_object_unlock(obj);
 			}
 
-			dma_resv_prune(obj->base.resv);
+			dma_resv_prune_unlocked(obj->base.resv);
 
 			scanned += obj->base.size >> PAGE_SHIFT;
 skip:
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
index 569658c7859c..1915d203a72d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
@@ -10,7 +10,6 @@ 
 
 #include "gt/intel_engine.h"
 
-#include "dma_resv_utils.h"
 #include "i915_gem_ioctls.h"
 #include "i915_gem_object.h"
 
@@ -53,7 +52,7 @@  i915_gem_object_wait_reservation(struct dma_resv *resv,
 	 * signaled.
 	 */
 	if (timeout > 0)
-		dma_resv_prune(resv);
+		dma_resv_prune_unlocked(resv);
 
 	return timeout;
 }
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
index eebf04325b34..e0558429a5ee 100644
--- a/include/linux/dma-resv.h
+++ b/include/linux/dma-resv.h
@@ -458,6 +458,7 @@  void dma_resv_fini(struct dma_resv *obj);
 int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences);
 void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence);
 void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence);
+void dma_resv_prune_unlocked(struct dma_resv *obj);
 int dma_resv_get_fences(struct dma_resv *obj, struct dma_fence **pfence_excl,
 			unsigned *pshared_count, struct dma_fence ***pshared);
 int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src);