| Message ID | 20230608145133.1059554-2-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fdinfo memory stats | expand |
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; >
On 09/06/2023 05:16, Iddamsetty, Aravind wrote: > 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" 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
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..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;