Re: [Intel-gfx] [PATCH 1/5] drm/i915: Track buffer objects belonging to clients

[Tvrtko Ursulin]

On 09/06/2023 05:16, Iddamsetty, Aravind wrote:
On 08-06-2023 20:21, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursu...@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.ursu...@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        | 36 +++++++++++++++++-
  drivers/gpu/drm/i915/i915_drm_client.h        | 37 ++++++++++++++++++-
  drivers/gpu/drm/i915/i915_gem.c               |  2 +-
  6 files changed, 119 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);

Do we intend to track only client created objects and not imported ?
or is that taken care by this "obj->base.handle_count > 1"

I missed the imports, now added in v3 of the series.

They wouldn't have been handled by the above check in the importer - 
only the exporter would have seen (sometimes) changes in the ratio of 
total vs shared.

Regards,

Tvrtko