* [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats
@ 2023-06-12 10:46 Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-06-12 10:46 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.
v4:
* Rely on DRM GEM handles for tracking user objects.
* Fix internal object accounting (no placements).
Tvrtko Ursulin (5):
drm/i915: Add ability for tracking buffer objects per client
drm/i915: Record which client owns a VM
drm/i915: Track page table backing store usage
drm/i915: Account ring buffer and context state storage
drm/i915: Implement fdinfo memory stats printing
drivers/gpu/drm/i915/gem/i915_gem_context.c | 11 +-
.../gpu/drm/i915/gem/i915_gem_context_types.h | 3 +
drivers/gpu/drm/i915/gem/i915_gem_object.c | 5 +
.../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_context.c | 8 ++
drivers/gpu/drm/i915/gt/intel_gtt.c | 6 +
drivers/gpu/drm/i915/gt/intel_gtt.h | 1 +
drivers/gpu/drm/i915/i915_drm_client.c | 124 +++++++++++++++++-
drivers/gpu/drm/i915/i915_drm_client.h | 42 +++++-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
11 files changed, 210 insertions(+), 8 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
@ 2023-06-12 10:46 ` Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 2/5] drm/i915: Record which client owns a VM Tvrtko Ursulin
` (6 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-06-12 10:46 UTC (permalink / raw)
To: Intel-gfx, dri-devel
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
In order to show per client memory usage lets add some infrastructure
which enables tracking buffer objects owned by clients.
We add a per client list protected by a new per client lock and to support
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_object.c | 5 +++
.../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 | 34 +++++++++++++++++-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
5 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 97ac6fb37958..d6961f6818f1 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);
@@ -410,6 +414,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
}
__i915_gem_object_pages_fini(obj);
+ i915_drm_client_remove_object(obj);
__i915_gem_free_object(obj);
/* But keep the pointer alive for RCU-protected lookups */
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..3c8d6a46a801 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,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));
+
+ spin_lock(&client->objects_lock);
+ obj->client = i915_drm_client_get(client);
+ list_add_tail(&obj->client_link, &client->objects_list);
+ spin_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;
+
+ spin_lock(&client->objects_lock);
+ list_del(&obj->client_link);
+ spin_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..5fc897ab1a6b 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,23 @@ 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] 17+ messages in thread
* [Intel-gfx] [PATCH 2/5] drm/i915: Record which client owns a VM
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
@ 2023-06-12 10:46 ` Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 3/5] drm/i915: Track page table backing store usage Tvrtko Ursulin
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-06-12 10:46 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] 17+ messages in thread
* [Intel-gfx] [PATCH 3/5] drm/i915: Track page table backing store usage
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 2/5] drm/i915: Record which client owns a VM Tvrtko Ursulin
@ 2023-06-12 10:46 ` Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 4/5] drm/i915: Account ring buffer and context state storage Tvrtko Ursulin
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-06-12 10:46 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] 17+ messages in thread
* [Intel-gfx] [PATCH 4/5] drm/i915: Account ring buffer and context state storage
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
` (2 preceding siblings ...)
2023-06-12 10:46 ` [Intel-gfx] [PATCH 3/5] drm/i915: Track page table backing store usage Tvrtko Ursulin
@ 2023-06-12 10:46 ` Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 5/5] drm/i915: Implement fdinfo memory stats printing Tvrtko Ursulin
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-06-12 10:46 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/gt/intel_context.c | 8 ++++++++
drivers/gpu/drm/i915/i915_drm_client.c | 10 ++++++++++
drivers/gpu/drm/i915/i915_drm_client.h | 8 ++++++++
3 files changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index a53b26178f0a..cb6d8e7cdd9d 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -6,6 +6,7 @@
#include "gem/i915_gem_context.h"
#include "gem/i915_gem_pm.h"
+#include "i915_drm_client.h"
#include "i915_drv.h"
#include "i915_trace.h"
@@ -50,6 +51,7 @@ intel_context_create(struct intel_engine_cs *engine)
int intel_context_alloc_state(struct intel_context *ce)
{
+ struct i915_gem_context *ctx;
int err = 0;
if (mutex_lock_interruptible(&ce->pin_mutex))
@@ -66,6 +68,12 @@ int intel_context_alloc_state(struct intel_context *ce)
goto unlock;
set_bit(CONTEXT_ALLOC_BIT, &ce->flags);
+
+ rcu_read_lock();
+ ctx = rcu_dereference(ce->gem_context);
+ if (ctx && ctx->file_priv)
+ i915_drm_client_add_context(ctx->file_priv->client, ce);
+ rcu_read_unlock();
}
unlock:
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index 3c8d6a46a801..da29d01d1c3d 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -142,4 +142,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 5fc897ab1a6b..744e48ed133c 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)
@@ -81,6 +84,11 @@ 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
#endif /* !__I915_DRM_CLIENT_H__ */
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 5/5] drm/i915: Implement fdinfo memory stats printing
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
` (3 preceding siblings ...)
2023-06-12 10:46 ` [Intel-gfx] [PATCH 4/5] drm/i915: Account ring buffer and context state storage Tvrtko Ursulin
@ 2023-06-12 10:46 ` Tvrtko Ursulin
2023-06-12 12:45 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for fdinfo memory stats (rev3) Patchwork
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-06-12 10:46 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 | 78 ++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
index da29d01d1c3d..406e5a5c2961 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -48,6 +48,82 @@ 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 (!obj->mm.n_placements) {
+ if (obj->base.handle_count > 1)
+ stats[id].shared += sz;
+ else
+ stats[id].private += sz;
+ }
+
+ 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;
+
+ /* Public objects. */
+ spin_lock(&file->table_lock);
+ idr_for_each_entry (&file->object_idr, obj, id)
+ obj_meminfo(obj, stats);
+ spin_unlock(&file->table_lock);
+
+ /* Internal objects. */
+ spin_lock(&client->objects_lock);
+ list_for_each_entry(obj, &client->objects_list, client_link)
+ obj_meminfo(obj, stats);
+ spin_unlock(&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 +185,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] 17+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for fdinfo memory stats (rev3)
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
` (4 preceding siblings ...)
2023-06-12 10:46 ` [Intel-gfx] [PATCH 5/5] drm/i915: Implement fdinfo memory stats printing Tvrtko Ursulin
@ 2023-06-12 12:45 ` Patchwork
2023-06-12 12:45 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-06-12 13:02 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2023-06-12 12:45 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: fdinfo memory stats (rev3)
URL : https://patchwork.freedesktop.org/series/119082/
State : warning
== Summary ==
Error: dim checkpatch failed
0e7a9361b003 drm/i915: Add ability for tracking buffer objects per client
e0e7d111353c drm/i915: Record which client owns a VM
8645bd51fede drm/i915: Track page table backing store usage
eb2fd9e3fbbe drm/i915: Account ring buffer and context state storage
02d2f2e3e48f drm/i915: Implement fdinfo memory stats printing
-:87: WARNING:SPACING: space prohibited between function name and open parenthesis '('
#87: FILE: drivers/gpu/drm/i915/i915_drm_client.c:109:
+ idr_for_each_entry (&file->object_idr, obj, id)
total: 0 errors, 1 warnings, 0 checks, 90 lines checked
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for fdinfo memory stats (rev3)
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
` (5 preceding siblings ...)
2023-06-12 12:45 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for fdinfo memory stats (rev3) Patchwork
@ 2023-06-12 12:45 ` Patchwork
2023-06-12 13:02 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2023-06-12 12:45 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
== Series Details ==
Series: fdinfo memory stats (rev3)
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] 17+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for fdinfo memory stats (rev3)
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
` (6 preceding siblings ...)
2023-06-12 12:45 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-06-12 13:02 ` Patchwork
7 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2023-06-12 13:02 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 7425 bytes --]
== Series Details ==
Series: fdinfo memory stats (rev3)
URL : https://patchwork.freedesktop.org/series/119082/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13262 -> Patchwork_119082v3
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_119082v3 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_119082v3, 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_119082v3/index.html
Participating hosts (42 -> 41)
------------------------------
Missing (1): fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_119082v3:
### IGT changes ###
#### Possible regressions ####
* igt@gem_busy@busy@all-engines:
- bat-dg1-5: [PASS][1] -> [ABORT][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-dg1-5/igt@gem_busy@busy@all-engines.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-dg1-5/igt@gem_busy@busy@all-engines.html
- bat-dg1-7: [PASS][3] -> [ABORT][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-dg1-7/igt@gem_busy@busy@all-engines.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-dg1-7/igt@gem_busy@busy@all-engines.html
- bat-dg2-9: [PASS][5] -> [ABORT][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-dg2-9/igt@gem_busy@busy@all-engines.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-dg2-9/igt@gem_busy@busy@all-engines.html
- bat-atsm-1: [PASS][7] -> [ABORT][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-atsm-1/igt@gem_busy@busy@all-engines.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-atsm-1/igt@gem_busy@busy@all-engines.html
- bat-dg2-11: [PASS][9] -> [ABORT][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-dg2-11/igt@gem_busy@busy@all-engines.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-dg2-11/igt@gem_busy@busy@all-engines.html
- bat-dg2-8: [PASS][11] -> [ABORT][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-dg2-8/igt@gem_busy@busy@all-engines.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-dg2-8/igt@gem_busy@busy@all-engines.html
Known issues
------------
Here are the changes found in Patchwork_119082v3 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-kbl-soraka: [PASS][13] -> [DMESG-FAIL][14] ([i915#5334] / [i915#7872])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@reset:
- bat-rpls-2: [PASS][15] -> [ABORT][16] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#7981] / [i915#8347])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-rpls-2/igt@i915_selftest@live@reset.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-rpls-2/igt@i915_selftest@live@reset.html
* igt@i915_suspend@basic-s3-without-i915:
- bat-rpls-1: NOTRUN -> [ABORT][17] ([i915#6687] / [i915#7978])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html
#### Possible fixes ####
* igt@i915_selftest@live@reset:
- bat-rpls-1: [ABORT][18] ([i915#4983] / [i915#7461] / [i915#8347] / [i915#8384]) -> [PASS][19]
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-rpls-1/igt@i915_selftest@live@reset.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-rpls-1/igt@i915_selftest@live@reset.html
* igt@i915_selftest@live@slpc:
- {bat-mtlp-6}: [DMESG-WARN][20] ([i915#6367]) -> [PASS][21]
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/bat-mtlp-6/igt@i915_selftest@live@slpc.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/bat-mtlp-6/igt@i915_selftest@live@slpc.html
* igt@kms_psr@cursor_plane_move:
- fi-kbl-soraka: [INCOMPLETE][22] -> [PASS][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13262/fi-kbl-soraka/igt@kms_psr@cursor_plane_move.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/fi-kbl-soraka/igt@kms_psr@cursor_plane_move.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[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#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
[i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
[i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
[i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
[i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981
[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_13262 -> Patchwork_119082v3
CI-20190529: 20190529
CI_DRM_13262: 26f62dc5b6119b2fcc3380b76e25dbe3903060a0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7326: 02c2cf17628b6203d6105d4a91dfe8a101d482ce @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_119082v3: 26f62dc5b6119b2fcc3380b76e25dbe3903060a0 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
0497e4b1f7c2 drm/i915: Implement fdinfo memory stats printing
942364187b4f drm/i915: Account ring buffer and context state storage
a6a8bce070ce drm/i915: Track page table backing store usage
6eaa9cbe37cb drm/i915: Record which client owns a VM
15570a43e2fe drm/i915: Add ability for tracking buffer objects per client
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_119082v3/index.html
[-- Attachment #2: Type: text/html, Size: 7693 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-07-07 13:02 [Intel-gfx] [PATCH v5 0/5] fdinfo memory stats Tvrtko Ursulin
@ 2023-07-07 13:02 ` Tvrtko Ursulin
2023-07-10 10:44 ` Iddamsetty, Aravind
0 siblings, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-07-07 13:02 UTC (permalink / raw)
To: Intel-gfx, dri-devel
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
In order to show per client memory usage lets add some infrastructure
which enables tracking buffer objects owned by clients.
We add a per client list protected by a new per client lock and to support
delayed destruction (post client exit) we make tracked objects hold
references to the owning client.
Also, object memory region teardown is moved to the existing RCU free
callback to allow safe dereference from the fdinfo RCU read section.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +++++--
.../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 | 32 +++++++++++++++++
4 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 97ac6fb37958..3dc4fbb67d2b 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);
@@ -292,6 +296,10 @@ void __i915_gem_free_object_rcu(struct rcu_head *head)
container_of(head, typeof(*obj), rcu);
struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ /* We need to keep this alive for RCU read access from fdinfo. */
+ if (obj->mm.n_placements > 1)
+ kfree(obj->mm.placements);
+
i915_gem_object_free(obj);
GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
@@ -388,9 +396,6 @@ void __i915_gem_free_object(struct drm_i915_gem_object *obj)
if (obj->ops->release)
obj->ops->release(obj);
- if (obj->mm.n_placements > 1)
- kfree(obj->mm.placements);
-
if (obj->shares_resv_from)
i915_vm_resv_put(obj->shares_resv_from);
@@ -441,6 +446,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..2e5e69edc0f9 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -28,6 +28,10 @@ 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);
+#endif
return client;
}
@@ -108,4 +112,36 @@ 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_rcu(&obj->client_link, &client->objects_list);
+ spin_unlock_irqrestore(&client->objects_lock, flags);
+}
+
+bool 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)
+ return false;
+
+ spin_lock_irqsave(&client->objects_lock, flags);
+ list_del_rcu(&obj->client_link);
+ spin_unlock_irqrestore(&client->objects_lock, flags);
+
+ i915_drm_client_put(client);
+
+ return true;
+}
#endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 67816c912bca..5f58fdf7dcb8 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,20 @@ 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
+ /**
+ * @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.
*/
@@ -49,4 +66,19 @@ struct i915_drm_client *i915_drm_client_alloc(void);
void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
+#ifdef CONFIG_PROC_FS
+void i915_drm_client_add_object(struct i915_drm_client *client,
+ struct drm_i915_gem_object *obj);
+bool 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 bool i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
+{
+}
+#endif
+
#endif /* !__I915_DRM_CLIENT_H__ */
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-07-07 13:02 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
@ 2023-07-10 10:44 ` Iddamsetty, Aravind
2023-07-10 13:20 ` Tvrtko Ursulin
0 siblings, 1 reply; 17+ messages in thread
From: Iddamsetty, Aravind @ 2023-07-10 10:44 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx, dri-devel
On 07-07-2023 18:32, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> In order to show per client memory usage lets add some infrastructure
> which enables tracking buffer objects owned by clients.
>
> We add a per client list protected by a new per client lock and to support
> delayed destruction (post client exit) we make tracked objects hold
> references to the owning client.
>
> Also, object memory region teardown is moved to the existing RCU free
> callback to allow safe dereference from the fdinfo RCU read section.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +++++--
> .../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 | 32 +++++++++++++++++
> 4 files changed, 90 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 97ac6fb37958..3dc4fbb67d2b 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);
>
> @@ -292,6 +296,10 @@ void __i915_gem_free_object_rcu(struct rcu_head *head)
> container_of(head, typeof(*obj), rcu);
> struct drm_i915_private *i915 = to_i915(obj->base.dev);
>
> + /* We need to keep this alive for RCU read access from fdinfo. */
> + if (obj->mm.n_placements > 1)
> + kfree(obj->mm.placements);
> +
> i915_gem_object_free(obj);
>
> GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
> @@ -388,9 +396,6 @@ void __i915_gem_free_object(struct drm_i915_gem_object *obj)
> if (obj->ops->release)
> obj->ops->release(obj);
>
> - if (obj->mm.n_placements > 1)
> - kfree(obj->mm.placements);
> -
> if (obj->shares_resv_from)
> i915_vm_resv_put(obj->shares_resv_from);
>
> @@ -441,6 +446,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..2e5e69edc0f9 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.c
> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> @@ -28,6 +28,10 @@ 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);
> +#endif
>
> return client;
> }
> @@ -108,4 +112,36 @@ 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_rcu(&obj->client_link, &client->objects_list);
> + spin_unlock_irqrestore(&client->objects_lock, flags);
> +}
would it be nice to mention that we use this client infra only to track
internal objects. While the user created through file->object_idr added
during handle creation time.
Thanks,
Aravind.
> +
> +bool 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)
> + return false;
> +
> + spin_lock_irqsave(&client->objects_lock, flags);
> + list_del_rcu(&obj->client_link);
> + spin_unlock_irqrestore(&client->objects_lock, flags);
> +
> + i915_drm_client_put(client);
> +
> + return true;
> +}
> #endif
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
> index 67816c912bca..5f58fdf7dcb8 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,20 @@ 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
> + /**
> + * @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.
> */
> @@ -49,4 +66,19 @@ struct i915_drm_client *i915_drm_client_alloc(void);
>
> void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
>
> +#ifdef CONFIG_PROC_FS
> +void i915_drm_client_add_object(struct i915_drm_client *client,
> + struct drm_i915_gem_object *obj);
> +bool 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 bool i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
> +{
> +}
> +#endif
> +
> #endif /* !__I915_DRM_CLIENT_H__ */
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-07-10 10:44 ` Iddamsetty, Aravind
@ 2023-07-10 13:20 ` Tvrtko Ursulin
2023-07-11 7:48 ` Iddamsetty, Aravind
0 siblings, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-07-10 13:20 UTC (permalink / raw)
To: Iddamsetty, Aravind, Intel-gfx, dri-devel
On 10/07/2023 11:44, Iddamsetty, Aravind wrote:
> On 07-07-2023 18:32, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> In order to show per client memory usage lets add some infrastructure
>> which enables tracking buffer objects owned by clients.
>>
>> We add a per client list protected by a new per client lock and to support
>> delayed destruction (post client exit) we make tracked objects hold
>> references to the owning client.
>>
>> Also, object memory region teardown is moved to the existing RCU free
>> callback to allow safe dereference from the fdinfo RCU read section.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +++++--
>> .../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 | 32 +++++++++++++++++
>> 4 files changed, 90 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>> index 97ac6fb37958..3dc4fbb67d2b 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);
>>
>> @@ -292,6 +296,10 @@ void __i915_gem_free_object_rcu(struct rcu_head *head)
>> container_of(head, typeof(*obj), rcu);
>> struct drm_i915_private *i915 = to_i915(obj->base.dev);
>>
>> + /* We need to keep this alive for RCU read access from fdinfo. */
>> + if (obj->mm.n_placements > 1)
>> + kfree(obj->mm.placements);
>> +
>> i915_gem_object_free(obj);
>>
>> GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
>> @@ -388,9 +396,6 @@ void __i915_gem_free_object(struct drm_i915_gem_object *obj)
>> if (obj->ops->release)
>> obj->ops->release(obj);
>>
>> - if (obj->mm.n_placements > 1)
>> - kfree(obj->mm.placements);
>> -
>> if (obj->shares_resv_from)
>> i915_vm_resv_put(obj->shares_resv_from);
>>
>> @@ -441,6 +446,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..2e5e69edc0f9 100644
>> --- a/drivers/gpu/drm/i915/i915_drm_client.c
>> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
>> @@ -28,6 +28,10 @@ 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);
>> +#endif
>>
>> return client;
>> }
>> @@ -108,4 +112,36 @@ 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_rcu(&obj->client_link, &client->objects_list);
>> + spin_unlock_irqrestore(&client->objects_lock, flags);
>> +}
>
> would it be nice to mention that we use this client infra only to track
> internal objects. While the user created through file->object_idr added
> during handle creation time.
In this series it is indeed only used for that.
But it would be nicer to use it to track everything, so fdinfo readers
would not be hitting the idr lock, which would avoid injecting latency
to real DRM clients.
The only fly in the ointment IMO is that I needed that drm core helper
to be able to track dmabuf imports. Possibly something for flink too,
did not look into that yet.
In the light of all that I can mention in the cover letter next time
round. It is a bit stale anyway (the cover letter).
Regards,
Tvrtko
>> +bool 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)
>> + return false;
>> +
>> + spin_lock_irqsave(&client->objects_lock, flags);
>> + list_del_rcu(&obj->client_link);
>> + spin_unlock_irqrestore(&client->objects_lock, flags);
>> +
>> + i915_drm_client_put(client);
>> +
>> + return true;
>> +}
>> #endif
>> diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
>> index 67816c912bca..5f58fdf7dcb8 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,20 @@ 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
>> + /**
>> + * @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.
>> */
>> @@ -49,4 +66,19 @@ struct i915_drm_client *i915_drm_client_alloc(void);
>>
>> void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
>>
>> +#ifdef CONFIG_PROC_FS
>> +void i915_drm_client_add_object(struct i915_drm_client *client,
>> + struct drm_i915_gem_object *obj);
>> +bool 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 bool i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
>> +{
>> +}
>> +#endif
>> +
>> #endif /* !__I915_DRM_CLIENT_H__ */
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-07-10 13:20 ` Tvrtko Ursulin
@ 2023-07-11 7:48 ` Iddamsetty, Aravind
2023-07-11 9:39 ` Tvrtko Ursulin
0 siblings, 1 reply; 17+ messages in thread
From: Iddamsetty, Aravind @ 2023-07-11 7:48 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx, dri-devel
On 10-07-2023 18:50, Tvrtko Ursulin wrote:
>
> On 10/07/2023 11:44, Iddamsetty, Aravind wrote:
>> On 07-07-2023 18:32, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>
>>> In order to show per client memory usage lets add some infrastructure
>>> which enables tracking buffer objects owned by clients.
>>>
>>> We add a per client list protected by a new per client lock and to
>>> support
>>> delayed destruction (post client exit) we make tracked objects hold
>>> references to the owning client.
>>>
>>> Also, object memory region teardown is moved to the existing RCU free
>>> callback to allow safe dereference from the fdinfo RCU read section.
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +++++--
>>> .../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 | 32 +++++++++++++++++
>>> 4 files changed, 90 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>> index 97ac6fb37958..3dc4fbb67d2b 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);
>>> @@ -292,6 +296,10 @@ void __i915_gem_free_object_rcu(struct
>>> rcu_head *head)
>>> container_of(head, typeof(*obj), rcu);
>>> struct drm_i915_private *i915 = to_i915(obj->base.dev);
>>> + /* We need to keep this alive for RCU read access from fdinfo. */
>>> + if (obj->mm.n_placements > 1)
>>> + kfree(obj->mm.placements);
>>> +
>>> i915_gem_object_free(obj);
>>> GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
>>> @@ -388,9 +396,6 @@ void __i915_gem_free_object(struct
>>> drm_i915_gem_object *obj)
>>> if (obj->ops->release)
>>> obj->ops->release(obj);
>>> - if (obj->mm.n_placements > 1)
>>> - kfree(obj->mm.placements);
>>> -
>>> if (obj->shares_resv_from)
>>> i915_vm_resv_put(obj->shares_resv_from);
>>> @@ -441,6 +446,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..2e5e69edc0f9 100644
>>> --- a/drivers/gpu/drm/i915/i915_drm_client.c
>>> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
>>> @@ -28,6 +28,10 @@ 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);
>>> +#endif
>>> return client;
>>> }
>>> @@ -108,4 +112,36 @@ 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_rcu(&obj->client_link, &client->objects_list);
>>> + spin_unlock_irqrestore(&client->objects_lock, flags);
>>> +}
>>
>> would it be nice to mention that we use this client infra only to track
>> internal objects. While the user created through file->object_idr added
>> during handle creation time.
>
> In this series it is indeed only used for that.
>
> But it would be nicer to use it to track everything, so fdinfo readers
> would not be hitting the idr lock, which would avoid injecting latency
> to real DRM clients.
>
> The only fly in the ointment IMO is that I needed that drm core helper
> to be able to track dmabuf imports. Possibly something for flink too,
> did not look into that yet.
wouldn't dmabuf be tracked via object_idr as a new handle is created for it.
Thanks,
Aravind.
>
> In the light of all that I can mention in the cover letter next time
> round. It is a bit stale anyway (the cover letter).
>
> Regards,
>
> Tvrtko
>
>>> +bool 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)
>>> + return false;
>>> +
>>> + spin_lock_irqsave(&client->objects_lock, flags);
>>> + list_del_rcu(&obj->client_link);
>>> + spin_unlock_irqrestore(&client->objects_lock, flags);
>>> +
>>> + i915_drm_client_put(client);
>>> +
>>> + return true;
>>> +}
>>> #endif
>>> diff --git a/drivers/gpu/drm/i915/i915_drm_client.h
>>> b/drivers/gpu/drm/i915/i915_drm_client.h
>>> index 67816c912bca..5f58fdf7dcb8 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,20 @@ 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
>>> + /**
>>> + * @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.
>>> */
>>> @@ -49,4 +66,19 @@ struct i915_drm_client *i915_drm_client_alloc(void);
>>> void i915_drm_client_fdinfo(struct drm_printer *p, struct
>>> drm_file *file);
>>> +#ifdef CONFIG_PROC_FS
>>> +void i915_drm_client_add_object(struct i915_drm_client *client,
>>> + struct drm_i915_gem_object *obj);
>>> +bool 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 bool i915_drm_client_remove_object(struct
>>> drm_i915_gem_object *obj)
>>> +{
>>> +}
>>> +#endif
>>> +
>>> #endif /* !__I915_DRM_CLIENT_H__ */
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-07-11 7:48 ` Iddamsetty, Aravind
@ 2023-07-11 9:39 ` Tvrtko Ursulin
0 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-07-11 9:39 UTC (permalink / raw)
To: Iddamsetty, Aravind, Intel-gfx, dri-devel
On 11/07/2023 08:48, Iddamsetty, Aravind wrote:
> On 10-07-2023 18:50, Tvrtko Ursulin wrote:
>>
>> On 10/07/2023 11:44, Iddamsetty, Aravind wrote:
>>> On 07-07-2023 18:32, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> In order to show per client memory usage lets add some infrastructure
>>>> which enables tracking buffer objects owned by clients.
>>>>
>>>> We add a per client list protected by a new per client lock and to
>>>> support
>>>> delayed destruction (post client exit) we make tracked objects hold
>>>> references to the owning client.
>>>>
>>>> Also, object memory region teardown is moved to the existing RCU free
>>>> callback to allow safe dereference from the fdinfo RCU read section.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +++++--
>>>> .../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 | 32 +++++++++++++++++
>>>> 4 files changed, 90 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>> b/drivers/gpu/drm/i915/gem/i915_gem_object.c
>>>> index 97ac6fb37958..3dc4fbb67d2b 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);
>>>> @@ -292,6 +296,10 @@ void __i915_gem_free_object_rcu(struct
>>>> rcu_head *head)
>>>> container_of(head, typeof(*obj), rcu);
>>>> struct drm_i915_private *i915 = to_i915(obj->base.dev);
>>>> + /* We need to keep this alive for RCU read access from fdinfo. */
>>>> + if (obj->mm.n_placements > 1)
>>>> + kfree(obj->mm.placements);
>>>> +
>>>> i915_gem_object_free(obj);
>>>> GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
>>>> @@ -388,9 +396,6 @@ void __i915_gem_free_object(struct
>>>> drm_i915_gem_object *obj)
>>>> if (obj->ops->release)
>>>> obj->ops->release(obj);
>>>> - if (obj->mm.n_placements > 1)
>>>> - kfree(obj->mm.placements);
>>>> -
>>>> if (obj->shares_resv_from)
>>>> i915_vm_resv_put(obj->shares_resv_from);
>>>> @@ -441,6 +446,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..2e5e69edc0f9 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drm_client.c
>>>> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
>>>> @@ -28,6 +28,10 @@ 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);
>>>> +#endif
>>>> return client;
>>>> }
>>>> @@ -108,4 +112,36 @@ 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_rcu(&obj->client_link, &client->objects_list);
>>>> + spin_unlock_irqrestore(&client->objects_lock, flags);
>>>> +}
>>>
>>> would it be nice to mention that we use this client infra only to track
>>> internal objects. While the user created through file->object_idr added
>>> during handle creation time.
>>
>> In this series it is indeed only used for that.
>>
>> But it would be nicer to use it to track everything, so fdinfo readers
>> would not be hitting the idr lock, which would avoid injecting latency
>> to real DRM clients.
>>
>> The only fly in the ointment IMO is that I needed that drm core helper
>> to be able to track dmabuf imports. Possibly something for flink too,
>> did not look into that yet.
>
> wouldn't dmabuf be tracked via object_idr as a new handle is created for it.
Yes it is/would. I was talking about hypothetically not using object_idr
and instead tracking everything via the mechanism introduced in this
patch, which would allow for lockless fdinfo reads for everything. If
you remember I had that approach in an earlier version but it needed a
patch to drm code to split the prime helpers (or so) and also did not
cover the question on how to handle flink.
Regards,
Tvrtko
>
> Thanks,
> Aravind.
>>
>> In the light of all that I can mention in the cover letter next time
>> round. It is a bit stale anyway (the cover letter).
>>
>> Regards,
>>
>> Tvrtko
>>
>>>> +bool 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)
>>>> + return false;
>>>> +
>>>> + spin_lock_irqsave(&client->objects_lock, flags);
>>>> + list_del_rcu(&obj->client_link);
>>>> + spin_unlock_irqrestore(&client->objects_lock, flags);
>>>> +
>>>> + i915_drm_client_put(client);
>>>> +
>>>> + return true;
>>>> +}
>>>> #endif
>>>> diff --git a/drivers/gpu/drm/i915/i915_drm_client.h
>>>> b/drivers/gpu/drm/i915/i915_drm_client.h
>>>> index 67816c912bca..5f58fdf7dcb8 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,20 @@ 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
>>>> + /**
>>>> + * @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.
>>>> */
>>>> @@ -49,4 +66,19 @@ struct i915_drm_client *i915_drm_client_alloc(void);
>>>> void i915_drm_client_fdinfo(struct drm_printer *p, struct
>>>> drm_file *file);
>>>> +#ifdef CONFIG_PROC_FS
>>>> +void i915_drm_client_add_object(struct i915_drm_client *client,
>>>> + struct drm_i915_gem_object *obj);
>>>> +bool 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 bool i915_drm_client_remove_object(struct
>>>> drm_i915_gem_object *obj)
>>>> +{
>>>> +}
>>>> +#endif
>>>> +
>>>> #endif /* !__I915_DRM_CLIENT_H__ */
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-07-27 10:13 [Intel-gfx] [PATCH v6 0/5] fdinfo memory stats Tvrtko Ursulin
@ 2023-07-27 10:13 ` Tvrtko Ursulin
2023-08-03 5:30 ` Iddamsetty, Aravind
0 siblings, 1 reply; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-07-27 10:13 UTC (permalink / raw)
To: Intel-gfx, dri-devel
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
In order to show per client memory usage lets add some infrastructure
which enables tracking buffer objects owned by clients.
We add a per client list protected by a new per client lock and to support
delayed destruction (post client exit) we make tracked objects hold
references to the owning client.
Also, object memory region teardown is moved to the existing RCU free
callback to allow safe dereference from the fdinfo RCU read section.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +++++--
.../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 | 32 +++++++++++++++++
4 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 97ac6fb37958..3dc4fbb67d2b 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);
@@ -292,6 +296,10 @@ void __i915_gem_free_object_rcu(struct rcu_head *head)
container_of(head, typeof(*obj), rcu);
struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ /* We need to keep this alive for RCU read access from fdinfo. */
+ if (obj->mm.n_placements > 1)
+ kfree(obj->mm.placements);
+
i915_gem_object_free(obj);
GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
@@ -388,9 +396,6 @@ void __i915_gem_free_object(struct drm_i915_gem_object *obj)
if (obj->ops->release)
obj->ops->release(obj);
- if (obj->mm.n_placements > 1)
- kfree(obj->mm.placements);
-
if (obj->shares_resv_from)
i915_vm_resv_put(obj->shares_resv_from);
@@ -441,6 +446,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..2e5e69edc0f9 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -28,6 +28,10 @@ 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);
+#endif
return client;
}
@@ -108,4 +112,36 @@ 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_rcu(&obj->client_link, &client->objects_list);
+ spin_unlock_irqrestore(&client->objects_lock, flags);
+}
+
+bool 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)
+ return false;
+
+ spin_lock_irqsave(&client->objects_lock, flags);
+ list_del_rcu(&obj->client_link);
+ spin_unlock_irqrestore(&client->objects_lock, flags);
+
+ i915_drm_client_put(client);
+
+ return true;
+}
#endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 67816c912bca..5f58fdf7dcb8 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,20 @@ 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
+ /**
+ * @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.
*/
@@ -49,4 +66,19 @@ struct i915_drm_client *i915_drm_client_alloc(void);
void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
+#ifdef CONFIG_PROC_FS
+void i915_drm_client_add_object(struct i915_drm_client *client,
+ struct drm_i915_gem_object *obj);
+bool 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 bool i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
+{
+}
+#endif
+
#endif /* !__I915_DRM_CLIENT_H__ */
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-07-27 10:13 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
@ 2023-08-03 5:30 ` Iddamsetty, Aravind
0 siblings, 0 replies; 17+ messages in thread
From: Iddamsetty, Aravind @ 2023-08-03 5:30 UTC (permalink / raw)
To: Tvrtko Ursulin, Intel-gfx, dri-devel
On 27-07-2023 15:43, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> In order to show per client memory usage lets add some infrastructure
> which enables tracking buffer objects owned by clients.
>
> We add a per client list protected by a new per client lock and to support
> delayed destruction (post client exit) we make tracked objects hold
> references to the owning client.
>
> Also, object memory region teardown is moved to the existing RCU free
> callback to allow safe dereference from the fdinfo RCU read section.
This is same as the earlier series, which I had reviewed but forgot to
give r-b. sorry for the delay.
Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Thanks,
Aravind.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +++++--
> .../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 | 32 +++++++++++++++++
> 4 files changed, 90 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 97ac6fb37958..3dc4fbb67d2b 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);
>
> @@ -292,6 +296,10 @@ void __i915_gem_free_object_rcu(struct rcu_head *head)
> container_of(head, typeof(*obj), rcu);
> struct drm_i915_private *i915 = to_i915(obj->base.dev);
>
> + /* We need to keep this alive for RCU read access from fdinfo. */
> + if (obj->mm.n_placements > 1)
> + kfree(obj->mm.placements);
> +
> i915_gem_object_free(obj);
>
> GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
> @@ -388,9 +396,6 @@ void __i915_gem_free_object(struct drm_i915_gem_object *obj)
> if (obj->ops->release)
> obj->ops->release(obj);
>
> - if (obj->mm.n_placements > 1)
> - kfree(obj->mm.placements);
> -
> if (obj->shares_resv_from)
> i915_vm_resv_put(obj->shares_resv_from);
>
> @@ -441,6 +446,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..2e5e69edc0f9 100644
> --- a/drivers/gpu/drm/i915/i915_drm_client.c
> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> @@ -28,6 +28,10 @@ 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);
> +#endif
>
> return client;
> }
> @@ -108,4 +112,36 @@ 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_rcu(&obj->client_link, &client->objects_list);
> + spin_unlock_irqrestore(&client->objects_lock, flags);
> +}
> +
> +bool 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)
> + return false;
> +
> + spin_lock_irqsave(&client->objects_lock, flags);
> + list_del_rcu(&obj->client_link);
> + spin_unlock_irqrestore(&client->objects_lock, flags);
> +
> + i915_drm_client_put(client);
> +
> + return true;
> +}
> #endif
> diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
> index 67816c912bca..5f58fdf7dcb8 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,20 @@ 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
> + /**
> + * @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.
> */
> @@ -49,4 +66,19 @@ struct i915_drm_client *i915_drm_client_alloc(void);
>
> void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
>
> +#ifdef CONFIG_PROC_FS
> +void i915_drm_client_add_object(struct i915_drm_client *client,
> + struct drm_i915_gem_object *obj);
> +bool 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 bool i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
> +{
> +}
> +#endif
> +
> #endif /* !__I915_DRM_CLIENT_H__ */
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client
2023-09-21 11:48 [Intel-gfx] [PATCH v7 0/5] fdinfo memory stats Tvrtko Ursulin
@ 2023-09-21 11:48 ` Tvrtko Ursulin
0 siblings, 0 replies; 17+ messages in thread
From: Tvrtko Ursulin @ 2023-09-21 11:48 UTC (permalink / raw)
To: Intel-gfx, dri-devel
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
In order to show per client memory usage lets add some infrastructure
which enables tracking buffer objects owned by clients.
We add a per client list protected by a new per client lock and to support
delayed destruction (post client exit) we make tracked objects hold
references to the owning client.
Also, object memory region teardown is moved to the existing RCU free
callback to allow safe dereference from the fdinfo RCU read section.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_object.c | 13 +++++--
.../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 | 32 +++++++++++++++++
4 files changed, 90 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index c26d87555825..25eeeb863209 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -106,6 +106,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);
@@ -293,6 +297,10 @@ void __i915_gem_free_object_rcu(struct rcu_head *head)
container_of(head, typeof(*obj), rcu);
struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ /* We need to keep this alive for RCU read access from fdinfo. */
+ if (obj->mm.n_placements > 1)
+ kfree(obj->mm.placements);
+
i915_gem_object_free(obj);
GEM_BUG_ON(!atomic_read(&i915->mm.free_count));
@@ -389,9 +397,6 @@ void __i915_gem_free_object(struct drm_i915_gem_object *obj)
if (obj->ops->release)
obj->ops->release(obj);
- if (obj->mm.n_placements > 1)
- kfree(obj->mm.placements);
-
if (obj->shares_resv_from)
i915_vm_resv_put(obj->shares_resv_from);
@@ -442,6 +447,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 2292404007c8..0c5cdab278b6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -302,6 +302,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..2e5e69edc0f9 100644
--- a/drivers/gpu/drm/i915/i915_drm_client.c
+++ b/drivers/gpu/drm/i915/i915_drm_client.c
@@ -28,6 +28,10 @@ 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);
+#endif
return client;
}
@@ -108,4 +112,36 @@ 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_rcu(&obj->client_link, &client->objects_list);
+ spin_unlock_irqrestore(&client->objects_lock, flags);
+}
+
+bool 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)
+ return false;
+
+ spin_lock_irqsave(&client->objects_lock, flags);
+ list_del_rcu(&obj->client_link);
+ spin_unlock_irqrestore(&client->objects_lock, flags);
+
+ i915_drm_client_put(client);
+
+ return true;
+}
#endif
diff --git a/drivers/gpu/drm/i915/i915_drm_client.h b/drivers/gpu/drm/i915/i915_drm_client.h
index 67816c912bca..5f58fdf7dcb8 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,20 @@ 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
+ /**
+ * @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.
*/
@@ -49,4 +66,19 @@ struct i915_drm_client *i915_drm_client_alloc(void);
void i915_drm_client_fdinfo(struct drm_printer *p, struct drm_file *file);
+#ifdef CONFIG_PROC_FS
+void i915_drm_client_add_object(struct i915_drm_client *client,
+ struct drm_i915_gem_object *obj);
+bool 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 bool i915_drm_client_remove_object(struct drm_i915_gem_object *obj)
+{
+}
+#endif
+
#endif /* !__I915_DRM_CLIENT_H__ */
--
2.39.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-09-21 11:49 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-12 10:46 [Intel-gfx] [PATCH v4 0/5] fdinfo memory stats Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 2/5] drm/i915: Record which client owns a VM Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 3/5] drm/i915: Track page table backing store usage Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 4/5] drm/i915: Account ring buffer and context state storage Tvrtko Ursulin
2023-06-12 10:46 ` [Intel-gfx] [PATCH 5/5] drm/i915: Implement fdinfo memory stats printing Tvrtko Ursulin
2023-06-12 12:45 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for fdinfo memory stats (rev3) Patchwork
2023-06-12 12:45 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-06-12 13:02 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2023-07-07 13:02 [Intel-gfx] [PATCH v5 0/5] fdinfo memory stats Tvrtko Ursulin
2023-07-07 13:02 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
2023-07-10 10:44 ` Iddamsetty, Aravind
2023-07-10 13:20 ` Tvrtko Ursulin
2023-07-11 7:48 ` Iddamsetty, Aravind
2023-07-11 9:39 ` Tvrtko Ursulin
2023-07-27 10:13 [Intel-gfx] [PATCH v6 0/5] fdinfo memory stats Tvrtko Ursulin
2023-07-27 10:13 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
2023-08-03 5:30 ` Iddamsetty, Aravind
2023-09-21 11:48 [Intel-gfx] [PATCH v7 0/5] fdinfo memory stats Tvrtko Ursulin
2023-09-21 11:48 ` [Intel-gfx] [PATCH 1/5] drm/i915: Add ability for tracking buffer objects per client Tvrtko Ursulin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox