All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Ekstrand <jason@jlekstrand.net>
To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Jon Bloomfield <jon.bloomfield@intel.com>,
	Matthew Auld <matthew.auld@intel.com>,
	Jason Ekstrand <jason@jlekstrand.net>
Subject: [PATCH 1/5] drm/i915: Move intel_engine_free_request_pool to i915_request.c
Date: Wed,  9 Jun 2021 16:29:55 -0500	[thread overview]
Message-ID: <20210609212959.471209-2-jason@jlekstrand.net> (raw)
In-Reply-To: <20210609212959.471209-1-jason@jlekstrand.net>

This appears to break encapsulation by moving an intel_engine_cs
function to a i915_request file.  However, this function is
intrinsically tied to the lifetime rules and allocation scheme of
i915_request and having it in intel_engine_cs.c leaks details of
i915_request.  We have an abstraction leak either way.  Since
i915_request's allocation scheme is far more subtle than the simple
pointer that is intel_engine_cs.request_pool, it's probably better to
keep i915_request's details to itself.

Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 8 --------
 drivers/gpu/drm/i915/i915_request.c       | 7 +++++--
 drivers/gpu/drm/i915/i915_request.h       | 2 --
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 9ceddfbb1687d..df6b80ec84199 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -422,14 +422,6 @@ void intel_engines_release(struct intel_gt *gt)
 	}
 }
 
-void intel_engine_free_request_pool(struct intel_engine_cs *engine)
-{
-	if (!engine->request_pool)
-		return;
-
-	kmem_cache_free(i915_request_slab_cache(), engine->request_pool);
-}
-
 void intel_engines_free(struct intel_gt *gt)
 {
 	struct intel_engine_cs *engine;
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 1014c71cf7f52..48c5f8527854b 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -106,9 +106,12 @@ static signed long i915_fence_wait(struct dma_fence *fence,
 				 timeout);
 }
 
-struct kmem_cache *i915_request_slab_cache(void)
+void intel_engine_free_request_pool(struct intel_engine_cs *engine)
 {
-	return global.slab_requests;
+	if (!engine->request_pool)
+		return;
+
+	kmem_cache_free(global.slab_requests, engine->request_pool);
 }
 
 static void i915_fence_release(struct dma_fence *fence)
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 270f6cd37650c..f84c38d29f988 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -300,8 +300,6 @@ static inline bool dma_fence_is_i915(const struct dma_fence *fence)
 	return fence->ops == &i915_fence_ops;
 }
 
-struct kmem_cache *i915_request_slab_cache(void);
-
 struct i915_request * __must_check
 __i915_request_create(struct intel_context *ce, gfp_t gfp);
 struct i915_request * __must_check
-- 
2.31.1


WARNING: multiple messages have this Message-ID (diff)
From: Jason Ekstrand <jason@jlekstrand.net>
To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Matthew Auld <matthew.auld@intel.com>
Subject: [Intel-gfx] [PATCH 1/5] drm/i915: Move intel_engine_free_request_pool to i915_request.c
Date: Wed,  9 Jun 2021 16:29:55 -0500	[thread overview]
Message-ID: <20210609212959.471209-2-jason@jlekstrand.net> (raw)
In-Reply-To: <20210609212959.471209-1-jason@jlekstrand.net>

This appears to break encapsulation by moving an intel_engine_cs
function to a i915_request file.  However, this function is
intrinsically tied to the lifetime rules and allocation scheme of
i915_request and having it in intel_engine_cs.c leaks details of
i915_request.  We have an abstraction leak either way.  Since
i915_request's allocation scheme is far more subtle than the simple
pointer that is intel_engine_cs.request_pool, it's probably better to
keep i915_request's details to itself.

Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 8 --------
 drivers/gpu/drm/i915/i915_request.c       | 7 +++++--
 drivers/gpu/drm/i915/i915_request.h       | 2 --
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 9ceddfbb1687d..df6b80ec84199 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -422,14 +422,6 @@ void intel_engines_release(struct intel_gt *gt)
 	}
 }
 
-void intel_engine_free_request_pool(struct intel_engine_cs *engine)
-{
-	if (!engine->request_pool)
-		return;
-
-	kmem_cache_free(i915_request_slab_cache(), engine->request_pool);
-}
-
 void intel_engines_free(struct intel_gt *gt)
 {
 	struct intel_engine_cs *engine;
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 1014c71cf7f52..48c5f8527854b 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -106,9 +106,12 @@ static signed long i915_fence_wait(struct dma_fence *fence,
 				 timeout);
 }
 
-struct kmem_cache *i915_request_slab_cache(void)
+void intel_engine_free_request_pool(struct intel_engine_cs *engine)
 {
-	return global.slab_requests;
+	if (!engine->request_pool)
+		return;
+
+	kmem_cache_free(global.slab_requests, engine->request_pool);
 }
 
 static void i915_fence_release(struct dma_fence *fence)
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 270f6cd37650c..f84c38d29f988 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -300,8 +300,6 @@ static inline bool dma_fence_is_i915(const struct dma_fence *fence)
 	return fence->ops == &i915_fence_ops;
 }
 
-struct kmem_cache *i915_request_slab_cache(void);
-
 struct i915_request * __must_check
 __i915_request_create(struct intel_context *ce, gfp_t gfp);
 struct i915_request * __must_check
-- 
2.31.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2021-06-09 21:30 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 21:29 [PATCH 0/5] dma-fence, i915: Stop allowing SLAB_TYPESAFE_BY_RCU for dma_fence Jason Ekstrand
2021-06-09 21:29 ` [Intel-gfx] " Jason Ekstrand
2021-06-09 21:29 ` Jason Ekstrand [this message]
2021-06-09 21:29   ` [Intel-gfx] [PATCH 1/5] drm/i915: Move intel_engine_free_request_pool to i915_request.c Jason Ekstrand
2021-06-10 10:03   ` Tvrtko Ursulin
2021-06-10 10:03     ` Tvrtko Ursulin
2021-06-10 13:57     ` Jason Ekstrand
2021-06-10 13:57       ` Jason Ekstrand
2021-06-10 15:07       ` Tvrtko Ursulin
2021-06-10 15:07         ` Tvrtko Ursulin
2021-06-10 16:32         ` Jason Ekstrand
2021-06-10 16:32           ` Jason Ekstrand
2021-06-09 21:29 ` [PATCH 2/5] drm/i915: Use a simpler scheme for caching i915_request Jason Ekstrand
2021-06-09 21:29   ` [Intel-gfx] " Jason Ekstrand
2021-06-10 10:08   ` Tvrtko Ursulin
2021-06-10 10:08     ` Tvrtko Ursulin
2021-06-10 13:50     ` Jason Ekstrand
2021-06-10 13:50       ` Jason Ekstrand
2021-06-09 21:29 ` [PATCH 3/5] drm/i915: Stop using SLAB_TYPESAFE_BY_RCU for i915_request Jason Ekstrand
2021-06-09 21:29   ` [Intel-gfx] " Jason Ekstrand
2021-06-09 21:29 ` [PATCH 4/5] dma-buf: Stop using SLAB_TYPESAFE_BY_RCU in selftests Jason Ekstrand
2021-06-09 21:29   ` [Intel-gfx] " Jason Ekstrand
2021-06-16 12:47   ` kernel test robot
2021-06-16 12:47     ` kernel test robot
2021-06-16 12:47     ` kernel test robot
2021-06-09 21:29 ` [PATCH 5/5] DONOTMERGE: dma-buf: Get rid of dma_fence_get_rcu_safe Jason Ekstrand
2021-06-09 21:29   ` [Intel-gfx] " Jason Ekstrand
2021-06-10  6:51   ` Christian König
2021-06-10  6:51     ` [Intel-gfx] " Christian König
2021-06-10 13:59     ` Jason Ekstrand
2021-06-10 13:59       ` [Intel-gfx] " Jason Ekstrand
2021-06-10 15:13       ` Daniel Vetter
2021-06-10 15:13         ` [Intel-gfx] " Daniel Vetter
2021-06-10 16:24         ` Jason Ekstrand
2021-06-10 16:24           ` [Intel-gfx] " Jason Ekstrand
2021-06-10 16:37           ` Daniel Vetter
2021-06-10 16:37             ` [Intel-gfx] " Daniel Vetter
2021-06-10 16:52             ` Jason Ekstrand
2021-06-10 16:52               ` [Intel-gfx] " Jason Ekstrand
2021-06-10 17:06               ` Daniel Vetter
2021-06-10 17:06                 ` [Intel-gfx] " Daniel Vetter
2021-06-10 16:54             ` Christian König
2021-06-10 16:54               ` [Intel-gfx] " Christian König
2021-06-10 17:11               ` Daniel Vetter
2021-06-10 17:11                 ` [Intel-gfx] " Daniel Vetter
2021-06-10 18:12                 ` Christian König
2021-06-10 18:12                   ` [Intel-gfx] " Christian König
2021-06-16 16:38   ` kernel test robot
2021-06-16 16:38     ` kernel test robot
2021-06-16 16:38     ` kernel test robot
2021-06-09 21:52 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for dma-fence, i915: Stop allowing SLAB_TYPESAFE_BY_RCU for dma_fence Patchwork
2021-06-09 21:54 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-06-09 22:22 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-06-09 22:22 ` [Intel-gfx] ✗ Fi.CI.BUILD: warning " Patchwork
2021-06-10  9:29 ` [Intel-gfx] [PATCH 0/5] " Tvrtko Ursulin
2021-06-10  9:29   ` Tvrtko Ursulin
2021-06-10  9:39   ` Christian König
2021-06-10  9:39     ` Christian König
2021-06-10 11:29     ` Daniel Vetter
2021-06-10 11:29       ` Daniel Vetter
2021-06-10 11:53       ` Daniel Vetter
2021-06-10 11:53         ` Daniel Vetter
2021-06-10 13:07       ` Tvrtko Ursulin
2021-06-10 13:07         ` Tvrtko Ursulin
2021-06-10 13:35       ` Jason Ekstrand
2021-06-10 13:35         ` Jason Ekstrand
2021-06-10 20:09         ` Jason Ekstrand
2021-06-10 20:09           ` Jason Ekstrand
2021-06-10 20:42           ` Daniel Vetter
2021-06-10 20:42             ` Daniel Vetter
2021-06-11  6:55             ` Christian König
2021-06-11  6:55               ` Christian König
2021-06-11  7:20               ` Daniel Vetter
2021-06-11  7:20                 ` Daniel Vetter
2021-06-11  7:42                 ` Christian König
2021-06-11  7:42                   ` Christian König
2021-06-11  9:33                   ` Daniel Vetter
2021-06-11  9:33                     ` Daniel Vetter
2021-06-11 10:03                     ` Christian König
2021-06-11 10:03                       ` Christian König
2021-06-11 15:08                       ` Daniel Vetter
2021-06-11 15:08                         ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210609212959.471209-2-jason@jlekstrand.net \
    --to=jason@jlekstrand.net \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jon.bloomfield@intel.com \
    --cc=matthew.auld@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.