From: "Iddamsetty, Aravind" <aravind.iddamsetty@intel.com>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
<Intel-gfx@lists.freedesktop.org>,
<dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 1/5] drm/i915: Track buffer objects belonging to clients
Date: Fri, 9 Jun 2023 09:46:11 +0530 [thread overview]
Message-ID: <b0990625-1d73-8b6d-d094-5e58be5ad6b0@intel.com> (raw)
In-Reply-To: <20230608145133.1059554-2-tvrtko.ursulin@linux.intel.com>
On 08-06-2023 20:21, Tvrtko Ursulin wrote:
> 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 | 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"
Thanks,
Aravind.
>
> - 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..4cacca568375 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
> + mutex_init(&client->objects_lock);
> + INIT_LIST_HEAD(&client->objects_list);
> +
> + client->fpriv = fpriv;
> +#endif
>
> return client;
> }
> @@ -108,4 +115,31 @@ 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)
> +{
> + GEM_WARN_ON(obj->client);
> + GEM_WARN_ON(!list_empty(&obj->client_link));
> +
> + mutex_lock(&client->objects_lock);
> + obj->client = i915_drm_client_get(client);
> + list_add_tail(&obj->client_link, &client->objects_list);
> + mutex_unlock(&client->objects_lock);
> +}
> +
> +void i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
> +{
> + struct i915_drm_client *client = fetch_and_zero(&obj->client);
> +
> + /* Object may not be associated with a client. */
> + if (!client || list_empty(&obj->client_link))
> + return;
> +
> + mutex_lock(&client->objects_lock);
> + list_del(&obj->client_link);
> + mutex_unlock(&client->objects_lock);
> +
> + 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..0db68b4d7a4f 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.h
> +++ b/drivers/gpu/drm/i915/i915_drm_client.h
> @@ -8,10 +8,14 @@
>
> #include <linux/kref.h>
> #include <linux/list.h>
> +#include <linux/mutex.h>
> #include <linux/spinlock.h>
>
> #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 +29,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
> + */
> + struct mutex 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 +65,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;
>
next prev parent reply other threads:[~2023-06-09 4:16 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-08 14:51 [PATCH v2 0/5] fdinfo memory stats Tvrtko Ursulin
2023-06-08 14:51 ` [Intel-gfx] " Tvrtko Ursulin
2023-06-08 14:51 ` [PATCH 1/5] drm/i915: Track buffer objects belonging to clients Tvrtko Ursulin
2023-06-08 14:51 ` [Intel-gfx] " Tvrtko Ursulin
2023-06-09 4:16 ` Iddamsetty, Aravind [this message]
2023-06-09 12:08 ` Tvrtko Ursulin
2023-06-08 14:51 ` [PATCH 2/5] drm/i915: Record which clients own a VM Tvrtko Ursulin
2023-06-08 14:51 ` [Intel-gfx] " Tvrtko Ursulin
2023-06-08 17:48 ` kernel test robot
2023-06-08 17:48 ` kernel test robot
2023-06-08 17:48 ` [Intel-gfx] " kernel test robot
2023-06-08 14:51 ` [PATCH 3/5] drm/i915: Track page table backing store usage Tvrtko Ursulin
2023-06-08 14:51 ` [Intel-gfx] " Tvrtko Ursulin
2023-06-08 14:51 ` [PATCH 4/5] drm/i915: Account ring buffer and context state storage Tvrtko Ursulin
2023-06-08 14:51 ` [Intel-gfx] " Tvrtko Ursulin
2023-06-08 14:51 ` [PATCH 5/5] drm/i915: Implement fdinfo memory stats printing Tvrtko Ursulin
2023-06-08 14:51 ` [Intel-gfx] " Tvrtko Ursulin
2023-06-08 18:47 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for fdinfo memory stats Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b0990625-1d73-8b6d-d094-5e58be5ad6b0@intel.com \
--to=aravind.iddamsetty@intel.com \
--cc=Intel-gfx@lists.freedesktop.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=tvrtko.ursulin@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.