Re: [Intel-gfx] [PATCH 03/28] drm/i915: Remove dma_resv_prune

Matthew Auld
2021-10-21T14:43:40Z

On Thu, 21 Oct 2021 at 11:36, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
>
> The signaled bit is already used for quick testing if a fence is signaled.

Why do we need this change? Can you add some more details to the commit please?

>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> 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 | 8 --------
> 5 files changed, 42 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/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 467872cca027..b87e3ed10d86 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..af3eb7fd951d 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,8 +228,6 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
> i915_gem_object_unlock(obj);
> }
>
> - dma_resv_prune(obj->base.resv);

Like here, why do we want to drop this? Later in the series it looks
like it gets added back, just in a slightly different form.

> -
> scanned += obj->base.size >> PAGE_SHIFT;
> skip:
> i915_gem_object_put(obj);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c
> index 840c13706999..1592d95c3ead 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"
>
> @@ -52,13 +51,6 @@ i915_gem_object_wait_reservation(struct dma_resv *resv,
> }
> dma_resv_iter_end(&cursor);
>
> - /*
> - * Opportunistically prune the fences iff we know they have *all* been
> - * signaled.
> - */
> - if (timeout > 0)
> - dma_resv_prune(resv);
> -
> return ret;
> }
>
> --
> 2.33.0
>