All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/8] fdinfo memory stats
@ 2023-06-09 12:11 ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I added tracking of most classes of objects which contribute to client's memory
footprint and accouting along the similar lines as in Rob's msm code. Then
printing it out to fdinfo using the drm helper Rob added.

Accounting by keeping per client lists may not be the most effient method,
perhaps we should simply add and subtract stats directly at convenient sites,
but that too is not straightforward due no existing connection between buffer
objects and clients. Possibly some other tricky bits in the buffer sharing
deparment. So lets see if this works for now. Infrequent reader penalty should
not be too bad (may be even useful to dump the lists in debugfs?) and additional
list_head per object pretty much drowns in the noise.

Example fdinfo with the series applied:

# cat /proc/1383/fdinfo/8
pos:    0
flags:  02100002
mnt_id: 21
ino:    397
drm-driver:     i915
drm-client-id:  18
drm-pdev:       0000:00:02.0
drm-total-system:       125 MiB
drm-shared-system:      16 MiB
drm-active-system:      110 MiB
drm-resident-system:    125 MiB
drm-purgeable-system:   2 MiB
drm-total-stolen-system:        0
drm-shared-stolen-system:       0
drm-active-stolen-system:       0
drm-resident-stolen-system:     0
drm-purgeable-stolen-system:    0
drm-engine-render:      25662044495 ns
drm-engine-copy:        0 ns
drm-engine-video:       0 ns
drm-engine-video-enhance:       0 ns

Example gputop output (local patches currently):

DRM minor 0
 PID     SMEM  SMEMRSS   render     copy     video    NAME
1233     124M     124M |████████||        ||        ||        | neverball
1130      59M      59M |█▌      ||        ||        ||        | Xorg
1207      12M      12M |        ||        ||        ||        | xfwm4

v2:
 * Now actually per client.

v3:
 * Track imported dma-buf objects.

P.S. Patch 1 in the series is to silence a false positive lockdep splat due
fence signaling annotations.

Tvrtko Ursulin (8):
  dma-fence: Bypass signaling annotation from dma_fence_is_signaled
  drm/i915: Track buffer objects belonging to clients
  drm/i915: Record which clients own a VM
  drm/i915: Track page table backing store usage
  drm/i915: Account ring buffer and context state storage
  drm: Add drm_gem_prime_fd_to_handle_obj
  drm/i915: Track imported dma-buf objects in memory stats
  drm/i915: Implement fdinfo memory stats printing

 drivers/dma-buf/dma-fence.c                   |  26 +++-
 drivers/gpu/drm/drm_prime.c                   |  41 ++++++-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  17 ++-
 .../gpu/drm/i915/gem/i915_gem_context_types.h |   3 +
 drivers/gpu/drm/i915/gem/i915_gem_create.c    |  32 ++++-
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c    |  32 +++++
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h    |   7 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |   6 +
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  12 ++
 .../gpu/drm/i915/gem/selftests/mock_context.c |   4 +-
 drivers/gpu/drm/i915/gt/intel_gtt.c           |   6 +
 drivers/gpu/drm/i915/gt/intel_gtt.h           |   1 +
 drivers/gpu/drm/i915/i915_driver.c            |   2 +-
 drivers/gpu/drm/i915/i915_drm_client.c        | 113 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_drm_client.h        |  45 ++++++-
 drivers/gpu/drm/i915/i915_gem.c               |   2 +-
 include/drm/drm_prime.h                       |   4 +
 include/linux/dma-fence.h                     |   3 +-
 18 files changed, 332 insertions(+), 24 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH v3 0/8] fdinfo memory stats
@ 2023-06-09 12:11 ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I added tracking of most classes of objects which contribute to client's memory
footprint and accouting along the similar lines as in Rob's msm code. Then
printing it out to fdinfo using the drm helper Rob added.

Accounting by keeping per client lists may not be the most effient method,
perhaps we should simply add and subtract stats directly at convenient sites,
but that too is not straightforward due no existing connection between buffer
objects and clients. Possibly some other tricky bits in the buffer sharing
deparment. So lets see if this works for now. Infrequent reader penalty should
not be too bad (may be even useful to dump the lists in debugfs?) and additional
list_head per object pretty much drowns in the noise.

Example fdinfo with the series applied:

# cat /proc/1383/fdinfo/8
pos:    0
flags:  02100002
mnt_id: 21
ino:    397
drm-driver:     i915
drm-client-id:  18
drm-pdev:       0000:00:02.0
drm-total-system:       125 MiB
drm-shared-system:      16 MiB
drm-active-system:      110 MiB
drm-resident-system:    125 MiB
drm-purgeable-system:   2 MiB
drm-total-stolen-system:        0
drm-shared-stolen-system:       0
drm-active-stolen-system:       0
drm-resident-stolen-system:     0
drm-purgeable-stolen-system:    0
drm-engine-render:      25662044495 ns
drm-engine-copy:        0 ns
drm-engine-video:       0 ns
drm-engine-video-enhance:       0 ns

Example gputop output (local patches currently):

DRM minor 0
 PID     SMEM  SMEMRSS   render     copy     video    NAME
1233     124M     124M |████████||        ||        ||        | neverball
1130      59M      59M |█▌      ||        ||        ||        | Xorg
1207      12M      12M |        ||        ||        ||        | xfwm4

v2:
 * Now actually per client.

v3:
 * Track imported dma-buf objects.

P.S. Patch 1 in the series is to silence a false positive lockdep splat due
fence signaling annotations.

Tvrtko Ursulin (8):
  dma-fence: Bypass signaling annotation from dma_fence_is_signaled
  drm/i915: Track buffer objects belonging to clients
  drm/i915: Record which clients own a VM
  drm/i915: Track page table backing store usage
  drm/i915: Account ring buffer and context state storage
  drm: Add drm_gem_prime_fd_to_handle_obj
  drm/i915: Track imported dma-buf objects in memory stats
  drm/i915: Implement fdinfo memory stats printing

 drivers/dma-buf/dma-fence.c                   |  26 +++-
 drivers/gpu/drm/drm_prime.c                   |  41 ++++++-
 drivers/gpu/drm/i915/gem/i915_gem_context.c   |  17 ++-
 .../gpu/drm/i915/gem/i915_gem_context_types.h |   3 +
 drivers/gpu/drm/i915/gem/i915_gem_create.c    |  32 ++++-
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c    |  32 +++++
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h    |   7 ++
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |   6 +
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |  12 ++
 .../gpu/drm/i915/gem/selftests/mock_context.c |   4 +-
 drivers/gpu/drm/i915/gt/intel_gtt.c           |   6 +
 drivers/gpu/drm/i915/gt/intel_gtt.h           |   1 +
 drivers/gpu/drm/i915/i915_driver.c            |   2 +-
 drivers/gpu/drm/i915/i915_drm_client.c        | 113 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_drm_client.h        |  45 ++++++-
 drivers/gpu/drm/i915/i915_gem.c               |   2 +-
 include/drm/drm_prime.h                       |   4 +
 include/linux/dma-fence.h                     |   3 +-
 18 files changed, 332 insertions(+), 24 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 29+ messages in thread
* [PATCH 1/8] dma-fence: Bypass signaling annotation from dma_fence_is_signaled
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Daniel Vetter, Christian König, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

For dma_fence_is_signaled signaling critical path annotations are an
annoying cause of false positives when using dma_fence_is_signaled and
indirectly higher level helpers such as dma_resv_test_signaled etc.

Drop the critical path annotation since the "is signaled" API does not
guarantee to ever change the signaled status anyway.

We do that by adding a low level _dma_fence_signal helper and use it from
dma_fence_is_signaled.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/dma-fence.c | 26 ++++++++++++++++++++------
 include/linux/dma-fence.h   |  3 ++-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index f177c56269bb..f216a189a755 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -444,6 +444,25 @@ int dma_fence_signal_locked(struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_fence_signal_locked);
 
+/**
+ * _dma_fence_signal - signal completion of a fence bypassing critical section annotation
+ * @fence: the fence to signal
+ *
+ * This low-level helper should not be used by code external to dma-fence.h|c!
+ */
+int _dma_fence_signal(struct dma_fence *fence)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(fence->lock, flags);
+	ret = dma_fence_signal_timestamp_locked(fence, ktime_get());
+	spin_unlock_irqrestore(fence->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(_dma_fence_signal);
+
 /**
  * dma_fence_signal - signal completion of a fence
  * @fence: the fence to signal
@@ -459,7 +478,6 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
  */
 int dma_fence_signal(struct dma_fence *fence)
 {
-	unsigned long flags;
 	int ret;
 	bool tmp;
 
@@ -467,11 +485,7 @@ int dma_fence_signal(struct dma_fence *fence)
 		return -EINVAL;
 
 	tmp = dma_fence_begin_signalling();
-
-	spin_lock_irqsave(fence->lock, flags);
-	ret = dma_fence_signal_timestamp_locked(fence, ktime_get());
-	spin_unlock_irqrestore(fence->lock, flags);
-
+	ret = _dma_fence_signal(fence);
 	dma_fence_end_signalling(tmp);
 
 	return ret;
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index d54b595a0fe0..d94768ad70e4 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -387,6 +387,7 @@ static inline void dma_fence_end_signalling(bool cookie) {}
 static inline void __dma_fence_might_wait(void) {}
 #endif
 
+int _dma_fence_signal(struct dma_fence *fence);
 int dma_fence_signal(struct dma_fence *fence);
 int dma_fence_signal_locked(struct dma_fence *fence);
 int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
@@ -452,7 +453,7 @@ dma_fence_is_signaled(struct dma_fence *fence)
 		return true;
 
 	if (fence->ops->signaled && fence->ops->signaled(fence)) {
-		dma_fence_signal(fence);
+		_dma_fence_signal(fence);
 		return true;
 	}
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH 1/8] dma-fence: Bypass signaling annotation from dma_fence_is_signaled
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Daniel Vetter, Christian König

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

For dma_fence_is_signaled signaling critical path annotations are an
annoying cause of false positives when using dma_fence_is_signaled and
indirectly higher level helpers such as dma_resv_test_signaled etc.

Drop the critical path annotation since the "is signaled" API does not
guarantee to ever change the signaled status anyway.

We do that by adding a low level _dma_fence_signal helper and use it from
dma_fence_is_signaled.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/dma-buf/dma-fence.c | 26 ++++++++++++++++++++------
 include/linux/dma-fence.h   |  3 ++-
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index f177c56269bb..f216a189a755 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -444,6 +444,25 @@ int dma_fence_signal_locked(struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_fence_signal_locked);
 
+/**
+ * _dma_fence_signal - signal completion of a fence bypassing critical section annotation
+ * @fence: the fence to signal
+ *
+ * This low-level helper should not be used by code external to dma-fence.h|c!
+ */
+int _dma_fence_signal(struct dma_fence *fence)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(fence->lock, flags);
+	ret = dma_fence_signal_timestamp_locked(fence, ktime_get());
+	spin_unlock_irqrestore(fence->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL(_dma_fence_signal);
+
 /**
  * dma_fence_signal - signal completion of a fence
  * @fence: the fence to signal
@@ -459,7 +478,6 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
  */
 int dma_fence_signal(struct dma_fence *fence)
 {
-	unsigned long flags;
 	int ret;
 	bool tmp;
 
@@ -467,11 +485,7 @@ int dma_fence_signal(struct dma_fence *fence)
 		return -EINVAL;
 
 	tmp = dma_fence_begin_signalling();
-
-	spin_lock_irqsave(fence->lock, flags);
-	ret = dma_fence_signal_timestamp_locked(fence, ktime_get());
-	spin_unlock_irqrestore(fence->lock, flags);
-
+	ret = _dma_fence_signal(fence);
 	dma_fence_end_signalling(tmp);
 
 	return ret;
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index d54b595a0fe0..d94768ad70e4 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -387,6 +387,7 @@ static inline void dma_fence_end_signalling(bool cookie) {}
 static inline void __dma_fence_might_wait(void) {}
 #endif
 
+int _dma_fence_signal(struct dma_fence *fence);
 int dma_fence_signal(struct dma_fence *fence);
 int dma_fence_signal_locked(struct dma_fence *fence);
 int dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
@@ -452,7 +453,7 @@ dma_fence_is_signaled(struct dma_fence *fence)
 		return true;
 
 	if (fence->ops->signaled && fence->ops->signaled(fence)) {
-		dma_fence_signal(fence);
+		_dma_fence_signal(fence);
 		return true;
 	}
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH 2/8] drm/i915: Track buffer objects belonging to clients
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

In order to show per client memory usage lets start tracking which
objects belong to which clients.

We start with objects explicitly created by object creation UAPI and
track it on a new per client lists, protected by a new per client lock.
In order for delayed destruction (post client exit), we make tracked
objects hold references to the owning client.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c    | 32 +++++++++++++--
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |  6 +++
 .../gpu/drm/i915/gem/i915_gem_object_types.h  | 12 ++++++
 drivers/gpu/drm/i915/i915_drm_client.c        | 39 ++++++++++++++++++-
 drivers/gpu/drm/i915/i915_drm_client.h        | 36 ++++++++++++++++-
 drivers/gpu/drm/i915/i915_gem.c               |  2 +-
 6 files changed, 121 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index d24c0ce8805c..4f1957638207 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -11,6 +11,7 @@
 #include "gem/i915_gem_region.h"
 #include "pxp/intel_pxp.h"
 
+#include "i915_drm_client.h"
 #include "i915_drv.h"
 #include "i915_gem_create.h"
 #include "i915_trace.h"
@@ -164,6 +165,14 @@ __i915_gem_object_create_user(struct drm_i915_private *i915, u64 size,
 						 n_placements, 0);
 }
 
+static void add_file_obj(struct drm_file *file,
+			 struct drm_i915_gem_object *obj)
+{
+	struct drm_i915_file_private *fpriv = file->driver_priv;
+
+	i915_drm_client_add_object(fpriv->client, obj);
+}
+
 int
 i915_gem_dumb_create(struct drm_file *file,
 		     struct drm_device *dev,
@@ -174,6 +183,7 @@ i915_gem_dumb_create(struct drm_file *file,
 	enum intel_memory_type mem_type;
 	int cpp = DIV_ROUND_UP(args->bpp, 8);
 	u32 format;
+	int ret;
 
 	switch (cpp) {
 	case 1:
@@ -212,7 +222,12 @@ i915_gem_dumb_create(struct drm_file *file,
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	return i915_gem_publish(obj, file, &args->size, &args->handle);
+	ret = i915_gem_publish(obj, file, &args->size, &args->handle);
+
+	if (!ret)
+		add_file_obj(file, obj);
+
+	return ret;
 }
 
 /**
@@ -229,6 +244,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_gem_create *args = data;
 	struct drm_i915_gem_object *obj;
 	struct intel_memory_region *mr;
+	int ret;
 
 	mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
 
@@ -236,7 +252,12 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	return i915_gem_publish(obj, file, &args->size, &args->handle);
+	ret = i915_gem_publish(obj, file, &args->size, &args->handle);
+
+	if (!ret)
+		add_file_obj(file, obj);
+
+	return ret;
 }
 
 struct create_ext {
@@ -494,5 +515,10 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
 		obj->pat_set_by_user = true;
 	}
 
-	return i915_gem_publish(obj, file, &args->size, &args->handle);
+	ret = i915_gem_publish(obj, file, &args->size, &args->handle);
+
+	if (!ret)
+		add_file_obj(file, obj);
+
+	return ret;
 }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 97ac6fb37958..46de9b1b3f1d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -105,6 +105,10 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&obj->mm.link);
 
+#ifdef CONFIG_PROC_FS
+	INIT_LIST_HEAD(&obj->client_link);
+#endif
+
 	INIT_LIST_HEAD(&obj->lut_list);
 	spin_lock_init(&obj->lut_lock);
 
@@ -441,6 +445,8 @@ static void i915_gem_free_object(struct drm_gem_object *gem_obj)
 
 	GEM_BUG_ON(i915_gem_object_is_framebuffer(obj));
 
+	i915_drm_client_remove_object(obj);
+
 	/*
 	 * Before we free the object, make sure any pure RCU-only
 	 * read-side critical sections are complete, e.g.
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index e72c57716bee..8de2b91b3edf 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -300,6 +300,18 @@ struct drm_i915_gem_object {
 	 */
 	struct i915_address_space *shares_resv_from;
 
+#ifdef CONFIG_PROC_FS
+	/**
+	 * @client: @i915_drm_client which created the object
+	 */
+	struct i915_drm_client *client;
+
+	/**
+	 * @client_link: Link into @i915_drm_client.objects_list
+	 */
+	struct list_head client_link;
+#endif
+
 	union {
 		struct rcu_head rcu;
 		struct llist_node freed;
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 2a44b3876cb5..b0b35bcdd2b3 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -17,7 +17,8 @@
 #include "i915_gem.h"
 #include "i915_utils.h"
 
-struct i915_drm_client *i915_drm_client_alloc(void)
+struct i915_drm_client *
+i915_drm_client_alloc(struct drm_i915_file_private *fpriv)
 {
 	struct i915_drm_client *client;
 
@@ -28,6 +29,12 @@ struct i915_drm_client *i915_drm_client_alloc(void)
 	kref_init(&client->kref);
 	spin_lock_init(&client->ctx_lock);
 	INIT_LIST_HEAD(&client->ctx_list);
+#ifdef CONFIG_PROC_FS
+	spin_lock_init(&client->objects_lock);
+	INIT_LIST_HEAD(&client->objects_list);
+
+	client->fpriv = fpriv;
+#endif
 
 	return client;
 }
@@ -108,4 +115,34 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file)
 	for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++)
 		show_client_class(p, i915, file_priv->client, i);
 }
+
+void i915_drm_client_add_object(struct i915_drm_client *client,
+				struct drm_i915_gem_object *obj)
+{
+	unsigned long flags;
+
+	GEM_WARN_ON(obj->client);
+	GEM_WARN_ON(!list_empty(&obj->client_link));
+
+	spin_lock_irqsave(&client->objects_lock, flags);
+	obj->client = i915_drm_client_get(client);
+	list_add_tail(&obj->client_link, &client->objects_list);
+	spin_unlock_irqrestore(&client->objects_lock, flags);
+}
+
+void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
+{
+	struct i915_drm_client *client = fetch_and_zero(&obj->client);
+	unsigned long flags;
+
+	/* Object may not be associated with a client. */
+	if (!client || list_empty(&obj->client_link))
+		return;
+
+	spin_lock_irqsave(&client->objects_lock, flags);
+	list_del(&obj->client_link);
+	spin_unlock_irqrestore(&client->objects_lock, flags);
+
+	i915_drm_client_put(client);
+}
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 4c18b99e10a4..dfeaaf204c00 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -12,6 +12,9 @@
 
 #include <uapi/drm/i915_drm.h>
 
+#include "i915_file_private.h"
+#include "gem/i915_gem_object_types.h"
+
 #define I915_LAST_UABI_ENGINE_CLASS I915_ENGINE_CLASS_COMPUTE
 
 struct drm_file;
@@ -25,6 +28,22 @@ struct i915_drm_client {
 	spinlock_t ctx_lock; /* For add/remove from ctx_list. */
 	struct list_head ctx_list; /* List of contexts belonging to client. */
 
+#ifdef CONFIG_PROC_FS
+	struct drm_i915_file_private *fpriv;
+
+	/**
+	 * @objects_lock: lock protecting @objects_list
+	 */
+	spinlock_t objects_lock;
+
+	/**
+	 * @objects_list: list of objects created by this client
+	 *
+	 * Protected by @objects_lock.
+	 */
+	struct list_head objects_list;
+#endif
+
 	/**
 	 * @past_runtime: Accumulation of pphwsp runtimes from closed contexts.
 	 */
@@ -45,10 +64,25 @@ static inline void i915_drm_client_put(struct i915_drm_client *client)
 	kref_put(&client->kref, __i915_drm_client_free);
 }
 
-struct i915_drm_client *i915_drm_client_alloc(void);
+struct i915_drm_client *i915_drm_client_alloc(struct drm_i915_file_private *fpriv);
 
 #ifdef CONFIG_PROC_FS
 void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
+
+void i915_drm_client_add_object(struct i915_drm_client *client,
+				struct drm_i915_gem_object *obj);
+void i915_drm_client_remove_object(struct drm_i915_gem_object *obj);
+#else
+static inline void i915_drm_client_add_object(struct i915_drm_client *client,
+					      struct drm_i915_gem_object *obj)
+{
+
+}
+
+static inline void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
+{
+
+}
 #endif
 
 #endif /* !__I915_DRM_CLIENT_H__ */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1f65bb33dd21..7ae42f746cc2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1325,7 +1325,7 @@ int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file)
 	if (!file_priv)
 		goto err_alloc;
 
-	client = i915_drm_client_alloc();
+	client = i915_drm_client_alloc(file_priv);
 	if (!client)
 		goto err_client;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH 2/8] drm/i915: Track buffer objects belonging to clients
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

In order to show per client memory usage lets start tracking which
objects belong to which clients.

We start with objects explicitly created by object creation UAPI and
track it on a new per client lists, protected by a new per client lock.
In order for delayed destruction (post client exit), we make tracked
objects hold references to the owning client.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_create.c    | 32 +++++++++++++--
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |  6 +++
 .../gpu/drm/i915/gem/i915_gem_object_types.h  | 12 ++++++
 drivers/gpu/drm/i915/i915_drm_client.c        | 39 ++++++++++++++++++-
 drivers/gpu/drm/i915/i915_drm_client.h        | 36 ++++++++++++++++-
 drivers/gpu/drm/i915/i915_gem.c               |  2 +-
 6 files changed, 121 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
index d24c0ce8805c..4f1957638207 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -11,6 +11,7 @@
 #include "gem/i915_gem_region.h"
 #include "pxp/intel_pxp.h"
 
+#include "i915_drm_client.h"
 #include "i915_drv.h"
 #include "i915_gem_create.h"
 #include "i915_trace.h"
@@ -164,6 +165,14 @@ __i915_gem_object_create_user(struct drm_i915_private *i915, u64 size,
 						 n_placements, 0);
 }
 
+static void add_file_obj(struct drm_file *file,
+			 struct drm_i915_gem_object *obj)
+{
+	struct drm_i915_file_private *fpriv = file->driver_priv;
+
+	i915_drm_client_add_object(fpriv->client, obj);
+}
+
 int
 i915_gem_dumb_create(struct drm_file *file,
 		     struct drm_device *dev,
@@ -174,6 +183,7 @@ i915_gem_dumb_create(struct drm_file *file,
 	enum intel_memory_type mem_type;
 	int cpp = DIV_ROUND_UP(args->bpp, 8);
 	u32 format;
+	int ret;
 
 	switch (cpp) {
 	case 1:
@@ -212,7 +222,12 @@ i915_gem_dumb_create(struct drm_file *file,
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	return i915_gem_publish(obj, file, &args->size, &args->handle);
+	ret = i915_gem_publish(obj, file, &args->size, &args->handle);
+
+	if (!ret)
+		add_file_obj(file, obj);
+
+	return ret;
 }
 
 /**
@@ -229,6 +244,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_gem_create *args = data;
 	struct drm_i915_gem_object *obj;
 	struct intel_memory_region *mr;
+	int ret;
 
 	mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
 
@@ -236,7 +252,12 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
 	if (IS_ERR(obj))
 		return PTR_ERR(obj);
 
-	return i915_gem_publish(obj, file, &args->size, &args->handle);
+	ret = i915_gem_publish(obj, file, &args->size, &args->handle);
+
+	if (!ret)
+		add_file_obj(file, obj);
+
+	return ret;
 }
 
 struct create_ext {
@@ -494,5 +515,10 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data,
 		obj->pat_set_by_user = true;
 	}
 
-	return i915_gem_publish(obj, file, &args->size, &args->handle);
+	ret = i915_gem_publish(obj, file, &args->size, &args->handle);
+
+	if (!ret)
+		add_file_obj(file, obj);
+
+	return ret;
 }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 97ac6fb37958..46de9b1b3f1d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -105,6 +105,10 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
 
 	INIT_LIST_HEAD(&obj->mm.link);
 
+#ifdef CONFIG_PROC_FS
+	INIT_LIST_HEAD(&obj->client_link);
+#endif
+
 	INIT_LIST_HEAD(&obj->lut_list);
 	spin_lock_init(&obj->lut_lock);
 
@@ -441,6 +445,8 @@ static void i915_gem_free_object(struct drm_gem_object *gem_obj)
 
 	GEM_BUG_ON(i915_gem_object_is_framebuffer(obj));
 
+	i915_drm_client_remove_object(obj);
+
 	/*
 	 * Before we free the object, make sure any pure RCU-only
 	 * read-side critical sections are complete, e.g.
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index e72c57716bee..8de2b91b3edf 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -300,6 +300,18 @@ struct drm_i915_gem_object {
 	 */
 	struct i915_address_space *shares_resv_from;
 
+#ifdef CONFIG_PROC_FS
+	/**
+	 * @client: @i915_drm_client which created the object
+	 */
+	struct i915_drm_client *client;
+
+	/**
+	 * @client_link: Link into @i915_drm_client.objects_list
+	 */
+	struct list_head client_link;
+#endif
+
 	union {
 		struct rcu_head rcu;
 		struct llist_node freed;
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 2a44b3876cb5..b0b35bcdd2b3 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -17,7 +17,8 @@
 #include "i915_gem.h"
 #include "i915_utils.h"
 
-struct i915_drm_client *i915_drm_client_alloc(void)
+struct i915_drm_client *
+i915_drm_client_alloc(struct drm_i915_file_private *fpriv)
 {
 	struct i915_drm_client *client;
 
@@ -28,6 +29,12 @@ struct i915_drm_client *i915_drm_client_alloc(void)
 	kref_init(&client->kref);
 	spin_lock_init(&client->ctx_lock);
 	INIT_LIST_HEAD(&client->ctx_list);
+#ifdef CONFIG_PROC_FS
+	spin_lock_init(&client->objects_lock);
+	INIT_LIST_HEAD(&client->objects_list);
+
+	client->fpriv = fpriv;
+#endif
 
 	return client;
 }
@@ -108,4 +115,34 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file)
 	for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++)
 		show_client_class(p, i915, file_priv->client, i);
 }
+
+void i915_drm_client_add_object(struct i915_drm_client *client,
+				struct drm_i915_gem_object *obj)
+{
+	unsigned long flags;
+
+	GEM_WARN_ON(obj->client);
+	GEM_WARN_ON(!list_empty(&obj->client_link));
+
+	spin_lock_irqsave(&client->objects_lock, flags);
+	obj->client = i915_drm_client_get(client);
+	list_add_tail(&obj->client_link, &client->objects_list);
+	spin_unlock_irqrestore(&client->objects_lock, flags);
+}
+
+void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
+{
+	struct i915_drm_client *client = fetch_and_zero(&obj->client);
+	unsigned long flags;
+
+	/* Object may not be associated with a client. */
+	if (!client || list_empty(&obj->client_link))
+		return;
+
+	spin_lock_irqsave(&client->objects_lock, flags);
+	list_del(&obj->client_link);
+	spin_unlock_irqrestore(&client->objects_lock, flags);
+
+	i915_drm_client_put(client);
+}
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 4c18b99e10a4..dfeaaf204c00 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -12,6 +12,9 @@
 
 #include <uapi/drm/i915_drm.h>
 
+#include "i915_file_private.h"
+#include "gem/i915_gem_object_types.h"
+
 #define I915_LAST_UABI_ENGINE_CLASS I915_ENGINE_CLASS_COMPUTE
 
 struct drm_file;
@@ -25,6 +28,22 @@ struct i915_drm_client {
 	spinlock_t ctx_lock; /* For add/remove from ctx_list. */
 	struct list_head ctx_list; /* List of contexts belonging to client. */
 
+#ifdef CONFIG_PROC_FS
+	struct drm_i915_file_private *fpriv;
+
+	/**
+	 * @objects_lock: lock protecting @objects_list
+	 */
+	spinlock_t objects_lock;
+
+	/**
+	 * @objects_list: list of objects created by this client
+	 *
+	 * Protected by @objects_lock.
+	 */
+	struct list_head objects_list;
+#endif
+
 	/**
 	 * @past_runtime: Accumulation of pphwsp runtimes from closed contexts.
 	 */
@@ -45,10 +64,25 @@ static inline void i915_drm_client_put(struct i915_drm_client *client)
 	kref_put(&client->kref, __i915_drm_client_free);
 }
 
-struct i915_drm_client *i915_drm_client_alloc(void);
+struct i915_drm_client *i915_drm_client_alloc(struct drm_i915_file_private *fpriv);
 
 #ifdef CONFIG_PROC_FS
 void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
+
+void i915_drm_client_add_object(struct i915_drm_client *client,
+				struct drm_i915_gem_object *obj);
+void i915_drm_client_remove_object(struct drm_i915_gem_object *obj);
+#else
+static inline void i915_drm_client_add_object(struct i915_drm_client *client,
+					      struct drm_i915_gem_object *obj)
+{
+
+}
+
+static inline void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
+{
+
+}
 #endif
 
 #endif /* !__I915_DRM_CLIENT_H__ */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1f65bb33dd21..7ae42f746cc2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1325,7 +1325,7 @@ int i915_gem_open(struct drm_i915_private *i915, struct drm_file *file)
 	if (!file_priv)
 		goto err_alloc;
 
-	client = i915_drm_client_alloc();
+	client = i915_drm_client_alloc(file_priv);
 	if (!client)
 		goto err_client;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH 3/8] drm/i915: Record which clients own a VM
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

To enable accounting of indirect client memory usage (such as page tables)
in the following patch, lets start recording the creator of each PPGTT.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c       | 11 ++++++++---
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  3 +++
 drivers/gpu/drm/i915/gem/selftests/mock_context.c |  4 ++--
 drivers/gpu/drm/i915/gt/intel_gtt.h               |  1 +
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 9a9ff84c90d7..35cf6608180e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -279,7 +279,8 @@ static int proto_context_set_protected(struct drm_i915_private *i915,
 }
 
 static struct i915_gem_proto_context *
-proto_context_create(struct drm_i915_private *i915, unsigned int flags)
+proto_context_create(struct drm_i915_file_private *fpriv,
+		     struct drm_i915_private *i915, unsigned int flags)
 {
 	struct i915_gem_proto_context *pc, *err;
 
@@ -287,6 +288,7 @@ proto_context_create(struct drm_i915_private *i915, unsigned int flags)
 	if (!pc)
 		return ERR_PTR(-ENOMEM);
 
+	pc->fpriv = fpriv;
 	pc->num_user_engines = -1;
 	pc->user_engines = NULL;
 	pc->user_flags = BIT(UCONTEXT_BANNABLE) |
@@ -1621,6 +1623,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
 			err = PTR_ERR(ppgtt);
 			goto err_ctx;
 		}
+		ppgtt->vm.fpriv = pc->fpriv;
 		vm = &ppgtt->vm;
 	}
 	if (vm)
@@ -1740,7 +1743,7 @@ int i915_gem_context_open(struct drm_i915_private *i915,
 	/* 0 reserved for invalid/unassigned ppgtt */
 	xa_init_flags(&file_priv->vm_xa, XA_FLAGS_ALLOC1);
 
-	pc = proto_context_create(i915, 0);
+	pc = proto_context_create(file_priv, i915, 0);
 	if (IS_ERR(pc)) {
 		err = PTR_ERR(pc);
 		goto err;
@@ -1822,6 +1825,7 @@ int i915_gem_vm_create_ioctl(struct drm_device *dev, void *data,
 
 	GEM_BUG_ON(id == 0); /* reserved for invalid/unassigned ppgtt */
 	args->vm_id = id;
+	ppgtt->vm.fpriv = file_priv;
 	return 0;
 
 err_put:
@@ -2284,7 +2288,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 		return -EIO;
 	}
 
-	ext_data.pc = proto_context_create(i915, args->flags);
+	ext_data.pc = proto_context_create(file->driver_priv, i915,
+					   args->flags);
 	if (IS_ERR(ext_data.pc))
 		return PTR_ERR(ext_data.pc);
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index cb78214a7dcd..c573c067779f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -188,6 +188,9 @@ struct i915_gem_proto_engine {
  * CONTEXT_CREATE_SET_PARAM during GEM_CONTEXT_CREATE.
  */
 struct i915_gem_proto_context {
+	/** @fpriv: Client which creates the context */
+	struct drm_i915_file_private *fpriv;
+
 	/** @vm: See &i915_gem_context.vm */
 	struct i915_address_space *vm;
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 8ac6726ec16b..125584ada282 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -83,7 +83,7 @@ live_context(struct drm_i915_private *i915, struct file *file)
 	int err;
 	u32 id;
 
-	pc = proto_context_create(i915, 0);
+	pc = proto_context_create(fpriv, i915, 0);
 	if (IS_ERR(pc))
 		return ERR_CAST(pc);
 
@@ -152,7 +152,7 @@ kernel_context(struct drm_i915_private *i915,
 	struct i915_gem_context *ctx;
 	struct i915_gem_proto_context *pc;
 
-	pc = proto_context_create(i915, 0);
+	pc = proto_context_create(NULL, i915, 0);
 	if (IS_ERR(pc))
 		return ERR_CAST(pc);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 4d6296cdbcfd..7192a534a654 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -248,6 +248,7 @@ struct i915_address_space {
 	struct drm_mm mm;
 	struct intel_gt *gt;
 	struct drm_i915_private *i915;
+	struct drm_i915_file_private *fpriv;
 	struct device *dma;
 	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
 	u64 reserved;		/* size addr space reserved */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH 3/8] drm/i915: Record which clients own a VM
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

To enable accounting of indirect client memory usage (such as page tables)
in the following patch, lets start recording the creator of each PPGTT.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c       | 11 ++++++++---
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h |  3 +++
 drivers/gpu/drm/i915/gem/selftests/mock_context.c |  4 ++--
 drivers/gpu/drm/i915/gt/intel_gtt.h               |  1 +
 4 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 9a9ff84c90d7..35cf6608180e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -279,7 +279,8 @@ static int proto_context_set_protected(struct drm_i915_private *i915,
 }
 
 static struct i915_gem_proto_context *
-proto_context_create(struct drm_i915_private *i915, unsigned int flags)
+proto_context_create(struct drm_i915_file_private *fpriv,
+		     struct drm_i915_private *i915, unsigned int flags)
 {
 	struct i915_gem_proto_context *pc, *err;
 
@@ -287,6 +288,7 @@ proto_context_create(struct drm_i915_private *i915, unsigned int flags)
 	if (!pc)
 		return ERR_PTR(-ENOMEM);
 
+	pc->fpriv = fpriv;
 	pc->num_user_engines = -1;
 	pc->user_engines = NULL;
 	pc->user_flags = BIT(UCONTEXT_BANNABLE) |
@@ -1621,6 +1623,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
 			err = PTR_ERR(ppgtt);
 			goto err_ctx;
 		}
+		ppgtt->vm.fpriv = pc->fpriv;
 		vm = &ppgtt->vm;
 	}
 	if (vm)
@@ -1740,7 +1743,7 @@ int i915_gem_context_open(struct drm_i915_private *i915,
 	/* 0 reserved for invalid/unassigned ppgtt */
 	xa_init_flags(&file_priv->vm_xa, XA_FLAGS_ALLOC1);
 
-	pc = proto_context_create(i915, 0);
+	pc = proto_context_create(file_priv, i915, 0);
 	if (IS_ERR(pc)) {
 		err = PTR_ERR(pc);
 		goto err;
@@ -1822,6 +1825,7 @@ int i915_gem_vm_create_ioctl(struct drm_device *dev, void *data,
 
 	GEM_BUG_ON(id == 0); /* reserved for invalid/unassigned ppgtt */
 	args->vm_id = id;
+	ppgtt->vm.fpriv = file_priv;
 	return 0;
 
 err_put:
@@ -2284,7 +2288,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 		return -EIO;
 	}
 
-	ext_data.pc = proto_context_create(i915, args->flags);
+	ext_data.pc = proto_context_create(file->driver_priv, i915,
+					   args->flags);
 	if (IS_ERR(ext_data.pc))
 		return PTR_ERR(ext_data.pc);
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index cb78214a7dcd..c573c067779f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -188,6 +188,9 @@ struct i915_gem_proto_engine {
  * CONTEXT_CREATE_SET_PARAM during GEM_CONTEXT_CREATE.
  */
 struct i915_gem_proto_context {
+	/** @fpriv: Client which creates the context */
+	struct drm_i915_file_private *fpriv;
+
 	/** @vm: See &i915_gem_context.vm */
 	struct i915_address_space *vm;
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 8ac6726ec16b..125584ada282 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -83,7 +83,7 @@ live_context(struct drm_i915_private *i915, struct file *file)
 	int err;
 	u32 id;
 
-	pc = proto_context_create(i915, 0);
+	pc = proto_context_create(fpriv, i915, 0);
 	if (IS_ERR(pc))
 		return ERR_CAST(pc);
 
@@ -152,7 +152,7 @@ kernel_context(struct drm_i915_private *i915,
 	struct i915_gem_context *ctx;
 	struct i915_gem_proto_context *pc;
 
-	pc = proto_context_create(i915, 0);
+	pc = proto_context_create(NULL, i915, 0);
 	if (IS_ERR(pc))
 		return ERR_CAST(pc);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 4d6296cdbcfd..7192a534a654 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -248,6 +248,7 @@ struct i915_address_space {
 	struct drm_mm mm;
 	struct intel_gt *gt;
 	struct drm_i915_private *i915;
+	struct drm_i915_file_private *fpriv;
 	struct device *dma;
 	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
 	u64 reserved;		/* size addr space reserved */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH 4/8] drm/i915: Track page table backing store usage
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Account page table backing store against the owning client memory usage
stats.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gtt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 2f6a9be0ffe6..126269a0d728 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -58,6 +58,9 @@ struct drm_i915_gem_object *alloc_pt_lmem(struct i915_address_space *vm, int sz)
 	if (!IS_ERR(obj)) {
 		obj->base.resv = i915_vm_resv_get(vm);
 		obj->shares_resv_from = vm;
+
+		if (vm->fpriv)
+			i915_drm_client_add_object(vm->fpriv->client, obj);
 	}
 
 	return obj;
@@ -79,6 +82,9 @@ struct drm_i915_gem_object *alloc_pt_dma(struct i915_address_space *vm, int sz)
 	if (!IS_ERR(obj)) {
 		obj->base.resv = i915_vm_resv_get(vm);
 		obj->shares_resv_from = vm;
+
+		if (vm->fpriv)
+			i915_drm_client_add_object(vm->fpriv->client, obj);
 	}
 
 	return obj;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH 4/8] drm/i915: Track page table backing store usage
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Account page table backing store against the owning client memory usage
stats.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gtt.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 2f6a9be0ffe6..126269a0d728 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -58,6 +58,9 @@ struct drm_i915_gem_object *alloc_pt_lmem(struct i915_address_space *vm, int sz)
 	if (!IS_ERR(obj)) {
 		obj->base.resv = i915_vm_resv_get(vm);
 		obj->shares_resv_from = vm;
+
+		if (vm->fpriv)
+			i915_drm_client_add_object(vm->fpriv->client, obj);
 	}
 
 	return obj;
@@ -79,6 +82,9 @@ struct drm_i915_gem_object *alloc_pt_dma(struct i915_address_space *vm, int sz)
 	if (!IS_ERR(obj)) {
 		obj->base.resv = i915_vm_resv_get(vm);
 		obj->shares_resv_from = vm;
+
+		if (vm->fpriv)
+			i915_drm_client_add_object(vm->fpriv->client, obj);
 	}
 
 	return obj;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH 5/8] drm/i915: Account ring buffer and context state storage
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Account ring buffers and logical context space against the owning client
memory usage stats.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c |  6 ++++++
 drivers/gpu/drm/i915/i915_drm_client.c      | 10 ++++++++++
 drivers/gpu/drm/i915/i915_drm_client.h      |  9 +++++++++
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 35cf6608180e..3f4c74aed3c5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1703,6 +1703,8 @@ static void gem_context_register(struct i915_gem_context *ctx,
 				 u32 id)
 {
 	struct drm_i915_private *i915 = ctx->i915;
+	struct i915_gem_engines_iter it;
+	struct intel_context *ce;
 	void *old;
 
 	ctx->file_priv = fpriv;
@@ -1721,6 +1723,10 @@ static void gem_context_register(struct i915_gem_context *ctx,
 	list_add_tail(&ctx->link, &i915->gem.contexts.list);
 	spin_unlock(&i915->gem.contexts.lock);
 
+	for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it)
+		i915_drm_client_add_context(fpriv->client, ce);
+	i915_gem_context_unlock_engines(ctx);
+
 	/* And finally expose ourselves to userspace via the idr */
 	old = xa_store(&fpriv->context_xa, id, ctx, GFP_KERNEL);
 	WARN_ON(old);
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index b0b35bcdd2b3..31316edbf30b 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -145,4 +145,14 @@ void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
 
 	i915_drm_client_put(client);
 }
+
+void i915_drm_client_add_context(struct i915_drm_client *client,
+				 struct intel_context *ce)
+{
+	if (ce->state)
+		i915_drm_client_add_object(client, ce->state->obj);
+
+	if (ce->ring != ce->engine->legacy.ring && ce->ring->vma)
+		i915_drm_client_add_object(client, ce->ring->vma->obj);
+}
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index dfeaaf204c00..e1e2a7cca1b1 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -14,6 +14,7 @@
 
 #include "i915_file_private.h"
 #include "gem/i915_gem_object_types.h"
+#include "gt/intel_context_types.h"
 
 #define I915_LAST_UABI_ENGINE_CLASS I915_ENGINE_CLASS_COMPUTE
 
@@ -72,6 +73,8 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
 void i915_drm_client_add_object(struct i915_drm_client *client,
 				struct drm_i915_gem_object *obj);
 void i915_drm_client_remove_object(struct drm_i915_gem_object *obj);
+void i915_drm_client_add_context(struct i915_drm_client *client,
+				 struct intel_context *ce);
 #else
 static inline void i915_drm_client_add_object(struct i915_drm_client *client,
 					      struct drm_i915_gem_object *obj)
@@ -82,6 +85,12 @@ static inline void i915_drm_client_add_object(struct i915_drm_client *client,
 static inline void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
 {
 
+}
+
+static inline void i915_drm_client_add_context(struct i915_drm_client *client,
+					       struct intel_context *ce)
+{
+
 }
 #endif
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH 5/8] drm/i915: Account ring buffer and context state storage
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Account ring buffers and logical context space against the owning client
memory usage stats.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c |  6 ++++++
 drivers/gpu/drm/i915/i915_drm_client.c      | 10 ++++++++++
 drivers/gpu/drm/i915/i915_drm_client.h      |  9 +++++++++
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 35cf6608180e..3f4c74aed3c5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1703,6 +1703,8 @@ static void gem_context_register(struct i915_gem_context *ctx,
 				 u32 id)
 {
 	struct drm_i915_private *i915 = ctx->i915;
+	struct i915_gem_engines_iter it;
+	struct intel_context *ce;
 	void *old;
 
 	ctx->file_priv = fpriv;
@@ -1721,6 +1723,10 @@ static void gem_context_register(struct i915_gem_context *ctx,
 	list_add_tail(&ctx->link, &i915->gem.contexts.list);
 	spin_unlock(&i915->gem.contexts.lock);
 
+	for_each_gem_engine(ce, i915_gem_context_lock_engines(ctx), it)
+		i915_drm_client_add_context(fpriv->client, ce);
+	i915_gem_context_unlock_engines(ctx);
+
 	/* And finally expose ourselves to userspace via the idr */
 	old = xa_store(&fpriv->context_xa, id, ctx, GFP_KERNEL);
 	WARN_ON(old);
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index b0b35bcdd2b3..31316edbf30b 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -145,4 +145,14 @@ void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
 
 	i915_drm_client_put(client);
 }
+
+void i915_drm_client_add_context(struct i915_drm_client *client,
+				 struct intel_context *ce)
+{
+	if (ce->state)
+		i915_drm_client_add_object(client, ce->state->obj);
+
+	if (ce->ring != ce->engine->legacy.ring && ce->ring->vma)
+		i915_drm_client_add_object(client, ce->ring->vma->obj);
+}
 #endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index dfeaaf204c00..e1e2a7cca1b1 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.h
+++ b/drivers/gpu/drm/i915/i915_drm_client.h
@@ -14,6 +14,7 @@
 
 #include "i915_file_private.h"
 #include "gem/i915_gem_object_types.h"
+#include "gt/intel_context_types.h"
 
 #define I915_LAST_UABI_ENGINE_CLASS I915_ENGINE_CLASS_COMPUTE
 
@@ -72,6 +73,8 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
 void i915_drm_client_add_object(struct i915_drm_client *client,
 				struct drm_i915_gem_object *obj);
 void i915_drm_client_remove_object(struct drm_i915_gem_object *obj);
+void i915_drm_client_add_context(struct i915_drm_client *client,
+				 struct intel_context *ce);
 #else
 static inline void i915_drm_client_add_object(struct i915_drm_client *client,
 					      struct drm_i915_gem_object *obj)
@@ -82,6 +85,12 @@ static inline void i915_drm_client_add_object(struct i915_drm_client *client,
 static inline void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
 {
 
+}
+
+static inline void i915_drm_client_add_context(struct i915_drm_client *client,
+					       struct intel_context *ce)
+{
+
 }
 #endif
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
will return a reference to a newly created GEM objects (if created), in
order to enable tracking of imported i915 GEM objects in the following
patch.

Minor code reshuffule and only trivial additions on top of
drm_gem_prime_fd_to_handle.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/drm_prime.c | 41 ++++++++++++++++++++++++++++++++-----
 include/drm/drm_prime.h     |  4 ++++
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index d29dafce9bb0..ef75f67e3057 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -284,11 +284,12 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
 EXPORT_SYMBOL(drm_gem_dmabuf_release);
 
 /**
- * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
+ * drm_gem_prime_fd_to_handle_obj - PRIME import function for GEM drivers
  * @dev: drm_device to import into
  * @file_priv: drm file-private structure
  * @prime_fd: fd id of the dma-buf which should be imported
  * @handle: pointer to storage for the handle of the imported buffer object
+ * @objp: optional pointer in which reference to created GEM object can be returned
  *
  * This is the PRIME import function which must be used mandatorily by GEM
  * drivers to ensure correct lifetime management of the underlying GEM object.
@@ -297,9 +298,10 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
  *
  * Returns 0 on success or a negative error code on failure.
  */
-int drm_gem_prime_fd_to_handle(struct drm_device *dev,
-			       struct drm_file *file_priv, int prime_fd,
-			       uint32_t *handle)
+int drm_gem_prime_fd_to_handle_obj(struct drm_device *dev,
+				   struct drm_file *file_priv, int prime_fd,
+				   uint32_t *handle,
+				   struct drm_gem_object **objp)
 {
 	struct dma_buf *dma_buf;
 	struct drm_gem_object *obj;
@@ -336,7 +338,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 
 	/* _handle_create_tail unconditionally unlocks dev->object_name_lock. */
 	ret = drm_gem_handle_create_tail(file_priv, obj, handle);
-	drm_gem_object_put(obj);
+	if (!objp)
+		drm_gem_object_put(obj);
 	if (ret)
 		goto out_put;
 
@@ -348,6 +351,9 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 
 	dma_buf_put(dma_buf);
 
+	if (objp)
+		*objp = obj;
+
 	return 0;
 
 fail:
@@ -356,6 +362,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 	 */
 	drm_gem_handle_delete(file_priv, *handle);
 	dma_buf_put(dma_buf);
+	if (objp)
+		drm_gem_object_put(obj);
 	return ret;
 
 out_unlock:
@@ -365,6 +373,29 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 	dma_buf_put(dma_buf);
 	return ret;
 }
+EXPORT_SYMBOL(drm_gem_prime_fd_to_handle_obj);
+
+/**
+ * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
+ * @dev: drm_device to import into
+ * @file_priv: drm file-private structure
+ * @prime_fd: fd id of the dma-buf which should be imported
+ * @handle: pointer to storage for the handle of the imported buffer object
+ *
+ * This is the PRIME import function which must be used mandatorily by GEM
+ * drivers to ensure correct lifetime management of the underlying GEM object.
+ * The actual importing of GEM object from the dma-buf is done through the
+ * &drm_driver.gem_prime_import driver callback.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+			       struct drm_file *file_priv, int prime_fd,
+			       uint32_t *handle)
+{
+	return drm_gem_prime_fd_to_handle_obj(dev, file_priv, prime_fd, handle,
+					      NULL);
+}
 EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
 
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 2a1d01e5b56b..10d145ce6586 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -69,6 +69,10 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
 
 int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 			       struct drm_file *file_priv, int prime_fd, uint32_t *handle);
+int drm_gem_prime_fd_to_handle_obj(struct drm_device *dev,
+				   struct drm_file *file_priv, int prime_fd,
+				   uint32_t *handle,
+				   struct drm_gem_object **obj);
 int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 			       struct drm_file *file_priv, uint32_t handle, uint32_t flags,
 			       int *prime_fd);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
will return a reference to a newly created GEM objects (if created), in
order to enable tracking of imported i915 GEM objects in the following
patch.

Minor code reshuffule and only trivial additions on top of
drm_gem_prime_fd_to_handle.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/drm_prime.c | 41 ++++++++++++++++++++++++++++++++-----
 include/drm/drm_prime.h     |  4 ++++
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index d29dafce9bb0..ef75f67e3057 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -284,11 +284,12 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
 EXPORT_SYMBOL(drm_gem_dmabuf_release);
 
 /**
- * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
+ * drm_gem_prime_fd_to_handle_obj - PRIME import function for GEM drivers
  * @dev: drm_device to import into
  * @file_priv: drm file-private structure
  * @prime_fd: fd id of the dma-buf which should be imported
  * @handle: pointer to storage for the handle of the imported buffer object
+ * @objp: optional pointer in which reference to created GEM object can be returned
  *
  * This is the PRIME import function which must be used mandatorily by GEM
  * drivers to ensure correct lifetime management of the underlying GEM object.
@@ -297,9 +298,10 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
  *
  * Returns 0 on success or a negative error code on failure.
  */
-int drm_gem_prime_fd_to_handle(struct drm_device *dev,
-			       struct drm_file *file_priv, int prime_fd,
-			       uint32_t *handle)
+int drm_gem_prime_fd_to_handle_obj(struct drm_device *dev,
+				   struct drm_file *file_priv, int prime_fd,
+				   uint32_t *handle,
+				   struct drm_gem_object **objp)
 {
 	struct dma_buf *dma_buf;
 	struct drm_gem_object *obj;
@@ -336,7 +338,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 
 	/* _handle_create_tail unconditionally unlocks dev->object_name_lock. */
 	ret = drm_gem_handle_create_tail(file_priv, obj, handle);
-	drm_gem_object_put(obj);
+	if (!objp)
+		drm_gem_object_put(obj);
 	if (ret)
 		goto out_put;
 
@@ -348,6 +351,9 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 
 	dma_buf_put(dma_buf);
 
+	if (objp)
+		*objp = obj;
+
 	return 0;
 
 fail:
@@ -356,6 +362,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 	 */
 	drm_gem_handle_delete(file_priv, *handle);
 	dma_buf_put(dma_buf);
+	if (objp)
+		drm_gem_object_put(obj);
 	return ret;
 
 out_unlock:
@@ -365,6 +373,29 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 	dma_buf_put(dma_buf);
 	return ret;
 }
+EXPORT_SYMBOL(drm_gem_prime_fd_to_handle_obj);
+
+/**
+ * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
+ * @dev: drm_device to import into
+ * @file_priv: drm file-private structure
+ * @prime_fd: fd id of the dma-buf which should be imported
+ * @handle: pointer to storage for the handle of the imported buffer object
+ *
+ * This is the PRIME import function which must be used mandatorily by GEM
+ * drivers to ensure correct lifetime management of the underlying GEM object.
+ * The actual importing of GEM object from the dma-buf is done through the
+ * &drm_driver.gem_prime_import driver callback.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_gem_prime_fd_to_handle(struct drm_device *dev,
+			       struct drm_file *file_priv, int prime_fd,
+			       uint32_t *handle)
+{
+	return drm_gem_prime_fd_to_handle_obj(dev, file_priv, prime_fd, handle,
+					      NULL);
+}
 EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
 
 int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 2a1d01e5b56b..10d145ce6586 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -69,6 +69,10 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
 
 int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 			       struct drm_file *file_priv, int prime_fd, uint32_t *handle);
+int drm_gem_prime_fd_to_handle_obj(struct drm_device *dev,
+				   struct drm_file *file_priv, int prime_fd,
+				   uint32_t *handle,
+				   struct drm_gem_object **obj);
 int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 			       struct drm_file *file_priv, uint32_t handle, uint32_t flags,
 			       int *prime_fd);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH 7/8] drm/i915: Track imported dma-buf objects in memory stats
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Aravind Iddamsetty, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We want to be able to show memory usage of imported dma-buf opjects in the
fdinfo stats.

To achieve this we wrap drm_gem_prime_fd_to_handle(_obj) in
i915_gem_prime_fd_to_handle and append some client management at the end.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 32 ++++++++++++++++++++++
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h |  7 +++++
 drivers/gpu/drm/i915/i915_driver.c         |  2 +-
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index fd556a076d05..2e2d9d7c1992 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -11,7 +11,10 @@
 
 #include <asm/smp.h>
 
+#include <drm/drm_file.h>
+
 #include "gem/i915_gem_dmabuf.h"
+#include "i915_drm_client.h"
 #include "i915_drv.h"
 #include "i915_gem_object.h"
 #include "i915_scatterlist.h"
@@ -344,6 +347,35 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
 	return ERR_PTR(ret);
 }
 
+int i915_gem_prime_fd_to_handle(struct drm_device *dev,
+				struct drm_file *file_priv, int prime_fd,
+				uint32_t *handle)
+{
+	struct drm_gem_object *gem_obj = NULL;
+	int ret;
+
+	if (IS_ENABLED(CONFIG_PROC_FS))
+		ret = drm_gem_prime_fd_to_handle_obj(dev, file_priv, prime_fd,
+						     handle, &gem_obj);
+	else
+		ret = drm_gem_prime_fd_to_handle(dev, file_priv, prime_fd,
+						 handle);
+	if (ret)
+		return ret;
+
+	if (gem_obj) {
+		struct drm_i915_file_private *fpriv = file_priv->driver_priv;
+		struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
+
+		/* Really imported and not just alias? */
+		if (obj->ops == &i915_gem_object_dmabuf_ops)
+			i915_drm_client_add_object(fpriv->client, obj);
+		i915_gem_object_put(obj);
+	}
+
+	return 0;
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/mock_dmabuf.c"
 #include "selftests/i915_gem_dmabuf.c"
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h
index 6e0405d47ce1..63635c221c7c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h
@@ -6,8 +6,11 @@
 #ifndef __I915_GEM_DMABUF_H__
 #define __I915_GEM_DMABUF_H__
 
+#include <linux/types.h>
+
 struct drm_gem_object;
 struct drm_device;
+struct drm_file;
 struct dma_buf;
 
 struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
@@ -15,4 +18,8 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
 
 struct dma_buf *i915_gem_prime_export(struct drm_gem_object *gem_obj, int flags);
 
+int i915_gem_prime_fd_to_handle(struct drm_device *dev,
+				struct drm_file *file_priv, int prime_fd,
+				uint32_t *handle);
+
 #endif /* __I915_GEM_DMABUF_H__ */
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index ace8534b6cc5..03f3157371bf 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1806,7 +1806,7 @@ static const struct drm_driver i915_drm_driver = {
 	.show_fdinfo = i915_drm_client_fdinfo,
 
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+	.prime_fd_to_handle = i915_gem_prime_fd_to_handle,
 	.gem_prime_import = i915_gem_prime_import,
 
 	.dumb_create = i915_gem_dumb_create,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH 7/8] drm/i915: Track imported dma-buf objects in memory stats
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

We want to be able to show memory usage of imported dma-buf opjects in the
fdinfo stats.

To achieve this we wrap drm_gem_prime_fd_to_handle(_obj) in
i915_gem_prime_fd_to_handle and append some client management at the end.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c | 32 ++++++++++++++++++++++
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h |  7 +++++
 drivers/gpu/drm/i915/i915_driver.c         |  2 +-
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index fd556a076d05..2e2d9d7c1992 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -11,7 +11,10 @@
 
 #include <asm/smp.h>
 
+#include <drm/drm_file.h>
+
 #include "gem/i915_gem_dmabuf.h"
+#include "i915_drm_client.h"
 #include "i915_drv.h"
 #include "i915_gem_object.h"
 #include "i915_scatterlist.h"
@@ -344,6 +347,35 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
 	return ERR_PTR(ret);
 }
 
+int i915_gem_prime_fd_to_handle(struct drm_device *dev,
+				struct drm_file *file_priv, int prime_fd,
+				uint32_t *handle)
+{
+	struct drm_gem_object *gem_obj = NULL;
+	int ret;
+
+	if (IS_ENABLED(CONFIG_PROC_FS))
+		ret = drm_gem_prime_fd_to_handle_obj(dev, file_priv, prime_fd,
+						     handle, &gem_obj);
+	else
+		ret = drm_gem_prime_fd_to_handle(dev, file_priv, prime_fd,
+						 handle);
+	if (ret)
+		return ret;
+
+	if (gem_obj) {
+		struct drm_i915_file_private *fpriv = file_priv->driver_priv;
+		struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
+
+		/* Really imported and not just alias? */
+		if (obj->ops == &i915_gem_object_dmabuf_ops)
+			i915_drm_client_add_object(fpriv->client, obj);
+		i915_gem_object_put(obj);
+	}
+
+	return 0;
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/mock_dmabuf.c"
 #include "selftests/i915_gem_dmabuf.c"
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h
index 6e0405d47ce1..63635c221c7c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.h
@@ -6,8 +6,11 @@
 #ifndef __I915_GEM_DMABUF_H__
 #define __I915_GEM_DMABUF_H__
 
+#include <linux/types.h>
+
 struct drm_gem_object;
 struct drm_device;
+struct drm_file;
 struct dma_buf;
 
 struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
@@ -15,4 +18,8 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
 
 struct dma_buf *i915_gem_prime_export(struct drm_gem_object *gem_obj, int flags);
 
+int i915_gem_prime_fd_to_handle(struct drm_device *dev,
+				struct drm_file *file_priv, int prime_fd,
+				uint32_t *handle);
+
 #endif /* __I915_GEM_DMABUF_H__ */
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index ace8534b6cc5..03f3157371bf 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -1806,7 +1806,7 @@ static const struct drm_driver i915_drm_driver = {
 	.show_fdinfo = i915_drm_client_fdinfo,
 
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
-	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+	.prime_fd_to_handle = i915_gem_prime_fd_to_handle,
 	.gem_prime_import = i915_gem_prime_import,
 
 	.dumb_create = i915_gem_dumb_create,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [PATCH 8/8] drm/i915: Implement fdinfo memory stats printing
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel; +Cc: Aravind Iddamsetty, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Use the newly added drm_print_memory_stats helper to show memory
utilisation of our objects in drm/driver specific fdinfo output.

To collect the stats we walk the per memory regions object lists
and accumulate object size into the respective drm_memory_stats
categories.

Objects with multiple possible placements are reported in multiple
regions for total and shared sizes, while other categories are
counted only for the currently active region.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Cc: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/i915/i915_drm_client.c | 64 ++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 31316edbf30b..596de36ee09c 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -48,6 +48,68 @@ void __i915_drm_client_free(struct kref *kref)
 }
 
 #ifdef CONFIG_PROC_FS
+static void
+obj_meminfo(struct drm_i915_gem_object *obj,
+	    struct drm_memory_stats stats[INTEL_REGION_UNKNOWN])
+{
+	struct intel_memory_region *mr;
+	u64 sz = obj->base.size;
+	enum intel_region_id id;
+	unsigned int i;
+
+	/* Attribute size and shared to all possible memory regions. */
+	for (i = 0; i < obj->mm.n_placements; i++) {
+		mr = obj->mm.placements[i];
+		id = mr->id;
+
+		if (obj->base.handle_count > 1)
+			stats[id].shared += sz;
+		else
+			stats[id].private += sz;
+	}
+
+	/* Attribute other categories to only the current region. */
+	mr = obj->mm.region;
+	if (mr)
+		id = mr->id;
+	else
+		id = INTEL_REGION_SMEM;
+
+	if (i915_gem_object_has_pages(obj)) {
+		stats[id].resident += sz;
+
+		if (!dma_resv_test_signaled(obj->base.resv,
+					    dma_resv_usage_rw(true)))
+			stats[id].active += sz;
+		else if (i915_gem_object_is_shrinkable(obj) &&
+			 obj->mm.madv == I915_MADV_DONTNEED)
+			stats[id].purgeable += sz;
+	}
+}
+
+static void show_meminfo(struct drm_printer *p, struct drm_file *file)
+{
+	struct drm_memory_stats stats[INTEL_REGION_UNKNOWN] = {};
+	struct drm_i915_file_private *fpriv = file->driver_priv;
+	struct i915_drm_client *client = fpriv->client;
+	struct drm_i915_private *i915 = fpriv->i915;
+	struct drm_i915_gem_object *obj;
+	struct intel_memory_region *mr;
+	unsigned int id;
+
+	spin_lock_irq(&client->objects_lock);
+	list_for_each_entry(obj, &client->objects_list, client_link)
+		obj_meminfo(obj, stats);
+	spin_unlock_irq(&client->objects_lock);
+
+	for_each_memory_region(mr, i915, id)
+		drm_print_memory_stats(p,
+				       &stats[id],
+				       DRM_GEM_OBJECT_RESIDENT |
+				       DRM_GEM_OBJECT_PURGEABLE,
+				       mr->name);
+}
+
 static const char * const uabi_class_names[] = {
 	[I915_ENGINE_CLASS_RENDER] = "render",
 	[I915_ENGINE_CLASS_COPY] = "copy",
@@ -109,6 +171,8 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file)
 	 * ******************************************************************
 	 */
 
+	show_meminfo(p, file);
+
 	if (GRAPHICS_VER(i915) < 8)
 		return;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [Intel-gfx] [PATCH 8/8] drm/i915: Implement fdinfo memory stats printing
@ 2023-06-09 12:11   ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 12:11 UTC (permalink / raw)
  To: Intel-gfx, dri-devel

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Use the newly added drm_print_memory_stats helper to show memory
utilisation of our objects in drm/driver specific fdinfo output.

To collect the stats we walk the per memory regions object lists
and accumulate object size into the respective drm_memory_stats
categories.

Objects with multiple possible placements are reported in multiple
regions for total and shared sizes, while other categories are
counted only for the currently active region.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Cc: Rob Clark <robdclark@gmail.com>
---
 drivers/gpu/drm/i915/i915_drm_client.c | 64 ++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 31316edbf30b..596de36ee09c 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -48,6 +48,68 @@ void __i915_drm_client_free(struct kref *kref)
 }
 
 #ifdef CONFIG_PROC_FS
+static void
+obj_meminfo(struct drm_i915_gem_object *obj,
+	    struct drm_memory_stats stats[INTEL_REGION_UNKNOWN])
+{
+	struct intel_memory_region *mr;
+	u64 sz = obj->base.size;
+	enum intel_region_id id;
+	unsigned int i;
+
+	/* Attribute size and shared to all possible memory regions. */
+	for (i = 0; i < obj->mm.n_placements; i++) {
+		mr = obj->mm.placements[i];
+		id = mr->id;
+
+		if (obj->base.handle_count > 1)
+			stats[id].shared += sz;
+		else
+			stats[id].private += sz;
+	}
+
+	/* Attribute other categories to only the current region. */
+	mr = obj->mm.region;
+	if (mr)
+		id = mr->id;
+	else
+		id = INTEL_REGION_SMEM;
+
+	if (i915_gem_object_has_pages(obj)) {
+		stats[id].resident += sz;
+
+		if (!dma_resv_test_signaled(obj->base.resv,
+					    dma_resv_usage_rw(true)))
+			stats[id].active += sz;
+		else if (i915_gem_object_is_shrinkable(obj) &&
+			 obj->mm.madv == I915_MADV_DONTNEED)
+			stats[id].purgeable += sz;
+	}
+}
+
+static void show_meminfo(struct drm_printer *p, struct drm_file *file)
+{
+	struct drm_memory_stats stats[INTEL_REGION_UNKNOWN] = {};
+	struct drm_i915_file_private *fpriv = file->driver_priv;
+	struct i915_drm_client *client = fpriv->client;
+	struct drm_i915_private *i915 = fpriv->i915;
+	struct drm_i915_gem_object *obj;
+	struct intel_memory_region *mr;
+	unsigned int id;
+
+	spin_lock_irq(&client->objects_lock);
+	list_for_each_entry(obj, &client->objects_list, client_link)
+		obj_meminfo(obj, stats);
+	spin_unlock_irq(&client->objects_lock);
+
+	for_each_memory_region(mr, i915, id)
+		drm_print_memory_stats(p,
+				       &stats[id],
+				       DRM_GEM_OBJECT_RESIDENT |
+				       DRM_GEM_OBJECT_PURGEABLE,
+				       mr->name);
+}
+
 static const char * const uabi_class_names[] = {
 	[I915_ENGINE_CLASS_RENDER] = "render",
 	[I915_ENGINE_CLASS_COPY] = "copy",
@@ -109,6 +171,8 @@ void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file)
 	 * ******************************************************************
 	 */
 
+	show_meminfo(p, file);
+
 	if (GRAPHICS_VER(i915) < 8)
 		return;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread
* Re: [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
  2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 12:44     ` Iddamsetty, Aravind
  -1 siblings, 0 replies; 29+ messages in thread
From: Iddamsetty, Aravind @ 2023-06-09 12:44 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx, dri-devel; +Cc: Tvrtko Ursulin



On 09-06-2023 17:41, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
> will return a reference to a newly created GEM objects (if created), in
> order to enable tracking of imported i915 GEM objects in the following
> patch.

instead of this what if we implement the open call back in i915

struct drm_gem_object_funcs {

        /**
         * @open:
         *
         * Called upon GEM handle creation.
         *
         * This callback is optional.
         */
        int (*open)(struct drm_gem_object *obj, struct drm_file *file);

which gets called whenever a handle(drm_gem_handle_create_tail) is
created and in the open we can check if to_intel_bo(obj)->base.dma_buf
then it is imported if not it is owned or created by it.

Thanks,
Aravind.

> 
> Minor code reshuffule and only trivial additions on top of
> drm_gem_prime_fd_to_handle.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/drm_prime.c | 41 ++++++++++++++++++++++++++++++++-----
>  include/drm/drm_prime.h     |  4 ++++
>  2 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index d29dafce9bb0..ef75f67e3057 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -284,11 +284,12 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
>  EXPORT_SYMBOL(drm_gem_dmabuf_release);
>  
>  /**
> - * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
> + * drm_gem_prime_fd_to_handle_obj - PRIME import function for GEM drivers
>   * @dev: drm_device to import into
>   * @file_priv: drm file-private structure
>   * @prime_fd: fd id of the dma-buf which should be imported
>   * @handle: pointer to storage for the handle of the imported buffer object
> + * @objp: optional pointer in which reference to created GEM object can be returned
>   *
>   * This is the PRIME import function which must be used mandatorily by GEM
>   * drivers to ensure correct lifetime management of the underlying GEM object.
> @@ -297,9 +298,10 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
>   *
>   * Returns 0 on success or a negative error code on failure.
>   */
> -int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> -			       struct drm_file *file_priv, int prime_fd,
> -			       uint32_t *handle)
> +int drm_gem_prime_fd_to_handle_obj(struct drm_device *dev,
> +				   struct drm_file *file_priv, int prime_fd,
> +				   uint32_t *handle,
> +				   struct drm_gem_object **objp)
>  {
>  	struct dma_buf *dma_buf;
>  	struct drm_gem_object *obj;
> @@ -336,7 +338,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  
>  	/* _handle_create_tail unconditionally unlocks dev->object_name_lock. */
>  	ret = drm_gem_handle_create_tail(file_priv, obj, handle);
> -	drm_gem_object_put(obj);
> +	if (!objp)
> +		drm_gem_object_put(obj);
>  	if (ret)
>  		goto out_put;
>  
> @@ -348,6 +351,9 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  
>  	dma_buf_put(dma_buf);
>  
> +	if (objp)
> +		*objp = obj;
> +
>  	return 0;
>  
>  fail:
> @@ -356,6 +362,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  	 */
>  	drm_gem_handle_delete(file_priv, *handle);
>  	dma_buf_put(dma_buf);
> +	if (objp)
> +		drm_gem_object_put(obj);
>  	return ret;
>  
>  out_unlock:
> @@ -365,6 +373,29 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  	dma_buf_put(dma_buf);
>  	return ret;
>  }
> +EXPORT_SYMBOL(drm_gem_prime_fd_to_handle_obj);
> +
> +/**
> + * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
> + * @dev: drm_device to import into
> + * @file_priv: drm file-private structure
> + * @prime_fd: fd id of the dma-buf which should be imported
> + * @handle: pointer to storage for the handle of the imported buffer object
> + *
> + * This is the PRIME import function which must be used mandatorily by GEM
> + * drivers to ensure correct lifetime management of the underlying GEM object.
> + * The actual importing of GEM object from the dma-buf is done through the
> + * &drm_driver.gem_prime_import driver callback.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> +			       struct drm_file *file_priv, int prime_fd,
> +			       uint32_t *handle)
> +{
> +	return drm_gem_prime_fd_to_handle_obj(dev, file_priv, prime_fd, handle,
> +					      NULL);
> +}
>  EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
>  
>  int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
> index 2a1d01e5b56b..10d145ce6586 100644
> --- a/include/drm/drm_prime.h
> +++ b/include/drm/drm_prime.h
> @@ -69,6 +69,10 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
>  
>  int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  			       struct drm_file *file_priv, int prime_fd, uint32_t *handle);
> +int drm_gem_prime_fd_to_handle_obj(struct drm_device *dev,
> +				   struct drm_file *file_priv, int prime_fd,
> +				   uint32_t *handle,
> +				   struct drm_gem_object **obj);
>  int drm_gem_prime_handle_to_fd(struct drm_device *dev,
>  			       struct drm_file *file_priv, uint32_t handle, uint32_t flags,
>  			       int *prime_fd);

^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [Intel-gfx] [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
@ 2023-06-09 12:44     ` Iddamsetty, Aravind
  0 siblings, 0 replies; 29+ messages in thread
From: Iddamsetty, Aravind @ 2023-06-09 12:44 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx, dri-devel



On 09-06-2023 17:41, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
> will return a reference to a newly created GEM objects (if created), in
> order to enable tracking of imported i915 GEM objects in the following
> patch.

instead of this what if we implement the open call back in i915

struct drm_gem_object_funcs {

        /**
         * @open:
         *
         * Called upon GEM handle creation.
         *
         * This callback is optional.
         */
        int (*open)(struct drm_gem_object *obj, struct drm_file *file);

which gets called whenever a handle(drm_gem_handle_create_tail) is
created and in the open we can check if to_intel_bo(obj)->base.dma_buf
then it is imported if not it is owned or created by it.

Thanks,
Aravind.

> 
> Minor code reshuffule and only trivial additions on top of
> drm_gem_prime_fd_to_handle.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/drm_prime.c | 41 ++++++++++++++++++++++++++++++++-----
>  include/drm/drm_prime.h     |  4 ++++
>  2 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index d29dafce9bb0..ef75f67e3057 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -284,11 +284,12 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
>  EXPORT_SYMBOL(drm_gem_dmabuf_release);
>  
>  /**
> - * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
> + * drm_gem_prime_fd_to_handle_obj - PRIME import function for GEM drivers
>   * @dev: drm_device to import into
>   * @file_priv: drm file-private structure
>   * @prime_fd: fd id of the dma-buf which should be imported
>   * @handle: pointer to storage for the handle of the imported buffer object
> + * @objp: optional pointer in which reference to created GEM object can be returned
>   *
>   * This is the PRIME import function which must be used mandatorily by GEM
>   * drivers to ensure correct lifetime management of the underlying GEM object.
> @@ -297,9 +298,10 @@ EXPORT_SYMBOL(drm_gem_dmabuf_release);
>   *
>   * Returns 0 on success or a negative error code on failure.
>   */
> -int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> -			       struct drm_file *file_priv, int prime_fd,
> -			       uint32_t *handle)
> +int drm_gem_prime_fd_to_handle_obj(struct drm_device *dev,
> +				   struct drm_file *file_priv, int prime_fd,
> +				   uint32_t *handle,
> +				   struct drm_gem_object **objp)
>  {
>  	struct dma_buf *dma_buf;
>  	struct drm_gem_object *obj;
> @@ -336,7 +338,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  
>  	/* _handle_create_tail unconditionally unlocks dev->object_name_lock. */
>  	ret = drm_gem_handle_create_tail(file_priv, obj, handle);
> -	drm_gem_object_put(obj);
> +	if (!objp)
> +		drm_gem_object_put(obj);
>  	if (ret)
>  		goto out_put;
>  
> @@ -348,6 +351,9 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  
>  	dma_buf_put(dma_buf);
>  
> +	if (objp)
> +		*objp = obj;
> +
>  	return 0;
>  
>  fail:
> @@ -356,6 +362,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  	 */
>  	drm_gem_handle_delete(file_priv, *handle);
>  	dma_buf_put(dma_buf);
> +	if (objp)
> +		drm_gem_object_put(obj);
>  	return ret;
>  
>  out_unlock:
> @@ -365,6 +373,29 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  	dma_buf_put(dma_buf);
>  	return ret;
>  }
> +EXPORT_SYMBOL(drm_gem_prime_fd_to_handle_obj);
> +
> +/**
> + * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
> + * @dev: drm_device to import into
> + * @file_priv: drm file-private structure
> + * @prime_fd: fd id of the dma-buf which should be imported
> + * @handle: pointer to storage for the handle of the imported buffer object
> + *
> + * This is the PRIME import function which must be used mandatorily by GEM
> + * drivers to ensure correct lifetime management of the underlying GEM object.
> + * The actual importing of GEM object from the dma-buf is done through the
> + * &drm_driver.gem_prime_import driver callback.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> +			       struct drm_file *file_priv, int prime_fd,
> +			       uint32_t *handle)
> +{
> +	return drm_gem_prime_fd_to_handle_obj(dev, file_priv, prime_fd, handle,
> +					      NULL);
> +}
>  EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
>  
>  int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
> diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
> index 2a1d01e5b56b..10d145ce6586 100644
> --- a/include/drm/drm_prime.h
> +++ b/include/drm/drm_prime.h
> @@ -69,6 +69,10 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
>  
>  int drm_gem_prime_fd_to_handle(struct drm_device *dev,
>  			       struct drm_file *file_priv, int prime_fd, uint32_t *handle);
> +int drm_gem_prime_fd_to_handle_obj(struct drm_device *dev,
> +				   struct drm_file *file_priv, int prime_fd,
> +				   uint32_t *handle,
> +				   struct drm_gem_object **obj);
>  int drm_gem_prime_handle_to_fd(struct drm_device *dev,
>  			       struct drm_file *file_priv, uint32_t handle, uint32_t flags,
>  			       int *prime_fd);

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for fdinfo memory stats (rev2)
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
                   ` (8 preceding siblings ...)
  (?)
@ 2023-06-09 12:48 ` Patchwork
  -1 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2023-06-09 12:48 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: fdinfo memory stats (rev2)
URL   : https://patchwork.freedesktop.org/series/119082/
State : warning

== Summary ==

Error: dim checkpatch failed
67ba6f7b318b dma-fence: Bypass signaling annotation from dma_fence_is_signaled
bc390e8b6613 drm/i915: Track buffer objects belonging to clients
-:262: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#262: FILE: drivers/gpu/drm/i915/i915_drm_client.h:79:
+{
+

-:263: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#263: FILE: drivers/gpu/drm/i915/i915_drm_client.h:80:
+
+}

-:267: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#267: FILE: drivers/gpu/drm/i915/i915_drm_client.h:84:
+{
+

-:268: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#268: FILE: drivers/gpu/drm/i915/i915_drm_client.h:85:
+
+}

total: 0 errors, 0 warnings, 4 checks, 228 lines checked
bf30cc82b5a3 drm/i915: Record which clients own a VM
f74104b7e164 drm/i915: Track page table backing store usage
e7e9f6705f0a drm/i915: Account ring buffer and context state storage
-:79: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#79: FILE: drivers/gpu/drm/i915/i915_drm_client.h:88:
 
+}

-:84: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#84: FILE: drivers/gpu/drm/i915/i915_drm_client.h:93:
+{
+

total: 0 errors, 0 warnings, 2 checks, 59 lines checked
a69e31d41fb8 drm: Add drm_gem_prime_fd_to_handle_obj
-:43: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u32' over 'uint32_t'
#43: FILE: drivers/gpu/drm/drm_prime.c:303:
+				   uint32_t *handle,

-:117: CHECK:PREFER_KERNEL_TYPES: Prefer kernel type 'u32' over 'uint32_t'
#117: FILE: include/drm/drm_prime.h:74:
+				   uint32_t *handle,

total: 0 errors, 0 warnings, 2 checks, 91 lines checked
91ae76e91672 drm/i915: Track imported dma-buf objects in memory stats
9d9b6f173db8 drm/i915: Implement fdinfo memory stats printing



^ permalink raw reply	[flat|nested] 29+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for fdinfo memory stats (rev2)
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
                   ` (9 preceding siblings ...)
  (?)
@ 2023-06-09 12:48 ` Patchwork
  -1 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2023-06-09 12:48 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

== Series Details ==

Series: fdinfo memory stats (rev2)
URL   : https://patchwork.freedesktop.org/series/119082/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
  2023-06-09 12:44     ` [Intel-gfx] " Iddamsetty, Aravind
@ 2023-06-09 14:12       ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 14:12 UTC (permalink / raw)
  To: Iddamsetty, Aravind, Intel-gfx, dri-devel; +Cc: Tvrtko Ursulin


On 09/06/2023 13:44, Iddamsetty, Aravind wrote:
> On 09-06-2023 17:41, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
>> will return a reference to a newly created GEM objects (if created), in
>> order to enable tracking of imported i915 GEM objects in the following
>> patch.
> 
> instead of this what if we implement the open call back in i915
> 
> struct drm_gem_object_funcs {
> 
>          /**
>           * @open:
>           *
>           * Called upon GEM handle creation.
>           *
>           * This callback is optional.
>           */
>          int (*open)(struct drm_gem_object *obj, struct drm_file *file);
> 
> which gets called whenever a handle(drm_gem_handle_create_tail) is
> created and in the open we can check if to_intel_bo(obj)->base.dma_buf
> then it is imported if not it is owned or created by it.

I wanted to track as much memory usage as we have which is buffer object 
backed, including page tables and contexts. And since those are not user 
visible (they don't have handles), they wouldn't be covered by the 
obj->funcs->open() callback.

If we wanted to limit to objects with handles we could simply do what 
Rob proposed and that is to walk the handles idr. But that does not feel 
like the right direction to me. Open for discussion I guess.

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [Intel-gfx] [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
@ 2023-06-09 14:12       ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-09 14:12 UTC (permalink / raw)
  To: Iddamsetty, Aravind, Intel-gfx, dri-devel


On 09/06/2023 13:44, Iddamsetty, Aravind wrote:
> On 09-06-2023 17:41, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
>> will return a reference to a newly created GEM objects (if created), in
>> order to enable tracking of imported i915 GEM objects in the following
>> patch.
> 
> instead of this what if we implement the open call back in i915
> 
> struct drm_gem_object_funcs {
> 
>          /**
>           * @open:
>           *
>           * Called upon GEM handle creation.
>           *
>           * This callback is optional.
>           */
>          int (*open)(struct drm_gem_object *obj, struct drm_file *file);
> 
> which gets called whenever a handle(drm_gem_handle_create_tail) is
> created and in the open we can check if to_intel_bo(obj)->base.dma_buf
> then it is imported if not it is owned or created by it.

I wanted to track as much memory usage as we have which is buffer object 
backed, including page tables and contexts. And since those are not user 
visible (they don't have handles), they wouldn't be covered by the 
obj->funcs->open() callback.

If we wanted to limit to objects with handles we could simply do what 
Rob proposed and that is to walk the handles idr. But that does not feel 
like the right direction to me. Open for discussion I guess.

Regards,

Tvrtko

^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
  2023-06-09 14:12       ` [Intel-gfx] " Tvrtko Ursulin
@ 2023-06-09 17:09         ` Rob Clark
  -1 siblings, 0 replies; 29+ messages in thread
From: Rob Clark @ 2023-06-09 17:09 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: Rob Clark, Intel-gfx, dri-devel, Iddamsetty, Aravind, Tvrtko Ursulin

On Fri, Jun 9, 2023 at 7:12 AM Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
>
> On 09/06/2023 13:44, Iddamsetty, Aravind wrote:
> > On 09-06-2023 17:41, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
> >> will return a reference to a newly created GEM objects (if created), in
> >> order to enable tracking of imported i915 GEM objects in the following
> >> patch.
> >
> > instead of this what if we implement the open call back in i915
> >
> > struct drm_gem_object_funcs {
> >
> >          /**
> >           * @open:
> >           *
> >           * Called upon GEM handle creation.
> >           *
> >           * This callback is optional.
> >           */
> >          int (*open)(struct drm_gem_object *obj, struct drm_file *file);
> >
> > which gets called whenever a handle(drm_gem_handle_create_tail) is
> > created and in the open we can check if to_intel_bo(obj)->base.dma_buf
> > then it is imported if not it is owned or created by it.
>
> I wanted to track as much memory usage as we have which is buffer object
> backed, including page tables and contexts. And since those are not user
> visible (they don't have handles), they wouldn't be covered by the
> obj->funcs->open() callback.
>
> If we wanted to limit to objects with handles we could simply do what
> Rob proposed and that is to walk the handles idr. But that does not feel
> like the right direction to me. Open for discussion I guess.

I guess you just have a few special case objects per context?
Wouldn't it be easier to just track _those_ specially and append them
to the results after doing the normal idr table walk?

(Also, doing something special for dma-buf smells a bit odd..
considering that we also have legacy flink name based sharing as
well.)

BR,
-R

^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [Intel-gfx] [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
@ 2023-06-09 17:09         ` Rob Clark
  0 siblings, 0 replies; 29+ messages in thread
From: Rob Clark @ 2023-06-09 17:09 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Rob Clark, Intel-gfx, dri-devel

On Fri, Jun 9, 2023 at 7:12 AM Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
>
> On 09/06/2023 13:44, Iddamsetty, Aravind wrote:
> > On 09-06-2023 17:41, Tvrtko Ursulin wrote:
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
> >> will return a reference to a newly created GEM objects (if created), in
> >> order to enable tracking of imported i915 GEM objects in the following
> >> patch.
> >
> > instead of this what if we implement the open call back in i915
> >
> > struct drm_gem_object_funcs {
> >
> >          /**
> >           * @open:
> >           *
> >           * Called upon GEM handle creation.
> >           *
> >           * This callback is optional.
> >           */
> >          int (*open)(struct drm_gem_object *obj, struct drm_file *file);
> >
> > which gets called whenever a handle(drm_gem_handle_create_tail) is
> > created and in the open we can check if to_intel_bo(obj)->base.dma_buf
> > then it is imported if not it is owned or created by it.
>
> I wanted to track as much memory usage as we have which is buffer object
> backed, including page tables and contexts. And since those are not user
> visible (they don't have handles), they wouldn't be covered by the
> obj->funcs->open() callback.
>
> If we wanted to limit to objects with handles we could simply do what
> Rob proposed and that is to walk the handles idr. But that does not feel
> like the right direction to me. Open for discussion I guess.

I guess you just have a few special case objects per context?
Wouldn't it be easier to just track _those_ specially and append them
to the results after doing the normal idr table walk?

(Also, doing something special for dma-buf smells a bit odd..
considering that we also have legacy flink name based sharing as
well.)

BR,
-R

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for fdinfo memory stats (rev2)
  2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
                   ` (10 preceding siblings ...)
  (?)
@ 2023-06-09 17:47 ` Patchwork
  -1 siblings, 0 replies; 29+ messages in thread
From: Patchwork @ 2023-06-09 17:47 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 9921 bytes --]

== Series Details ==

Series: fdinfo memory stats (rev2)
URL   : https://patchwork.freedesktop.org/series/119082/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13256 -> Patchwork_119082v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_119082v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_119082v2, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/index.html

Participating hosts (36 -> 37)
------------------------------

  Additional (1): fi-kbl-soraka 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_119082v2:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - fi-ilk-650:         [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/fi-ilk-650/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-ilk-650/igt@i915_module_load@load.html
    - fi-blb-e6850:       [PASS][3] -> [ABORT][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/fi-blb-e6850/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-blb-e6850/igt@i915_module_load@load.html
    - fi-elk-e7500:       [PASS][5] -> [ABORT][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/fi-elk-e7500/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-elk-e7500/igt@i915_module_load@load.html
    - fi-hsw-4770:        [PASS][7] -> [ABORT][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/fi-hsw-4770/igt@i915_module_load@load.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-hsw-4770/igt@i915_module_load@load.html
    - fi-ivb-3770:        [PASS][9] -> [ABORT][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/fi-ivb-3770/igt@i915_module_load@load.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-ivb-3770/igt@i915_module_load@load.html

  
Known issues
------------

  Here are the changes found in Patchwork_119082v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#2190])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][12] ([fdo#109271] / [i915#4613]) +3 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_module_load@load:
    - fi-pnv-d510:        [PASS][13] -> [ABORT][14] ([i915#8143])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/fi-pnv-d510/igt@i915_module_load@load.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-pnv-d510/igt@i915_module_load@load.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-rkl-11600:       [PASS][15] -> [DMESG-FAIL][16] ([i915#5334])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/fi-rkl-11600/igt@i915_selftest@live@gt_heartbeat.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-rkl-11600/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][17] ([i915#1886] / [i915#7913])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-1:         NOTRUN -> [DMESG-WARN][18] ([i915#6367])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/bat-rpls-1/igt@i915_selftest@live@slpc.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-rpls-1:         NOTRUN -> [ABORT][19] ([i915#6687] / [i915#7978])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][20] ([fdo#109271]) +14 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1:
    - bat-dg2-8:          [PASS][21] -> [FAIL][22] ([i915#7932])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][23] ([fdo#109271] / [i915#4579])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-kbl-soraka/igt@kms_setmode@basic-clone-single-crtc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gem_contexts:
    - {bat-mtlp-8}:       [ABORT][24] -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/bat-mtlp-8/igt@i915_selftest@live@gem_contexts.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/bat-mtlp-8/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-1:         [ABORT][26] ([i915#4983] / [i915#7461] / [i915#7981] / [i915#8347] / [i915#8384]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/bat-rpls-1/igt@i915_selftest@live@reset.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/bat-rpls-1/igt@i915_selftest@live@reset.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         [DMESG-WARN][28] ([i915#6367]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/bat-rpls-2/igt@i915_selftest@live@slpc.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/bat-rpls-2/igt@i915_selftest@live@slpc.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1:
    - bat-dg2-8:          [FAIL][30] ([i915#7932]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-c-dp-1.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-c-dp-1:
    - fi-cfl-8109u:       [ABORT][32] -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13256/fi-cfl-8109u/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-c-dp-1.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/fi-cfl-8109u/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-c-dp-1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#6763]: https://gitlab.freedesktop.org/drm/intel/issues/6763
  [i915#7269]: https://gitlab.freedesktop.org/drm/intel/issues/7269
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981
  [i915#8143]: https://gitlab.freedesktop.org/drm/intel/issues/8143
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
  [i915#8384]: https://gitlab.freedesktop.org/drm/intel/issues/8384


Build changes
-------------

  * Linux: CI_DRM_13256 -> Patchwork_119082v2

  CI-20190529: 20190529
  CI_DRM_13256: be85dc2d44c075230eec4366e27bc1fe75ee59ff @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7322: 2dd77d6d827a308caae49ce3eba759c2bab394ed @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_119082v2: be85dc2d44c075230eec4366e27bc1fe75ee59ff @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

fecd56a0b5bb drm/i915: Implement fdinfo memory stats printing
6f4c8bdba33c drm/i915: Track imported dma-buf objects in memory stats
8dae4d6ade09 drm: Add drm_gem_prime_fd_to_handle_obj
21a8c18f3e7a drm/i915: Account ring buffer and context state storage
7f515768b8a4 drm/i915: Track page table backing store usage
86071a491921 drm/i915: Record which clients own a VM
9fa54f17d458 drm/i915: Track buffer objects belonging to clients
37e74fac7b42 dma-fence: Bypass signaling annotation from dma_fence_is_signaled

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v2/index.html

[-- Attachment #2: Type: text/html, Size: 10968 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
  2023-06-09 17:09         ` [Intel-gfx] " Rob Clark
@ 2023-06-12  8:26           ` Tvrtko Ursulin
  -1 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-12  8:26 UTC (permalink / raw)
  To: Rob Clark
  Cc: Rob Clark, Intel-gfx, dri-devel, Iddamsetty, Aravind, Tvrtko Ursulin


On 09/06/2023 18:09, Rob Clark wrote:
> On Fri, Jun 9, 2023 at 7:12 AM Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>>
>>
>> On 09/06/2023 13:44, Iddamsetty, Aravind wrote:
>>> On 09-06-2023 17:41, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
>>>> will return a reference to a newly created GEM objects (if created), in
>>>> order to enable tracking of imported i915 GEM objects in the following
>>>> patch.
>>>
>>> instead of this what if we implement the open call back in i915
>>>
>>> struct drm_gem_object_funcs {
>>>
>>>           /**
>>>            * @open:
>>>            *
>>>            * Called upon GEM handle creation.
>>>            *
>>>            * This callback is optional.
>>>            */
>>>           int (*open)(struct drm_gem_object *obj, struct drm_file *file);
>>>
>>> which gets called whenever a handle(drm_gem_handle_create_tail) is
>>> created and in the open we can check if to_intel_bo(obj)->base.dma_buf
>>> then it is imported if not it is owned or created by it.
>>
>> I wanted to track as much memory usage as we have which is buffer object
>> backed, including page tables and contexts. And since those are not user
>> visible (they don't have handles), they wouldn't be covered by the
>> obj->funcs->open() callback.
>>
>> If we wanted to limit to objects with handles we could simply do what
>> Rob proposed and that is to walk the handles idr. But that does not feel
>> like the right direction to me. Open for discussion I guess.
> 
> I guess you just have a few special case objects per context?

Per context we have context image (register state etc) and ring buffer 
(command emission), per engine.

Then we have all the page table backing store per each VM/ppgtt/context 
allocated as GEM objects.

> Wouldn't it be easier to just track _those_ specially and append them
> to the results after doing the normal idr table walk?

In a way yes and in a way it is not as elegant. IMHO at least.
> (Also, doing something special for dma-buf smells a bit odd..
> considering that we also have legacy flink name based sharing as
> well.)

It's not really special, just needed to return a tuple of (object, 
handle) from the prime import helper. So it can plug into the very same 
tracking I use from other paths.

I was going for some kind of elegance with one loop - single tracking - 
as long as I had to add new list head to our buffer object.

Anyway, I can re-spin a simplified version (fewer patches) with two 
loops. Only downside is that the new list head will only be used for 
internal objects.

Regards,

Tvrtko

P.S.
Flink I indeed missed. Is that used nowadays btw?

^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [Intel-gfx] [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj
@ 2023-06-12  8:26           ` Tvrtko Ursulin
  0 siblings, 0 replies; 29+ messages in thread
From: Tvrtko Ursulin @ 2023-06-12  8:26 UTC (permalink / raw)
  To: Rob Clark; +Cc: Rob Clark, Intel-gfx, dri-devel


On 09/06/2023 18:09, Rob Clark wrote:
> On Fri, Jun 9, 2023 at 7:12 AM Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>>
>>
>> On 09/06/2023 13:44, Iddamsetty, Aravind wrote:
>>> On 09-06-2023 17:41, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> I need a new flavour of the drm_gem_prime_fd_to_handle helper, one which
>>>> will return a reference to a newly created GEM objects (if created), in
>>>> order to enable tracking of imported i915 GEM objects in the following
>>>> patch.
>>>
>>> instead of this what if we implement the open call back in i915
>>>
>>> struct drm_gem_object_funcs {
>>>
>>>           /**
>>>            * @open:
>>>            *
>>>            * Called upon GEM handle creation.
>>>            *
>>>            * This callback is optional.
>>>            */
>>>           int (*open)(struct drm_gem_object *obj, struct drm_file *file);
>>>
>>> which gets called whenever a handle(drm_gem_handle_create_tail) is
>>> created and in the open we can check if to_intel_bo(obj)->base.dma_buf
>>> then it is imported if not it is owned or created by it.
>>
>> I wanted to track as much memory usage as we have which is buffer object
>> backed, including page tables and contexts. And since those are not user
>> visible (they don't have handles), they wouldn't be covered by the
>> obj->funcs->open() callback.
>>
>> If we wanted to limit to objects with handles we could simply do what
>> Rob proposed and that is to walk the handles idr. But that does not feel
>> like the right direction to me. Open for discussion I guess.
> 
> I guess you just have a few special case objects per context?

Per context we have context image (register state etc) and ring buffer 
(command emission), per engine.

Then we have all the page table backing store per each VM/ppgtt/context 
allocated as GEM objects.

> Wouldn't it be easier to just track _those_ specially and append them
> to the results after doing the normal idr table walk?

In a way yes and in a way it is not as elegant. IMHO at least.
> (Also, doing something special for dma-buf smells a bit odd..
> considering that we also have legacy flink name based sharing as
> well.)

It's not really special, just needed to return a tuple of (object, 
handle) from the prime import helper. So it can plug into the very same 
tracking I use from other paths.

I was going for some kind of elegance with one loop - single tracking - 
as long as I had to add new list head to our buffer object.

Anyway, I can re-spin a simplified version (fewer patches) with two 
loops. Only downside is that the new list head will only be used for 
internal objects.

Regards,

Tvrtko

P.S.
Flink I indeed missed. Is that used nowadays btw?

^ permalink raw reply	[flat|nested] 29+ messages in thread
end of thread, other threads:[~2023-06-12  8:26 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 12:11 [PATCH v3 0/8] fdinfo memory stats Tvrtko Ursulin
2023-06-09 12:11 ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:11 ` [PATCH 1/8] dma-fence: Bypass signaling annotation from dma_fence_is_signaled Tvrtko Ursulin
2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:11 ` [PATCH 2/8] drm/i915: Track buffer objects belonging to clients Tvrtko Ursulin
2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:11 ` [PATCH 3/8] drm/i915: Record which clients own a VM Tvrtko Ursulin
2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:11 ` [PATCH 4/8] drm/i915: Track page table backing store usage Tvrtko Ursulin
2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:11 ` [PATCH 5/8] drm/i915: Account ring buffer and context state storage Tvrtko Ursulin
2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:11 ` [PATCH 6/8] drm: Add drm_gem_prime_fd_to_handle_obj Tvrtko Ursulin
2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:44   ` Iddamsetty, Aravind
2023-06-09 12:44     ` [Intel-gfx] " Iddamsetty, Aravind
2023-06-09 14:12     ` Tvrtko Ursulin
2023-06-09 14:12       ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 17:09       ` Rob Clark
2023-06-09 17:09         ` [Intel-gfx] " Rob Clark
2023-06-12  8:26         ` Tvrtko Ursulin
2023-06-12  8:26           ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:11 ` [PATCH 7/8] drm/i915: Track imported dma-buf objects in memory stats Tvrtko Ursulin
2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:11 ` [PATCH 8/8] drm/i915: Implement fdinfo memory stats printing Tvrtko Ursulin
2023-06-09 12:11   ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 12:48 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for fdinfo memory stats (rev2) Patchwork
2023-06-09 12:48 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-06-09 17:47 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

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.