This is latest iteration of GPU memory tracepoints [1].
In the past, there were questions about the "big picture" of memory accounting [2], especially given related work on dma-buf heaps and DRM cgroups [3]. Also, there was a desire for a non-driver specific solution.
The great news is the dma-buf heaps work as recently landed [4]. It uses sys-fs and the plan is to use it in conjunction with the tracepoint solution [5]. We're aiming for the GPU tracepoint to calculate totals per DRM-instance (a proxy for per-process on Android) and per-DRM device.
The cgroups work looks terrific too and hopefully we can deduplicate code once that's merged. Though that's abit of an implementation detail, so long as the "GPU tracepoints" + "dma-buf heap stats" plan sounds good for Android.
This series modifies the GPU memory tracepoint API in a non-breaking fashion (patch 1), and adds accounting via the GEM subsystem (patches 2 --> 7). Given the multiple places where memory events happen, there's a bunch trace events scattered in various places. The hardest part is allocation, where each driver has their own API. If there's a better way, do say so.
The last patch is incomplete; we would like general feedback before proceeding further.
[1] https://lore.kernel.org/lkml/20200302235044.59163-1-zzyiwei@google.com/ [2] https://lists.freedesktop.org/archives/dri-devel/2021-January/295120.html [3] https://www.spinics.net/lists/cgroups/msg27867.html [4] https://www.spinics.net/lists/linux-doc/msg97788.html [5] https://source.android.com/devices/graphics/implement-dma-buf-gpu-mem
Gurchetan Singh (8): tracing/gpu: modify gpu_mem_total drm: add new tracepoint fields to drm_device and drm_file drm: add helper functions for gpu_mem_total and gpu_mem_instance drm: start using drm_gem_trace_gpu_mem_total drm: start using drm_gem_trace_gpu_mem_instance drm: track real and fake imports in drm_prime_member drm: trace memory import per DRM file drm: trace memory import per DRM device
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_gem.c | 65 +++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_internal.h | 4 +-- drivers/gpu/drm/drm_prime.c | 22 +++++++++--- include/drm/drm_device.h | 16 +++++++++ include/drm/drm_file.h | 16 +++++++++ include/drm/drm_gem.h | 7 ++++ include/trace/events/gpu_mem.h | 61 +++++++++++++++++++++---------- 8 files changed, 166 insertions(+), 26 deletions(-)
The existing gpu_mem_total tracepoint [1] is not currently used by any in-tree consumers, we should add some.
In addition, there's a desire to report imported memory via the counters too [2].
To do this, we'll have to redefine the event to:
a) Change 'pid' to 'ctx_id'
The reason is DRM subsystem is created with GEM objects, DRM devices and DRM files in mind. A GEM object is associated with DRM device, and it may be shared between one or more DRM files.
Per-instance (or "context") counters make more sense than per-process counters for DRM. For GPUs that per process counters (kgsl), this change is backwards compatible.
b) add an "import_mem_total" field
We're just appending a field, so no problem here. Change "size" to "mem_total" as well (name changes are backwards compatible).
[1] https://lore.kernel.org/r/20200302234840.57188-1-zzyiwei@google.com/ [2] https://www.spinics.net/lists/kernel/msg4062769.html
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- include/trace/events/gpu_mem.h | 61 ++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/include/trace/events/gpu_mem.h b/include/trace/events/gpu_mem.h index 26d871f96e94..198b87f50356 100644 --- a/include/trace/events/gpu_mem.h +++ b/include/trace/events/gpu_mem.h @@ -14,41 +14,66 @@ #include <linux/tracepoint.h>
/* - * The gpu_memory_total event indicates that there's an update to either the - * global or process total gpu memory counters. + * The gpu_mem_total event indicates that there's an update to local or + * global gpu memory counters. * - * This event should be emitted whenever the kernel device driver allocates, - * frees, imports, unimports memory in the GPU addressable space. + * This event should be emitted whenever a GPU device (ctx_id == 0): * - * @gpu_id: This is the gpu id. + * 1) allocates memory. + * 2) frees memory. + * 3) imports memory from an external exporter. * - * @pid: Put 0 for global total, while positive pid for process total. + * OR when a GPU device instance (ctx_id != 0): * - * @size: Size of the allocation in bytes. + * 1) allocates or acquires a reference to memory from another instance. + * 2) frees or releases a reference to memory from another instance. + * 3) imports memory from another GPU device instance. * + * When ctx_id == 0, both mem_total and import_mem_total total counters + * represent a global total. When ctx_id == 0, these counters represent + * an instance specifical total. + * + * Note allocation does not necessarily mean backing the memory with pages. + * + * @gpu_id: unique ID of the GPU. + * + * @ctx_id: an ID for specific instance of the GPU device. + * + * @mem_total: - total size of memory known to a GPU device, including + * imports (ctx_id == 0) + * - total size of memory known to a GPU device instance + * (ctx_id != 0) + * + * @import_mem_total: - size of memory imported from outside GPU + * device (ctx_id == 0) + * - size of memory imported into GPU device instance. + * (ctx_id == 0) */ TRACE_EVENT(gpu_mem_total,
- TP_PROTO(uint32_t gpu_id, uint32_t pid, uint64_t size), + TP_PROTO(u32 gpu_id, u32 ctx_id, u64 mem_total, u64 import_mem_total),
- TP_ARGS(gpu_id, pid, size), + TP_ARGS(gpu_id, ctx_id, mem_total, import_mem_total),
TP_STRUCT__entry( - __field(uint32_t, gpu_id) - __field(uint32_t, pid) - __field(uint64_t, size) + __field(u32, gpu_id) + __field(u32, ctx_id) + __field(u64, mem_total) + __field(u64, import_mem_total) ),
TP_fast_assign( __entry->gpu_id = gpu_id; - __entry->pid = pid; - __entry->size = size; + __entry->ctx_id = ctx_id; + __entry->mem_total = mem_total; + __entry->import_mem_total = import_mem_total; ),
- TP_printk("gpu_id=%u pid=%u size=%llu", - __entry->gpu_id, - __entry->pid, - __entry->size) + TP_printk("gpu_id=%u, ctx_id=%u, mem total=%llu, mem import total=%llu", + __entry->gpu_id, + __entry->ctx_id, + __entry->mem_total, + __entry->import_mem_total) );
#endif /* _TRACE_GPU_MEM_H */
On Wed, Oct 20, 2021 at 08:10:20PM -0700, Gurchetan Singh wrote:
The existing gpu_mem_total tracepoint [1] is not currently used by any in-tree consumers, we should add some.
In addition, there's a desire to report imported memory via the counters too [2].
To do this, we'll have to redefine the event to:
a) Change 'pid' to 'ctx_id'
The reason is DRM subsystem is created with GEM objects, DRM devices and DRM files in mind. A GEM object is associated with DRM device, and it may be shared between one or more DRM files.
Per-instance (or "context") counters make more sense than per-process counters for DRM. For GPUs that per process counters (kgsl), this change is backwards compatible.
b) add an "import_mem_total" field
We're just appending a field, so no problem here. Change "size" to "mem_total" as well (name changes are backwards compatible).
[1] https://lore.kernel.org/r/20200302234840.57188-1-zzyiwei@google.com/ [2] https://www.spinics.net/lists/kernel/msg4062769.html
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org
Yay, that patch is just impressive. Lands a new gpu tracepoints, never even showed up on the gpu subsystem discussion list.
Imo just delete this and start over with something proper, that's actually used by drivers. -Daniel
include/trace/events/gpu_mem.h | 61 ++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/include/trace/events/gpu_mem.h b/include/trace/events/gpu_mem.h index 26d871f96e94..198b87f50356 100644 --- a/include/trace/events/gpu_mem.h +++ b/include/trace/events/gpu_mem.h @@ -14,41 +14,66 @@ #include <linux/tracepoint.h>
/*
- The gpu_memory_total event indicates that there's an update to either the
- global or process total gpu memory counters.
- The gpu_mem_total event indicates that there's an update to local or
- global gpu memory counters.
- This event should be emitted whenever the kernel device driver allocates,
- frees, imports, unimports memory in the GPU addressable space.
- This event should be emitted whenever a GPU device (ctx_id == 0):
- @gpu_id: This is the gpu id.
- allocates memory.
- frees memory.
- imports memory from an external exporter.
- @pid: Put 0 for global total, while positive pid for process total.
- OR when a GPU device instance (ctx_id != 0):
- @size: Size of the allocation in bytes.
- allocates or acquires a reference to memory from another instance.
- frees or releases a reference to memory from another instance.
- imports memory from another GPU device instance.
- When ctx_id == 0, both mem_total and import_mem_total total counters
- represent a global total. When ctx_id == 0, these counters represent
- an instance specifical total.
- Note allocation does not necessarily mean backing the memory with pages.
- @gpu_id: unique ID of the GPU.
- @ctx_id: an ID for specific instance of the GPU device.
- @mem_total: - total size of memory known to a GPU device, including
imports (ctx_id == 0)
- total size of memory known to a GPU device instance
(ctx_id != 0)
- @import_mem_total: - size of memory imported from outside GPU
device (ctx_id == 0)
- size of memory imported into GPU device instance.*/
(ctx_id == 0)TRACE_EVENT(gpu_mem_total,
- TP_PROTO(uint32_t gpu_id, uint32_t pid, uint64_t size),
- TP_PROTO(u32 gpu_id, u32 ctx_id, u64 mem_total, u64 import_mem_total),
- TP_ARGS(gpu_id, pid, size),
TP_ARGS(gpu_id, ctx_id, mem_total, import_mem_total),
TP_STRUCT__entry(
__field(uint32_t, gpu_id)__field(uint32_t, pid)__field(uint64_t, size)
__field(u32, gpu_id)__field(u32, ctx_id)__field(u64, mem_total)__field(u64, import_mem_total)),
TP_fast_assign( __entry->gpu_id = gpu_id;
__entry->pid = pid;__entry->size = size;
__entry->ctx_id = ctx_id;__entry->mem_total = mem_total;__entry->import_mem_total = import_mem_total;
- TP_printk("gpu_id=%u pid=%u size=%llu",
__entry->gpu_id,__entry->pid,__entry->size)
- TP_printk("gpu_id=%u, ctx_id=%u, mem total=%llu, mem import total=%llu",
__entry->gpu_id,__entry->ctx_id,__entry->mem_total,__entry->import_mem_total));
#endif /* _TRACE_GPU_MEM_H */
2.25.1
On Thu, 21 Oct 2021 13:56:38 +0200 Daniel Vetter daniel@ffwll.ch wrote:
Yay, that patch is just impressive. Lands a new gpu tracepoints, never even showed up on the gpu subsystem discussion list.
I'm guessing there was some confusion. When this was first introduced, I stated it needs to go into the gpu tree, which a new set of patches included more Cc's. I never checked if those Cc's were for the GPU maintainers or not (I assumed they were).
https://lore.kernel.org/all/20200224113805.134f8b95@gandalf.local.home/
I'm not sure where Yiwei Zhang got his email list for the GPU maintainers from. As he obviously thought it was going to them.
https://lore.kernel.org/all/CAKT=dDnFpj2hJd5z73pfcrhXXacDpPVyKzC7+K94tsX=+e_...
Seeing that this patch set is going through dri-devel list, which I'm guessing is also for GPU, even though it's not under that in the MAINTAINERS file:
DRM DRIVERS AND MISC GPU PATCHES M: Maarten Lankhorst maarten.lankhorst@linux.intel.com M: Maxime Ripard mripard@kernel.org M: Thomas Zimmermann tzimmermann@suse.de S: Maintained W: https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/gpu/ F: drivers/gpu/drm/* F: drivers/gpu/vga/ F: include/drm/drm* F: include/linux/vga* F: include/uapi/drm/drm*
Should the list be added there?
I've been struggling with this patch set because nobody claimed ownership for it. But now I believe we have one (you? :-) And I can now just comment on the tracing POV and leave the content and usability to you folks ;-)
-- Steve
On Thu, Oct 21, 2021 at 09:07:15AM -0400, Steven Rostedt wrote:
On Thu, 21 Oct 2021 13:56:38 +0200 Daniel Vetter daniel@ffwll.ch wrote:
Yay, that patch is just impressive. Lands a new gpu tracepoints, never even showed up on the gpu subsystem discussion list.
I'm guessing there was some confusion. When this was first introduced, I stated it needs to go into the gpu tree, which a new set of patches included more Cc's. I never checked if those Cc's were for the GPU maintainers or not (I assumed they were).
https://lore.kernel.org/all/20200224113805.134f8b95@gandalf.local.home/
I'm not sure where Yiwei Zhang got his email list for the GPU maintainers from. As he obviously thought it was going to them.
https://lore.kernel.org/all/CAKT=dDnFpj2hJd5z73pfcrhXXacDpPVyKzC7+K94tsX=+e_...
Seeing that this patch set is going through dri-devel list, which I'm guessing is also for GPU, even though it's not under that in the MAINTAINERS file:
DRM DRIVERS AND MISC GPU PATCHES M: Maarten Lankhorst maarten.lankhorst@linux.intel.com M: Maxime Ripard mripard@kernel.org M: Thomas Zimmermann tzimmermann@suse.de S: Maintained W: https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html T: git git://anongit.freedesktop.org/drm/drm-misc F: Documentation/gpu/ F: drivers/gpu/drm/* F: drivers/gpu/vga/ F: include/drm/drm* F: include/linux/vga* F: include/uapi/drm/drm*
Should the list be added there?
I've been struggling with this patch set because nobody claimed ownership for it. But now I believe we have one (you? :-) And I can now just comment on the tracing POV and leave the content and usability to you folks ;-)
Hm we indeed don't have an entry for drivers/gpu, but there is one for gpu/drm overall:
DRM DRIVERS M: David Airlie airlied@linux.ie M: Daniel Vetter daniel@ffwll.ch L: dri-devel@lists.freedesktop.org S: Maintained B: https://gitlab.freedesktop.org/drm C: irc://irc.oftc.net/dri-devel T: git git://anongit.freedesktop.org/drm/drm F: Documentation/devicetree/bindings/display/ F: Documentation/devicetree/bindings/gpu/ F: Documentation/gpu/ F: drivers/gpu/drm/ F: drivers/gpu/vga/ F: include/drm/ F: include/linux/vga* F: include/uapi/drm/
I'll do a patch to include drivers/gpu here, not sure why that's different. -Daniel
For struct drm_device, add: - mem_total - import_mem_total
For struct drm_file, add: - mem_instance - import_mem_instance
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- include/drm/drm_device.h | 16 ++++++++++++++++ include/drm/drm_file.h | 16 ++++++++++++++++ 2 files changed, 32 insertions(+)
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 604b1d1b2d72..35a96bda5320 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -298,6 +298,22 @@ struct drm_device { */ struct drm_fb_helper *fb_helper;
+ /** + * @mem_total: + * + * The total size of all GEM objects known to this DRM device. Used + * with `gpu_mem_total` tracepoint. + */ + atomic64_t mem_total; + + /** + * @import_mem_total: + * + * The total size of all GEM objects imported into this DRM device from + * external exporters. Used with `gpu_mem_total` tracepoint. + */ + atomic64_t import_mem_total; + /* Everything below here is for legacy driver, never use! */ /* private: */ #if IS_ENABLED(CONFIG_DRM_LEGACY) diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index a3acb7ac3550..a5b9befcf1db 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -362,6 +362,22 @@ struct drm_file { */ struct drm_prime_file_private prime;
+ /** + * @mem_instance: + * + * The total size of all GEM objects known into this instance of the DRM + * device. Used with `gpu_mem_instance` tracepoint. + */ + atomic64_t mem_instance; + + /** + * @import_mem_instance: + * + * The total size of all GEM objects imported into this instance of the + * DRM device. Used with `gpu_mem_instance` tracepoint. + */ + atomic64_t import_mem_instance; + /* private: */ #if IS_ENABLED(CONFIG_DRM_LEGACY) unsigned long lock_count; /* DRI1 legacy lock count */
- Add helper functions for above tracepoints in the drm_gem.{h,c} files
- Given more tracepoints, a drm_trace.* file may be started
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_gem.c | 49 +++++++++++++++++++++++++++++++++++++++ include/drm/drm_gem.h | 7 ++++++ 3 files changed, 57 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index b91f0ce8154c..cef8545df1c9 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -15,6 +15,7 @@ menuconfig DRM select I2C_ALGOBIT select DMA_SHARED_BUFFER select SYNC_FILE + select TRACE_GPU_MEM # gallium uses SYS_kcmp for os_same_file_description() to de-duplicate # device and dmabuf fd. Let's make sure that is available for our userspace. select KCMP diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 4dcdec6487bb..24a719b79400 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -49,6 +49,8 @@ #include <drm/drm_print.h> #include <drm/drm_vma_manager.h>
+#include <trace/events/gpu_mem.h> + #include "drm_internal.h"
/** @file drm_gem.c @@ -138,6 +140,53 @@ int drm_gem_object_init(struct drm_device *dev, } EXPORT_SYMBOL(drm_gem_object_init);
+/** + * drm_gem_trace_gpu_mem_total - emit a total memory trace event + * @dev: drm_device to emit trace event for + * @delta: size change + * @imported: whether the imported or total memory counter should be used + * + * Emits a `gpu_mem_total` trace event with given parameters. + */ +void +drm_gem_trace_gpu_mem_total(struct drm_device *dev, s64 delta, bool imported) +{ + if (imported) + atomic64_add(delta, &dev->import_mem_total); + else + atomic64_add(delta, &dev->mem_total); + + trace_gpu_mem_total(dev->primary->index, 0, + atomic64_read(&dev->mem_total), + atomic64_read(&dev->import_mem_total)); +} +EXPORT_SYMBOL(drm_gem_trace_gpu_mem_total); + +/** + * drm_gem_trace_gpu_mem_instance - emit a per instance memory trace event + * @dev: drm_device associated with DRM file + * @file: drm_file to emit event for + * @delta: size change + * @imported: whether the imported or total memory counter should be used + * + * Emits a `gpu_mem_instance` trace event with given parameters. + */ +void +drm_gem_trace_gpu_mem_instance(struct drm_device *dev, struct drm_file *file, + s64 delta, bool imported) +{ + if (imported) + atomic64_add(delta, &file->import_mem_instance); + else + atomic64_add(delta, &file->mem_instance); + + trace_gpu_mem_total(dev->primary->index, + file_inode(file->filp)->i_ino, + atomic64_read(&file->mem_instance), + atomic64_read(&file->import_mem_instance)); +} +EXPORT_SYMBOL(drm_gem_trace_gpu_mem_instance); + /** * drm_gem_private_object_init - initialize an allocated private GEM object * @dev: drm_device the object should be initialized for diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 35e7f44c2a75..d61937cce222 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -342,6 +342,13 @@ struct drm_gem_object {
void drm_gem_object_release(struct drm_gem_object *obj); void drm_gem_object_free(struct kref *kref); + +void drm_gem_trace_gpu_mem_total(struct drm_device *dev, s64 delta, + bool imported); +void drm_gem_trace_gpu_mem_instance(struct drm_device *dev, + struct drm_file *file, + s64 delta, bool imported); + int drm_gem_object_init(struct drm_device *dev, struct drm_gem_object *obj, size_t size); void drm_gem_private_object_init(struct drm_device *dev,
- drm_gem_private_object_init(..) increases the total memory counter.
* All GEM objects (whether allocated or imported) seem to begin there. * If there's a better place/method, please do let me know.
- drm_gem_object_free(..) decreases the total memory counter.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/gpu/drm/drm_gem.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 24a719b79400..528d7b29dccf 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -213,6 +213,7 @@ void drm_gem_private_object_init(struct drm_device *dev, obj->resv = &obj->_resv;
drm_vma_node_reset(&obj->vma_node); + drm_gem_trace_gpu_mem_total(dev, obj->size, false); } EXPORT_SYMBOL(drm_gem_private_object_init);
@@ -1015,6 +1016,10 @@ drm_gem_object_free(struct kref *kref) struct drm_gem_object *obj = container_of(kref, struct drm_gem_object, refcount);
+ struct drm_device *dev = obj->dev; + + drm_gem_trace_gpu_mem_total(dev, -obj->size, false); + if (WARN_ON(!obj->funcs->free)) return;
On Wed, 20 Oct 2021 20:10:23 -0700 Gurchetan Singh gurchetansingh@chromium.org wrote:
drm_gem_private_object_init(..) increases the total memory counter.
- All GEM objects (whether allocated or imported) seem to begin there.
- If there's a better place/method, please do let me know.
drm_gem_object_free(..) decreases the total memory counter.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org
drivers/gpu/drm/drm_gem.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 24a719b79400..528d7b29dccf 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -213,6 +213,7 @@ void drm_gem_private_object_init(struct drm_device *dev, obj->resv = &obj->_resv;
drm_vma_node_reset(&obj->vma_node);
To save yourself a function call when tracing is disabled, you can add:
if (trace_gpu_mem_total_enabled())
here, which is a static_branch (meaning it's not a compare and branch, but a nop when tracing is disabled, and a jmp (to the if block) when the event is enabled).
- drm_gem_trace_gpu_mem_total(dev, obj->size, false);
} EXPORT_SYMBOL(drm_gem_private_object_init);
@@ -1015,6 +1016,10 @@ drm_gem_object_free(struct kref *kref) struct drm_gem_object *obj = container_of(kref, struct drm_gem_object, refcount);
- struct drm_device *dev = obj->dev;
Same here.
-- Steve
- drm_gem_trace_gpu_mem_total(dev, -obj->size, false);
- if (WARN_ON(!obj->funcs->free)) return;
- drm_gem_handle_create_tail(..) increases the per instance memory counter.
- drm_gem_object_release_handle(..) decreases the per instance memory counter.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/gpu/drm/drm_gem.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 528d7b29dccf..6f70419f2c90 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -298,6 +298,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) { struct drm_file *file_priv = data; struct drm_gem_object *obj = ptr; + struct drm_device *dev = file_priv->minor->dev;
if (obj->funcs->close) obj->funcs->close(obj, file_priv); @@ -305,6 +306,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) drm_gem_remove_prime_handles(obj, file_priv); drm_vma_node_revoke(&obj->vma_node, file_priv);
+ drm_gem_trace_gpu_mem_instance(dev, file_priv, -obj->size, false); drm_gem_object_handle_put_unlocked(obj);
return 0; @@ -447,6 +449,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, goto err_revoke; }
+ drm_gem_trace_gpu_mem_instance(dev, file_priv, obj->size, false); *handlep = handle; return 0;
On Wed, 20 Oct 2021 20:10:24 -0700 Gurchetan Singh gurchetansingh@chromium.org wrote:
@@ -305,6 +306,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data) drm_gem_remove_prime_handles(obj, file_priv); drm_vma_node_revoke(&obj->vma_node, file_priv);
- drm_gem_trace_gpu_mem_instance(dev, file_priv, -obj->size, false);
I would suggest adding the trace_*_enabled() if statements around all these callers.
-- Steve
drm_gem_object_handle_put_unlocked(obj);
return 0;
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: a31246115b33b3c3ab456e3f689174a076f09bbf ("[RFC PATCH 5/8] drm: start using drm_gem_trace_gpu_mem_instance") url: https://github.com/0day-ci/linux/commits/Gurchetan-Singh/GPU-memory-tracepoi... base: git://anongit.freedesktop.org/drm-intel for-linux-next patch link: https://lore.kernel.org/dri-devel/20211021031027.537-6-gurchetansingh@chromi...
in testcase: boot
on test machine: qemu-system-x86_64 -enable-kvm -cpu Icelake-Server -smp 4 -m 16G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+---------------------------------------------+------------+------------+ | | 4f27e9667d | a31246115b | +---------------------------------------------+------------+------------+ | boot_successes | 16 | 0 | | boot_failures | 0 | 16 | | BUG:kernel_NULL_pointer_dereference,address | 0 | 16 | | Oops:#[##] | 0 | 16 | | RIP:drm_gem_trace_gpu_mem_instance | 0 | 16 | | Kernel_panic-not_syncing:Fatal_exception | 0 | 16 | +---------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag Reported-by: kernel test robot oliver.sang@intel.com
[ 23.584758][ T1] BUG: kernel NULL pointer dereference, address: 0000000000000020 [ 23.586495][ T1] #PF: supervisor read access in kernel mode [ 23.587820][ T1] #PF: error_code(0x0000) - not-present page [ 23.589173][ T1] PGD 0 P4D 0 [ 23.589661][ T1] Oops: 0000 [#1] SMP [ 23.589661][ T1] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 5.15.0-rc2-01062-ga31246115b33 #1 [ 23.589661][ T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 [ 23.589661][ T1] RIP: 0010:drm_gem_trace_gpu_mem_instance (drivers/gpu/drm/drm_gem.c:184) [ 23.589661][ T1] Code: 48 83 05 2c 56 e0 05 01 e8 c7 c1 0b ff 48 8b 83 d0 01 00 00 4c 8b ab 78 04 00 00 4c 8b a3 70 04 00 00 48 83 05 7a 54 e0 05 01 <48> 8b 40 20 48 8b 58 40 48 8b 85 80 00 00 00 8b 28 66 90 e8 92 c1 All code ======== 0: 48 83 05 2c 56 e0 05 addq $0x1,0x5e0562c(%rip) # 0x5e05634 7: 01 8: e8 c7 c1 0b ff callq 0xffffffffff0bc1d4 d: 48 8b 83 d0 01 00 00 mov 0x1d0(%rbx),%rax 14: 4c 8b ab 78 04 00 00 mov 0x478(%rbx),%r13 1b: 4c 8b a3 70 04 00 00 mov 0x470(%rbx),%r12 22: 48 83 05 7a 54 e0 05 addq $0x1,0x5e0547a(%rip) # 0x5e054a4 29: 01 2a:* 48 8b 40 20 mov 0x20(%rax),%rax <-- trapping instruction 2e: 48 8b 58 40 mov 0x40(%rax),%rbx 32: 48 8b 85 80 00 00 00 mov 0x80(%rbp),%rax 39: 8b 28 mov (%rax),%ebp 3b: 66 90 xchg %ax,%ax 3d: e8 .byte 0xe8 3e: 92 xchg %eax,%edx 3f: c1 .byte 0xc1
Code starting with the faulting instruction =========================================== 0: 48 8b 40 20 mov 0x20(%rax),%rax 4: 48 8b 58 40 mov 0x40(%rax),%rbx 8: 48 8b 85 80 00 00 00 mov 0x80(%rbp),%rax f: 8b 28 mov (%rax),%ebp 11: 66 90 xchg %ax,%ax 13: e8 .byte 0xe8 14: 92 xchg %eax,%edx 15: c1 .byte 0xc1 [ 23.589661][ T1] RSP: 0000:ffffc900000138f8 EFLAGS: 00010202 [ 23.589661][ T1] RAX: 0000000000000000 RBX: ffff888114cd4800 RCX: 0000000000000000 [ 23.589661][ T1] RDX: ffff8881002d8000 RSI: ffffffff8221ce49 RDI: ffff88810f9c6000 [ 23.589661][ T1] RBP: ffff88810f9c6000 R08: 0000000000000000 R09: 0000000000000001 [ 23.589661][ T1] R10: 00000000e4a45f4b R11: 000000000000007f R12: 0000000000300000 [ 23.589661][ T1] R13: 0000000000000000 R14: ffff888114cd48e0 R15: ffff88810f985418 [ 23.589661][ T1] FS: 0000000000000000(0000) GS:ffff88842fa00000(0000) knlGS:0000000000000000 [ 23.589661][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 23.589661][ T1] CR2: 0000000000000020 CR3: 0000000004e6a000 CR4: 0000000000000ea0 [ 23.589661][ T1] Call Trace: [ 23.589661][ T1] drm_gem_handle_create_tail (drivers/gpu/drm/drm_gem.c:452) [ 23.589661][ T1] drm_gem_handle_create (drivers/gpu/drm/drm_gem.c:486) [ 23.589661][ T1] drm_gem_vram_fill_create_dumb (drivers/gpu/drm/drm_gem_vram_helper.c:527) [ 23.589661][ T1] drm_gem_vram_driver_dumb_create (drivers/gpu/drm/drm_gem_vram_helper.c:624) [ 23.589661][ T1] drm_mode_create_dumb (drivers/gpu/drm/drm_dumb_buffers.c:96) [ 23.589661][ T1] drm_client_framebuffer_create (drivers/gpu/drm/drm_client.c:268 drivers/gpu/drm/drm_client.c:418) [ 23.589661][ T1] drm_fb_helper_generic_probe (drivers/gpu/drm/drm_fb_helper.c:2321 (discriminator 4)) [ 23.589661][ T1] drm_fb_helper_single_fb_probe (drivers/gpu/drm/drm_fb_helper.c:1668) [ 23.589661][ T1] __drm_fb_helper_initial_config_and_unlock (drivers/gpu/drm/drm_fb_helper.c:1827) [ 23.589661][ T1] drm_fb_helper_initial_config (drivers/gpu/drm/drm_fb_helper.c:1921) [ 23.589661][ T1] drm_fbdev_client_hotplug (drivers/gpu/drm/drm_fb_helper.c:2423) [ 23.589661][ T1] drm_fbdev_generic_setup (drivers/gpu/drm/drm_fb_helper.c:2510) [ 23.589661][ T1] bochs_pci_probe (drivers/gpu/drm/tiny/bochs.c:667) [ 23.589661][ T1] local_pci_probe (drivers/pci/pci-driver.c:323) [ 23.589661][ T1] pci_device_probe (drivers/pci/pci-driver.c:380 drivers/pci/pci-driver.c:405 drivers/pci/pci-driver.c:448) [ 23.589661][ T1] ? pci_device_remove (drivers/pci/pci-driver.c:433) [ 23.589661][ T1] really_probe (drivers/base/dd.c:515 drivers/base/dd.c:596) [ 23.589661][ T1] __driver_probe_device (drivers/base/dd.c:751) [ 23.589661][ T1] driver_probe_device (drivers/base/dd.c:781) [ 23.589661][ T1] __driver_attach (drivers/base/dd.c:1141) [ 23.589661][ T1] ? driver_allows_async_probing (drivers/base/dd.c:1093) [ 23.589661][ T1] bus_for_each_dev (drivers/base/bus.c:301) [ 23.589661][ T1] driver_attach (drivers/base/dd.c:1157) [ 23.589661][ T1] bus_add_driver (drivers/base/bus.c:618) [ 23.589661][ T1] driver_register (drivers/base/driver.c:171) [ 23.589661][ T1] __pci_register_driver (drivers/pci/pci-driver.c:1407) [ 23.589661][ T1] ? ch7006_init (drivers/gpu/drm/tiny/bochs.c:721) [ 23.589661][ T1] bochs_init (drivers/gpu/drm/tiny/bochs.c:728) [ 23.589661][ T1] do_one_initcall (init/main.c:1303) [ 23.589661][ T1] ? rcu_read_lock_sched_held (include/linux/lockdep.h:283 kernel/rcu/update.c:125) [ 23.589661][ T1] do_initcalls (init/main.c:1376 init/main.c:1392) [ 23.589661][ T1] kernel_init_freeable (init/main.c:1411 init/main.c:1614) [ 23.589661][ T1] ? rest_init (init/main.c:1497) [ 23.589661][ T1] kernel_init (init/main.c:1505) [ 23.589661][ T1] ret_from_fork (arch/x86/entry/entry_64.S:301) [ 23.589661][ T1] Modules linked in: [ 23.589661][ T1] CR2: 0000000000000020 [ 23.589661][ T1] ---[ end trace 2603038b65df9faf ]--- [ 23.589661][ T1] RIP: 0010:drm_gem_trace_gpu_mem_instance (drivers/gpu/drm/drm_gem.c:184) [ 23.589661][ T1] Code: 48 83 05 2c 56 e0 05 01 e8 c7 c1 0b ff 48 8b 83 d0 01 00 00 4c 8b ab 78 04 00 00 4c 8b a3 70 04 00 00 48 83 05 7a 54 e0 05 01 <48> 8b 40 20 48 8b 58 40 48 8b 85 80 00 00 00 8b 28 66 90 e8 92 c1 All code ======== 0: 48 83 05 2c 56 e0 05 addq $0x1,0x5e0562c(%rip) # 0x5e05634 7: 01 8: e8 c7 c1 0b ff callq 0xffffffffff0bc1d4 d: 48 8b 83 d0 01 00 00 mov 0x1d0(%rbx),%rax 14: 4c 8b ab 78 04 00 00 mov 0x478(%rbx),%r13 1b: 4c 8b a3 70 04 00 00 mov 0x470(%rbx),%r12 22: 48 83 05 7a 54 e0 05 addq $0x1,0x5e0547a(%rip) # 0x5e054a4 29: 01 2a:* 48 8b 40 20 mov 0x20(%rax),%rax <-- trapping instruction 2e: 48 8b 58 40 mov 0x40(%rax),%rbx 32: 48 8b 85 80 00 00 00 mov 0x80(%rbp),%rax 39: 8b 28 mov (%rax),%ebp 3b: 66 90 xchg %ax,%ax 3d: e8 .byte 0xe8 3e: 92 xchg %eax,%edx 3f: c1 .byte 0xc1
Code starting with the faulting instruction =========================================== 0: 48 8b 40 20 mov 0x20(%rax),%rax 4: 48 8b 58 40 mov 0x40(%rax),%rbx 8: 48 8b 85 80 00 00 00 mov 0x80(%rbp),%rax f: 8b 28 mov (%rax),%ebp 11: 66 90 xchg %ax,%ax 13: e8 .byte 0xe8 14: 92 xchg %eax,%edx 15: c1 .byte 0xc1
To reproduce:
# build kernel cd linux cp config-5.15.0-rc2-01062-ga31246115b33 .config make HOSTCC=gcc-9 CC=gcc-9 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
# if come across any failure that blocks the test, # please remove ~/.lkp and /lkp dir to run from a clean state.
--- 0DAY/LKP+ Test Infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/lkp@lists.01.org Intel Corporation
Thanks, Oliver Sang
Sometimes, an exported dma-buf is added to the import list. That messes up with trace point accounting, so track real vs. fake imports to correct this.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/gpu/drm/drm_gem.c | 5 ++++- drivers/gpu/drm/drm_internal.h | 4 ++-- drivers/gpu/drm/drm_prime.c | 18 +++++++++++++----- 3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 6f70419f2c90..7637be0ceb74 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -226,8 +226,11 @@ drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) */ mutex_lock(&filp->prime.lock); if (obj->dma_buf) { + struct drm_device *dev = filp->minor->dev; + bool removed_real_import = false; drm_prime_remove_buf_handle_locked(&filp->prime, - obj->dma_buf); + obj->dma_buf, + &removed_real_import); } mutex_unlock(&filp->prime.lock); } diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 17f3548c8ed2..40d572e46e2a 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -75,8 +75,8 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf); - + struct dma_buf *dma_buf, + bool *removed_real_import); /* drm_drv.c */ struct drm_minor *drm_minor_acquire(unsigned int minor_id); void drm_minor_release(struct drm_minor *minor); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index deb23dbec8b5..31f033ec8549 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -90,13 +90,15 @@ struct drm_prime_member { struct dma_buf *dma_buf; uint32_t handle; + bool fake_import;
struct rb_node dmabuf_rb; struct rb_node handle_rb; };
static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf, uint32_t handle) + struct dma_buf *dma_buf, uint32_t handle, + bool fake_import) { struct drm_prime_member *member; struct rb_node **p, *rb; @@ -108,6 +110,7 @@ static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv, get_dma_buf(dma_buf); member->dma_buf = dma_buf; member->handle = handle; + member->fake_import = fake_import;
rb = NULL; p = &prime_fpriv->dmabufs.rb_node; @@ -188,9 +191,11 @@ static int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpri }
void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpriv, - struct dma_buf *dma_buf) + struct dma_buf *dma_buf, + bool *removed_real_import) { struct rb_node *rb; + *removed_real_import = false;
rb = prime_fpriv->dmabufs.rb_node; while (rb) { @@ -201,6 +206,9 @@ void drm_prime_remove_buf_handle_locked(struct drm_prime_file_private *prime_fpr rb_erase(&member->handle_rb, &prime_fpriv->handles); rb_erase(&member->dmabuf_rb, &prime_fpriv->dmabufs);
+ if (!member->fake_import) + *removed_real_import = true; + dma_buf_put(dma_buf); kfree(member); return; @@ -303,7 +311,6 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, return PTR_ERR(dma_buf);
mutex_lock(&file_priv->prime.lock); - ret = drm_prime_lookup_buf_handle(&file_priv->prime, dma_buf, handle); if (ret == 0) @@ -315,6 +322,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, obj = dev->driver->gem_prime_import(dev, dma_buf); else obj = drm_gem_prime_import(dev, dma_buf); + if (IS_ERR(obj)) { ret = PTR_ERR(obj); goto out_unlock; @@ -334,7 +342,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, goto out_put;
ret = drm_prime_add_buf_handle(&file_priv->prime, - dma_buf, *handle); + dma_buf, *handle, false); mutex_unlock(&file_priv->prime.lock); if (ret) goto fail; @@ -473,7 +481,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, * ioctl doesn't miss to remove this buffer handle from the cache. */ ret = drm_prime_add_buf_handle(&file_priv->prime, - dmabuf, handle); + dmabuf, handle, true); mutex_unlock(&dev->object_name_lock); if (ret) goto fail_put_dmabuf;
- drm_gem_prime_fd_to_handle increases the per-instance imported memory counter
- drm_gem_remove_prime_handles decreases the per-instance imported memory counter on non-fake imports.
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/gpu/drm/drm_gem.c | 3 +++ drivers/gpu/drm/drm_prime.c | 2 ++ 2 files changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 7637be0ceb74..c07568ea8442 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -231,6 +231,9 @@ drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) drm_prime_remove_buf_handle_locked(&filp->prime, obj->dma_buf, &removed_real_import); + if (removed_real_import) + drm_gem_trace_gpu_mem_instance(dev, filp, -obj->size, + true); } mutex_unlock(&filp->prime.lock); } diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 31f033ec8549..1afcae0c4038 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -349,6 +349,8 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
dma_buf_put(dma_buf);
+ drm_gem_trace_gpu_mem_instance(dev, file_priv, obj->size, true); + return 0;
fail:
- drm_gem_prime_import_dev increases the per-device import memory counter
* Most drivers use it.
* drivers that have a (*gem_prime_import) callback will need additional changes, which can be done if everyone likes the overall RFC.
- drm_prime_gem_destroy decreases the per-device import memory counter.
* All drivers seem to use it?
Signed-off-by: Gurchetan Singh gurchetansingh@chromium.org --- drivers/gpu/drm/drm_prime.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 1afcae0c4038..c2057b7a63b4 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -955,6 +955,7 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
obj->import_attach = attach; obj->resv = dma_buf->resv; + drm_gem_trace_gpu_mem_total(dev, obj->size, true);
return obj;
@@ -1055,6 +1056,7 @@ void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *sg) struct dma_buf_attachment *attach; struct dma_buf *dma_buf;
+ drm_gem_trace_gpu_mem_total(obj->dev, -obj->size, true); attach = obj->import_attach; if (sg) dma_buf_unmap_attachment(attach, sg, DMA_BIDIRECTIONAL);
On Wed, Oct 20, 2021 at 08:10:19PM -0700, Gurchetan Singh wrote:
This is latest iteration of GPU memory tracepoints [1].
In the past, there were questions about the "big picture" of memory accounting [2], especially given related work on dma-buf heaps and DRM cgroups [3]. Also, there was a desire for a non-driver specific solution.
The great news is the dma-buf heaps work as recently landed [4]. It uses sys-fs and the plan is to use it in conjunction with the tracepoint solution [5]. We're aiming for the GPU tracepoint to calculate totals per DRM-instance (a proxy for per-process on Android) and per-DRM device.
The cgroups work looks terrific too and hopefully we can deduplicate code once that's merged. Though that's abit of an implementation detail, so long as the "GPU tracepoints" + "dma-buf heap stats" plan sounds good for Android.
Can we please start out with depulicated code, and integrate this with cgroups?
The problem with gpu memory account is that everyone wants their own thing, they're all slightly differently, and all supported by a different subset of drivers. That doesn't make sense to support in upstream at all.
Please huddle together so that there's one set of "track gpu memory" calls, and that does cgroups, tracepoints and everything else that an OS might want to have.
Also ideally this thing works for both integrated soc gpu (including an answer for special memory pools like cma) _and_ discrete gpus using ttm. Or at least has an answer to both, because again if we end up with totally different tracking for the soc vs the discrete gpu world, we've lost. -Daniel
This series modifies the GPU memory tracepoint API in a non-breaking fashion (patch 1), and adds accounting via the GEM subsystem (patches 2 --> 7). Given the multiple places where memory events happen, there's a bunch trace events scattered in various places. The hardest part is allocation, where each driver has their own API. If there's a better way, do say so.
The last patch is incomplete; we would like general feedback before proceeding further.
[1] https://lore.kernel.org/lkml/20200302235044.59163-1-zzyiwei@google.com/ [2] https://lists.freedesktop.org/archives/dri-devel/2021-January/295120.html [3] https://www.spinics.net/lists/cgroups/msg27867.html [4] https://www.spinics.net/lists/linux-doc/msg97788.html [5] https://source.android.com/devices/graphics/implement-dma-buf-gpu-mem
Gurchetan Singh (8): tracing/gpu: modify gpu_mem_total drm: add new tracepoint fields to drm_device and drm_file drm: add helper functions for gpu_mem_total and gpu_mem_instance drm: start using drm_gem_trace_gpu_mem_total drm: start using drm_gem_trace_gpu_mem_instance drm: track real and fake imports in drm_prime_member drm: trace memory import per DRM file drm: trace memory import per DRM device
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_gem.c | 65 +++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_internal.h | 4 +-- drivers/gpu/drm/drm_prime.c | 22 +++++++++--- include/drm/drm_device.h | 16 +++++++++ include/drm/drm_file.h | 16 +++++++++ include/drm/drm_gem.h | 7 ++++ include/trace/events/gpu_mem.h | 61 +++++++++++++++++++++---------- 8 files changed, 166 insertions(+), 26 deletions(-)
-- 2.25.1
On Thu, Oct 21, 2021 at 5:00 AM Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Oct 20, 2021 at 08:10:19PM -0700, Gurchetan Singh wrote:
This is latest iteration of GPU memory tracepoints [1].
In the past, there were questions about the "big picture" of memory accounting [2], especially given related work on dma-buf heaps and DRM cgroups [3]. Also, there was a desire for a non-driver specific solution.
The great news is the dma-buf heaps work as recently landed [4]. It uses sys-fs and the plan is to use it in conjunction with the tracepoint solution [5]. We're aiming for the GPU tracepoint to calculate totals per DRM-instance (a proxy for per-process on Android) and per-DRM device.
The cgroups work looks terrific too and hopefully we can deduplicate code once that's merged. Though that's abit of an implementation detail, so long as the "GPU tracepoints" + "dma-buf heap stats" plan sounds good for Android.
Can we please start out with depulicated code, and integrate this with cgroups?
Thanks for the comments Dan,
The cgroups work is currently targeting allocator attribution so it wouldn’t give insight to shared / imported memory - this is included as part of the totals in the tracepoint. We will start a separate discussion with the gpu community on including imported memory into cgroups design. Who would you recommend to be included? (in case we don't already know all the interested parties).
The current tracepoint and the cgroups are not conflicting designs but rather complimentary. These are some of the gaps which the tracepoint helps to cover: 1. Imported gpu memory accounting 2. The tracepoint can be used to track gpu memory usage over time (useful to detect memory usage spikes, for example), while cgroups can be used to view usage as a more granular and static state. 3. For systems where cgroups aren't enabled the tracepoint data can be a good alternative to identify memory issues. 4. Non-drm devices can make use of the tracepoint for reporting.
It would be great if we can also keep the tracepoint, as we don’t have another alternative that provides all it offers (cgroups can certainly be extended to cover some of these), and it's currently being used by all Android devices.
Thanks, Kalesh
The problem with gpu memory account is that everyone wants their own thing, they're all slightly differently, and all supported by a different subset of drivers. That doesn't make sense to support in upstream at all.
Please huddle together so that there's one set of "track gpu memory" calls, and that does cgroups, tracepoints and everything else that an OS might want to have.
Also ideally this thing works for both integrated soc gpu (including an answer for special memory pools like cma) _and_ discrete gpus using ttm. Or at least has an answer to both, because again if we end up with totally different tracking for the soc vs the discrete gpu world, we've lost. -Daniel
This series modifies the GPU memory tracepoint API in a non-breaking fashion (patch 1), and adds accounting via the GEM subsystem (patches 2 --> 7). Given the multiple places where memory events happen, there's a bunch trace events scattered in various places. The hardest part is allocation, where each driver has their own API. If there's a better way, do say so.
The last patch is incomplete; we would like general feedback before proceeding further.
[1] https://lore.kernel.org/lkml/20200302235044.59163-1-zzyiwei@google.com/ [2] https://lists.freedesktop.org/archives/dri-devel/2021-January/295120.html [3] https://www.spinics.net/lists/cgroups/msg27867.html [4] https://www.spinics.net/lists/linux-doc/msg97788.html [5] https://source.android.com/devices/graphics/implement-dma-buf-gpu-mem
Gurchetan Singh (8): tracing/gpu: modify gpu_mem_total drm: add new tracepoint fields to drm_device and drm_file drm: add helper functions for gpu_mem_total and gpu_mem_instance drm: start using drm_gem_trace_gpu_mem_total drm: start using drm_gem_trace_gpu_mem_instance drm: track real and fake imports in drm_prime_member drm: trace memory import per DRM file drm: trace memory import per DRM device
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_gem.c | 65 +++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_internal.h | 4 +-- drivers/gpu/drm/drm_prime.c | 22 +++++++++--- include/drm/drm_device.h | 16 +++++++++ include/drm/drm_file.h | 16 +++++++++ include/drm/drm_gem.h | 7 ++++ include/trace/events/gpu_mem.h | 61 +++++++++++++++++++++---------- 8 files changed, 166 insertions(+), 26 deletions(-)
-- 2.25.1
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Thu, Oct 21, 2021 at 3:38 PM Kalesh Singh kaleshsingh@google.com wrote:
On Thu, Oct 21, 2021 at 5:00 AM Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Oct 20, 2021 at 08:10:19PM -0700, Gurchetan Singh wrote:
This is latest iteration of GPU memory tracepoints [1].
In the past, there were questions about the "big picture" of memory accounting [2], especially given related work on dma-buf heaps and DRM cgroups [3]. Also, there was a desire for a non-driver specific solution.
The great news is the dma-buf heaps work as recently landed [4]. It uses sys-fs and the plan is to use it in conjunction with the tracepoint solution [5]. We're aiming for the GPU tracepoint to calculate totals per DRM-instance (a proxy for per-process on Android) and per-DRM device.
The cgroups work looks terrific too and hopefully we can deduplicate code once that's merged. Though that's abit of an implementation detail, so long as the "GPU tracepoints" + "dma-buf heap stats" plan sounds good for Android.
Can we please start out with depulicated code, and integrate this with cgroups?
Thanks for the comments Dan,
The cgroups work is currently targeting allocator attribution so it wouldn’t give insight to shared / imported memory - this is included as part of the totals in the tracepoint. We will start a separate discussion with the gpu community on including imported memory into cgroups design. Who would you recommend to be included? (in case we don't already know all the interested parties).
The current tracepoint and the cgroups are not conflicting designs but rather complimentary. These are some of the gaps which the tracepoint helps to cover:
- Imported gpu memory accounting
- The tracepoint can be used to track gpu memory usage over time
(useful to detect memory usage spikes, for example), while cgroups can be used to view usage as a more granular and static state. 3. For systems where cgroups aren't enabled the tracepoint data can be a good alternative to identify memory issues. 4. Non-drm devices can make use of the tracepoint for reporting.
It would be great if we can also keep the tracepoint, as we don’t have another alternative that provides all it offers (cgroups can certainly be extended to cover some of these), and it's currently being used by all Android devices.
Hi Daniel,
We had a follow up discussion with Kenny on using drm cgroups. In summary, we think that the tracepoints and croups here are orthogonal and should not block each other. Would appreciate any advice you have on moving this forward.
Thanks, Kalesh
Thanks, Kalesh
The problem with gpu memory account is that everyone wants their own thing, they're all slightly differently, and all supported by a different subset of drivers. That doesn't make sense to support in upstream at all.
Please huddle together so that there's one set of "track gpu memory" calls, and that does cgroups, tracepoints and everything else that an OS might want to have.
Also ideally this thing works for both integrated soc gpu (including an answer for special memory pools like cma) _and_ discrete gpus using ttm. Or at least has an answer to both, because again if we end up with totally different tracking for the soc vs the discrete gpu world, we've lost. -Daniel
This series modifies the GPU memory tracepoint API in a non-breaking fashion (patch 1), and adds accounting via the GEM subsystem (patches 2 --> 7). Given the multiple places where memory events happen, there's a bunch trace events scattered in various places. The hardest part is allocation, where each driver has their own API. If there's a better way, do say so.
The last patch is incomplete; we would like general feedback before proceeding further.
[1] https://lore.kernel.org/lkml/20200302235044.59163-1-zzyiwei@google.com/ [2] https://lists.freedesktop.org/archives/dri-devel/2021-January/295120.html [3] https://www.spinics.net/lists/cgroups/msg27867.html [4] https://www.spinics.net/lists/linux-doc/msg97788.html [5] https://source.android.com/devices/graphics/implement-dma-buf-gpu-mem
Gurchetan Singh (8): tracing/gpu: modify gpu_mem_total drm: add new tracepoint fields to drm_device and drm_file drm: add helper functions for gpu_mem_total and gpu_mem_instance drm: start using drm_gem_trace_gpu_mem_total drm: start using drm_gem_trace_gpu_mem_instance drm: track real and fake imports in drm_prime_member drm: trace memory import per DRM file drm: trace memory import per DRM device
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_gem.c | 65 +++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_internal.h | 4 +-- drivers/gpu/drm/drm_prime.c | 22 +++++++++--- include/drm/drm_device.h | 16 +++++++++ include/drm/drm_file.h | 16 +++++++++ include/drm/drm_gem.h | 7 ++++ include/trace/events/gpu_mem.h | 61 +++++++++++++++++++++---------- 8 files changed, 166 insertions(+), 26 deletions(-)
-- 2.25.1
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Mon, Nov 1, 2021 at 11:54 AM Kalesh Singh kaleshsingh@google.com wrote:
On Thu, Oct 21, 2021 at 3:38 PM Kalesh Singh kaleshsingh@google.com wrote:
On Thu, Oct 21, 2021 at 5:00 AM Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Oct 20, 2021 at 08:10:19PM -0700, Gurchetan Singh wrote:
This is latest iteration of GPU memory tracepoints [1].
In the past, there were questions about the "big picture" of memory accounting [2], especially given related work on dma-buf heaps and DRM cgroups [3]. Also, there was a desire for a non-driver specific solution.
The great news is the dma-buf heaps work as recently landed [4]. It uses sys-fs and the plan is to use it in conjunction with the tracepoint solution [5]. We're aiming for the GPU tracepoint to calculate totals per DRM-instance (a proxy for per-process on Android) and per-DRM device.
The cgroups work looks terrific too and hopefully we can deduplicate code once that's merged. Though that's abit of an implementation detail, so long as the "GPU tracepoints" + "dma-buf heap stats" plan sounds good for Android.
Can we please start out with depulicated code, and integrate this with cgroups?
Thanks for the comments Dan,
The cgroups work is currently targeting allocator attribution so it wouldn’t give insight to shared / imported memory - this is included as part of the totals in the tracepoint. We will start a separate discussion with the gpu community on including imported memory into cgroups design. Who would you recommend to be included? (in case we don't already know all the interested parties).
The current tracepoint and the cgroups are not conflicting designs but rather complimentary. These are some of the gaps which the tracepoint helps to cover:
- Imported gpu memory accounting
- The tracepoint can be used to track gpu memory usage over time
(useful to detect memory usage spikes, for example), while cgroups can be used to view usage as a more granular and static state. 3. For systems where cgroups aren't enabled the tracepoint data can be a good alternative to identify memory issues. 4. Non-drm devices can make use of the tracepoint for reporting.
It would be great if we can also keep the tracepoint, as we don’t have another alternative that provides all it offers (cgroups can certainly be extended to cover some of these), and it's currently being used by all Android devices.
Hi Daniel,
We had a follow up discussion with Kenny on using drm cgroups. In summary, we think that the tracepoints and croups here are orthogonal and should not block each other. Would appreciate any advice you have on moving this forward.
Hi Daniel,
Friendly ping on this. After discussion with Kenny, we think the tracepoint and cgroups are complimentary accounting mechanisms. One of the main use cases for the tracepoint in Android is for profiling GPU memory using tools like perfetto [1], instead of using periodic polling. Are there still objections to this? Please advise.
[1] https://perfetto.dev/docs/quickstart/android-tracing
Thanks, Kalesh
Thanks, Kalesh
Thanks, Kalesh
The problem with gpu memory account is that everyone wants their own thing, they're all slightly differently, and all supported by a different subset of drivers. That doesn't make sense to support in upstream at all.
Please huddle together so that there's one set of "track gpu memory" calls, and that does cgroups, tracepoints and everything else that an OS might want to have.
Also ideally this thing works for both integrated soc gpu (including an answer for special memory pools like cma) _and_ discrete gpus using ttm. Or at least has an answer to both, because again if we end up with totally different tracking for the soc vs the discrete gpu world, we've lost. -Daniel
This series modifies the GPU memory tracepoint API in a non-breaking fashion (patch 1), and adds accounting via the GEM subsystem (patches 2 --> 7). Given the multiple places where memory events happen, there's a bunch trace events scattered in various places. The hardest part is allocation, where each driver has their own API. If there's a better way, do say so.
The last patch is incomplete; we would like general feedback before proceeding further.
[1] https://lore.kernel.org/lkml/20200302235044.59163-1-zzyiwei@google.com/ [2] https://lists.freedesktop.org/archives/dri-devel/2021-January/295120.html [3] https://www.spinics.net/lists/cgroups/msg27867.html [4] https://www.spinics.net/lists/linux-doc/msg97788.html [5] https://source.android.com/devices/graphics/implement-dma-buf-gpu-mem
Gurchetan Singh (8): tracing/gpu: modify gpu_mem_total drm: add new tracepoint fields to drm_device and drm_file drm: add helper functions for gpu_mem_total and gpu_mem_instance drm: start using drm_gem_trace_gpu_mem_total drm: start using drm_gem_trace_gpu_mem_instance drm: track real and fake imports in drm_prime_member drm: trace memory import per DRM file drm: trace memory import per DRM device
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_gem.c | 65 +++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_internal.h | 4 +-- drivers/gpu/drm/drm_prime.c | 22 +++++++++--- include/drm/drm_device.h | 16 +++++++++ include/drm/drm_file.h | 16 +++++++++ include/drm/drm_gem.h | 7 ++++ include/trace/events/gpu_mem.h | 61 +++++++++++++++++++++---------- 8 files changed, 166 insertions(+), 26 deletions(-)
-- 2.25.1
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Gurchetan Singh -
Kalesh Singh -
kernel test robot -
Steven Rostedt