* [PATCH] Track total GPU memory for virtio driver @ 2021-01-18 22:46 Yiwei Zhang 2021-01-18 23:23 ` Yiwei Zhang 0 siblings, 1 reply; 15+ messages in thread From: Yiwei Zhang @ 2021-01-18 22:46 UTC (permalink / raw) To: David Airlie, Gerd Hoffmann, Daniel Vetter Cc: dri-devel, virtualization, linux-kernel, android-kernel-team, Yiwei Zhang On the success of virtio_gpu_object_create, add size of newly allocated bo to the tracled total_mem. In drm_gem_object_funcs.free, after the gem bo lost its last refcount, subtract the bo size from the tracked total_mem if the original underlying memory allocation is successful. Signed-off-by: Yiwei Zhang <zzyiwei@android.com> --- drivers/gpu/drm/virtio/Kconfig | 1 + drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++++ drivers/gpu/drm/virtio/virtgpu_object.c | 19 +++++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig index b925b8b1da16..e103b7e883b1 100644 --- a/drivers/gpu/drm/virtio/Kconfig +++ b/drivers/gpu/drm/virtio/Kconfig @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU select DRM_KMS_HELPER select DRM_GEM_SHMEM_HELPER select VIRTIO_DMA_SHARED_BUFFER + select TRACE_GPU_MEM help This is the virtual GPU driver for virtio. It can be used with QEMU based VMMs (like KVM or Xen). diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 6a232553c99b..7c60e7486bc4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -249,6 +249,10 @@ struct virtio_gpu_device { spinlock_t resource_export_lock; /* protects map state and host_visible_mm */ spinlock_t host_visible_lock; + +#ifdef CONFIG_TRACE_GPU_MEM + atomic64_t total_mem; +#endif }; struct virtio_gpu_fpriv { diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index d69a5b6da553..1e16226cebbe 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -25,12 +25,29 @@ #include <linux/dma-mapping.h> #include <linux/moduleparam.h> +#ifdef CONFIG_TRACE_GPU_MEM +#include <trace/events/gpu_mem.h> +#endif #include "virtgpu_drv.h" static int virtio_gpu_virglrenderer_workaround = 1; module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); +#ifdef CONFIG_TRACE_GPU_MEM +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, + s64 delta) +{ + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); + + trace_gpu_mem_total(0, 0, total_mem); +} +#else +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *, s64) +{ +} +#endif + int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) { if (virtio_gpu_virglrenderer_workaround) { @@ -104,6 +121,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; if (bo->created) { + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); virtio_gpu_cmd_unref_resource(vgdev, bo); virtio_gpu_notify(vgdev); /* completion handler calls virtio_gpu_cleanup_object() */ @@ -265,6 +283,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, virtio_gpu_object_attach(vgdev, bo, ents, nents); } + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); *bo_ptr = bo; return 0; -- 2.30.0.284.gd98b1dd5eaa7-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] Track total GPU memory for virtio driver 2021-01-18 22:46 [PATCH] Track total GPU memory for virtio driver Yiwei Zhang @ 2021-01-18 23:23 ` Yiwei Zhang 2021-01-18 23:40 ` [PATCH v2] drm/virtio: " Yiwei Zhang 0 siblings, 1 reply; 15+ messages in thread From: Yiwei Zhang @ 2021-01-18 23:23 UTC (permalink / raw) To: David Airlie, Gerd Hoffmann, Daniel Vetter Cc: dri-devel, virtualization, linux-kernel, kernel-team On Mon, Jan 18, 2021 at 2:47 PM Yiwei Zhang <zzyiwei@android.com> wrote: > > On the success of virtio_gpu_object_create, add size of newly allocated > bo to the tracled total_mem. In drm_gem_object_funcs.free, after the gem > bo lost its last refcount, subtract the bo size from the tracked > total_mem if the original underlying memory allocation is successful. > > Signed-off-by: Yiwei Zhang <zzyiwei@android.com> > --- > drivers/gpu/drm/virtio/Kconfig | 1 + > drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++++ > drivers/gpu/drm/virtio/virtgpu_object.c | 19 +++++++++++++++++++ > 3 files changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig > index b925b8b1da16..e103b7e883b1 100644 > --- a/drivers/gpu/drm/virtio/Kconfig > +++ b/drivers/gpu/drm/virtio/Kconfig > @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU > select DRM_KMS_HELPER > select DRM_GEM_SHMEM_HELPER > select VIRTIO_DMA_SHARED_BUFFER > + select TRACE_GPU_MEM > help > This is the virtual GPU driver for virtio. It can be used with > QEMU based VMMs (like KVM or Xen). > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > index 6a232553c99b..7c60e7486bc4 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -249,6 +249,10 @@ struct virtio_gpu_device { > spinlock_t resource_export_lock; > /* protects map state and host_visible_mm */ > spinlock_t host_visible_lock; > + > +#ifdef CONFIG_TRACE_GPU_MEM > + atomic64_t total_mem; > +#endif > }; > > struct virtio_gpu_fpriv { > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > index d69a5b6da553..1e16226cebbe 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > @@ -25,12 +25,29 @@ > > #include <linux/dma-mapping.h> > #include <linux/moduleparam.h> > +#ifdef CONFIG_TRACE_GPU_MEM > +#include <trace/events/gpu_mem.h> > +#endif > > #include "virtgpu_drv.h" > > static int virtio_gpu_virglrenderer_workaround = 1; > module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); > > +#ifdef CONFIG_TRACE_GPU_MEM > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > + s64 delta) > +{ > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > + > + trace_gpu_mem_total(0, 0, total_mem); > +} > +#else > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *, s64) > +{ > +} > +#endif > + > int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) > { > if (virtio_gpu_virglrenderer_workaround) { > @@ -104,6 +121,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) > struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; > > if (bo->created) { > + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); > virtio_gpu_cmd_unref_resource(vgdev, bo); > virtio_gpu_notify(vgdev); > /* completion handler calls virtio_gpu_cleanup_object() */ > @@ -265,6 +283,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > virtio_gpu_object_attach(vgdev, bo, ents, nents); > } > > + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); > *bo_ptr = bo; > return 0; > > -- > 2.30.0.284.gd98b1dd5eaa7-goog > -CC android-kernel-team@google.com +CC kernel-team@android.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-18 23:23 ` Yiwei Zhang @ 2021-01-18 23:40 ` Yiwei Zhang 2021-01-19 7:03 ` Daniel Vetter 0 siblings, 1 reply; 15+ messages in thread From: Yiwei Zhang @ 2021-01-18 23:40 UTC (permalink / raw) To: David Airlie, Gerd Hoffmann, Daniel Vetter Cc: dri-devel, virtualization, linux-kernel, kernel-team, Yiwei Zhang On the success of virtio_gpu_object_create, add size of newly allocated bo to the tracled total_mem. In drm_gem_object_funcs.free, after the gem bo lost its last refcount, subtract the bo size from the tracked total_mem if the original underlying memory allocation is successful. Signed-off-by: Yiwei Zhang <zzyiwei@android.com> --- drivers/gpu/drm/virtio/Kconfig | 1 + drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++++ drivers/gpu/drm/virtio/virtgpu_object.c | 19 +++++++++++++++++++ 3 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig index b925b8b1da16..e103b7e883b1 100644 --- a/drivers/gpu/drm/virtio/Kconfig +++ b/drivers/gpu/drm/virtio/Kconfig @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU select DRM_KMS_HELPER select DRM_GEM_SHMEM_HELPER select VIRTIO_DMA_SHARED_BUFFER + select TRACE_GPU_MEM help This is the virtual GPU driver for virtio. It can be used with QEMU based VMMs (like KVM or Xen). diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 6a232553c99b..7c60e7486bc4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -249,6 +249,10 @@ struct virtio_gpu_device { spinlock_t resource_export_lock; /* protects map state and host_visible_mm */ spinlock_t host_visible_lock; + +#ifdef CONFIG_TRACE_GPU_MEM + atomic64_t total_mem; +#endif }; struct virtio_gpu_fpriv { diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index d69a5b6da553..1e16226cebbe 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -25,12 +25,29 @@ #include <linux/dma-mapping.h> #include <linux/moduleparam.h> +#ifdef CONFIG_TRACE_GPU_MEM +#include <trace/events/gpu_mem.h> +#endif #include "virtgpu_drv.h" static int virtio_gpu_virglrenderer_workaround = 1; module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); +#ifdef CONFIG_TRACE_GPU_MEM +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, + s64 delta) +{ + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); + + trace_gpu_mem_total(0, 0, total_mem); +} +#else +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *, s64) +{ +} +#endif + int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) { if (virtio_gpu_virglrenderer_workaround) { @@ -104,6 +121,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; if (bo->created) { + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); virtio_gpu_cmd_unref_resource(vgdev, bo); virtio_gpu_notify(vgdev); /* completion handler calls virtio_gpu_cleanup_object() */ @@ -265,6 +283,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, virtio_gpu_object_attach(vgdev, bo, ents, nents); } + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); *bo_ptr = bo; return 0; -- 2.30.0.284.gd98b1dd5eaa7-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-18 23:40 ` [PATCH v2] drm/virtio: " Yiwei Zhang @ 2021-01-19 7:03 ` Daniel Vetter 2021-01-19 19:08 ` Yiwei Zhang 0 siblings, 1 reply; 15+ messages in thread From: Daniel Vetter @ 2021-01-19 7:03 UTC (permalink / raw) To: Yiwei Zhang Cc: David Airlie, Gerd Hoffmann, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, Android Kernel Team On Tue, Jan 19, 2021 at 12:41 AM Yiwei Zhang <zzyiwei@android.com> wrote: > > On the success of virtio_gpu_object_create, add size of newly allocated > bo to the tracled total_mem. In drm_gem_object_funcs.free, after the gem > bo lost its last refcount, subtract the bo size from the tracked > total_mem if the original underlying memory allocation is successful. > > Signed-off-by: Yiwei Zhang <zzyiwei@android.com> Isn't this something that ideally we'd for everyone? Also tracepoint for showing the total feels like tracepoint abuse, usually we show totals somewhere in debugfs or similar, and tracepoint just for what's happening (i.e. which object got deleted/created). What is this for exactly? -Daniel > --- > drivers/gpu/drm/virtio/Kconfig | 1 + > drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++++ > drivers/gpu/drm/virtio/virtgpu_object.c | 19 +++++++++++++++++++ > 3 files changed, 24 insertions(+) > > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig > index b925b8b1da16..e103b7e883b1 100644 > --- a/drivers/gpu/drm/virtio/Kconfig > +++ b/drivers/gpu/drm/virtio/Kconfig > @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU > select DRM_KMS_HELPER > select DRM_GEM_SHMEM_HELPER > select VIRTIO_DMA_SHARED_BUFFER > + select TRACE_GPU_MEM > help > This is the virtual GPU driver for virtio. It can be used with > QEMU based VMMs (like KVM or Xen). > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > index 6a232553c99b..7c60e7486bc4 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -249,6 +249,10 @@ struct virtio_gpu_device { > spinlock_t resource_export_lock; > /* protects map state and host_visible_mm */ > spinlock_t host_visible_lock; > + > +#ifdef CONFIG_TRACE_GPU_MEM > + atomic64_t total_mem; > +#endif > }; > > struct virtio_gpu_fpriv { > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > index d69a5b6da553..1e16226cebbe 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > @@ -25,12 +25,29 @@ > > #include <linux/dma-mapping.h> > #include <linux/moduleparam.h> > +#ifdef CONFIG_TRACE_GPU_MEM > +#include <trace/events/gpu_mem.h> > +#endif > > #include "virtgpu_drv.h" > > static int virtio_gpu_virglrenderer_workaround = 1; > module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); > > +#ifdef CONFIG_TRACE_GPU_MEM > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > + s64 delta) > +{ > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > + > + trace_gpu_mem_total(0, 0, total_mem); > +} > +#else > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *, s64) > +{ > +} > +#endif > + > int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) > { > if (virtio_gpu_virglrenderer_workaround) { > @@ -104,6 +121,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) > struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; > > if (bo->created) { > + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); > virtio_gpu_cmd_unref_resource(vgdev, bo); > virtio_gpu_notify(vgdev); > /* completion handler calls virtio_gpu_cleanup_object() */ > @@ -265,6 +283,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > virtio_gpu_object_attach(vgdev, bo, ents, nents); > } > > + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); > *bo_ptr = bo; > return 0; > > -- > 2.30.0.284.gd98b1dd5eaa7-goog > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-19 7:03 ` Daniel Vetter @ 2021-01-19 19:08 ` Yiwei Zhang 2021-01-20 9:11 ` Daniel Vetter 0 siblings, 1 reply; 15+ messages in thread From: Yiwei Zhang @ 2021-01-19 19:08 UTC (permalink / raw) To: Daniel Vetter Cc: David Airlie, Gerd Hoffmann, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, Android Kernel Team On Mon, Jan 18, 2021 at 11:03 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Tue, Jan 19, 2021 at 12:41 AM Yiwei Zhang <zzyiwei@android.com> wrote: > > > > On the success of virtio_gpu_object_create, add size of newly allocated > > bo to the tracled total_mem. In drm_gem_object_funcs.free, after the gem > > bo lost its last refcount, subtract the bo size from the tracked > > total_mem if the original underlying memory allocation is successful. > > > > Signed-off-by: Yiwei Zhang <zzyiwei@android.com> > > Isn't this something that ideally we'd for everyone? Also tracepoint > for showing the total feels like tracepoint abuse, usually we show > totals somewhere in debugfs or similar, and tracepoint just for what's > happening (i.e. which object got deleted/created). > > What is this for exactly? > -Daniel > > > --- > > drivers/gpu/drm/virtio/Kconfig | 1 + > > drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++++ > > drivers/gpu/drm/virtio/virtgpu_object.c | 19 +++++++++++++++++++ > > 3 files changed, 24 insertions(+) > > > > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig > > index b925b8b1da16..e103b7e883b1 100644 > > --- a/drivers/gpu/drm/virtio/Kconfig > > +++ b/drivers/gpu/drm/virtio/Kconfig > > @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU > > select DRM_KMS_HELPER > > select DRM_GEM_SHMEM_HELPER > > select VIRTIO_DMA_SHARED_BUFFER > > + select TRACE_GPU_MEM > > help > > This is the virtual GPU driver for virtio. It can be used with > > QEMU based VMMs (like KVM or Xen). > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > > index 6a232553c99b..7c60e7486bc4 100644 > > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > > @@ -249,6 +249,10 @@ struct virtio_gpu_device { > > spinlock_t resource_export_lock; > > /* protects map state and host_visible_mm */ > > spinlock_t host_visible_lock; > > + > > +#ifdef CONFIG_TRACE_GPU_MEM > > + atomic64_t total_mem; > > +#endif > > }; > > > > struct virtio_gpu_fpriv { > > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > > index d69a5b6da553..1e16226cebbe 100644 > > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > > @@ -25,12 +25,29 @@ > > > > #include <linux/dma-mapping.h> > > #include <linux/moduleparam.h> > > +#ifdef CONFIG_TRACE_GPU_MEM > > +#include <trace/events/gpu_mem.h> > > +#endif > > > > #include "virtgpu_drv.h" > > > > static int virtio_gpu_virglrenderer_workaround = 1; > > module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > > + s64 delta) > > +{ > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > > + > > + trace_gpu_mem_total(0, 0, total_mem); > > +} > > +#else > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *, s64) > > +{ > > +} > > +#endif > > + > > int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) > > { > > if (virtio_gpu_virglrenderer_workaround) { > > @@ -104,6 +121,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) > > struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; > > > > if (bo->created) { > > + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); > > virtio_gpu_cmd_unref_resource(vgdev, bo); > > virtio_gpu_notify(vgdev); > > /* completion handler calls virtio_gpu_cleanup_object() */ > > @@ -265,6 +283,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > > virtio_gpu_object_attach(vgdev, bo, ents, nents); > > } > > > > + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); > > *bo_ptr = bo; > > return 0; > > > > -- > > 2.30.0.284.gd98b1dd5eaa7-goog > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch Thanks for your reply! Android Cuttlefish virtual platform is using the virtio-gpu driver, and we currently are carrying this small patch at the downstream side. This is essential for us because: (1) Android has deprecated debugfs on production devices already (2) Android GPU drivers are not DRM based, and this won't change in a short term. Android relies on this tracepoint + eBPF to make the GPU memory totals available at runtime on production devices, which has been enforced already. Not only game developers can have a reliable kernel total GPU memory to look at, but also Android leverages this to take GPU memory usage out from the system lost ram. I'm not sure whether the other DRM drivers would like to integrate this tracepoint(maybe upstream drivers will move away from debugfs later as well?), but at least we hope virtio-gpu can take this. Many thanks! Yiwei ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-19 19:08 ` Yiwei Zhang @ 2021-01-20 9:11 ` Daniel Vetter 2021-01-20 9:51 ` Yiwei Zhang 0 siblings, 1 reply; 15+ messages in thread From: Daniel Vetter @ 2021-01-20 9:11 UTC (permalink / raw) To: Yiwei Zhang Cc: Daniel Vetter, David Airlie, Gerd Hoffmann, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, Android Kernel Team On Tue, Jan 19, 2021 at 11:08:12AM -0800, Yiwei Zhang wrote: > On Mon, Jan 18, 2021 at 11:03 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Tue, Jan 19, 2021 at 12:41 AM Yiwei Zhang <zzyiwei@android.com> wrote: > > > > > > On the success of virtio_gpu_object_create, add size of newly allocated > > > bo to the tracled total_mem. In drm_gem_object_funcs.free, after the gem > > > bo lost its last refcount, subtract the bo size from the tracked > > > total_mem if the original underlying memory allocation is successful. > > > > > > Signed-off-by: Yiwei Zhang <zzyiwei@android.com> > > > > Isn't this something that ideally we'd for everyone? Also tracepoint > > for showing the total feels like tracepoint abuse, usually we show > > totals somewhere in debugfs or similar, and tracepoint just for what's > > happening (i.e. which object got deleted/created). > > > > What is this for exactly? > > -Daniel > > > > > --- > > > drivers/gpu/drm/virtio/Kconfig | 1 + > > > drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++++ > > > drivers/gpu/drm/virtio/virtgpu_object.c | 19 +++++++++++++++++++ > > > 3 files changed, 24 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig > > > index b925b8b1da16..e103b7e883b1 100644 > > > --- a/drivers/gpu/drm/virtio/Kconfig > > > +++ b/drivers/gpu/drm/virtio/Kconfig > > > @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU > > > select DRM_KMS_HELPER > > > select DRM_GEM_SHMEM_HELPER > > > select VIRTIO_DMA_SHARED_BUFFER > > > + select TRACE_GPU_MEM > > > help > > > This is the virtual GPU driver for virtio. It can be used with > > > QEMU based VMMs (like KVM or Xen). > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > > > index 6a232553c99b..7c60e7486bc4 100644 > > > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > > > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > > > @@ -249,6 +249,10 @@ struct virtio_gpu_device { > > > spinlock_t resource_export_lock; > > > /* protects map state and host_visible_mm */ > > > spinlock_t host_visible_lock; > > > + > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > + atomic64_t total_mem; > > > +#endif > > > }; > > > > > > struct virtio_gpu_fpriv { > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > > > index d69a5b6da553..1e16226cebbe 100644 > > > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > > > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > > > @@ -25,12 +25,29 @@ > > > > > > #include <linux/dma-mapping.h> > > > #include <linux/moduleparam.h> > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > +#include <trace/events/gpu_mem.h> > > > +#endif > > > > > > #include "virtgpu_drv.h" > > > > > > static int virtio_gpu_virglrenderer_workaround = 1; > > > module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > > > + s64 delta) > > > +{ > > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > > > + > > > + trace_gpu_mem_total(0, 0, total_mem); > > > +} > > > +#else > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *, s64) > > > +{ > > > +} > > > +#endif > > > + > > > int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) > > > { > > > if (virtio_gpu_virglrenderer_workaround) { > > > @@ -104,6 +121,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) > > > struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; > > > > > > if (bo->created) { > > > + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); > > > virtio_gpu_cmd_unref_resource(vgdev, bo); > > > virtio_gpu_notify(vgdev); > > > /* completion handler calls virtio_gpu_cleanup_object() */ > > > @@ -265,6 +283,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > > > virtio_gpu_object_attach(vgdev, bo, ents, nents); > > > } > > > > > > + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); > > > *bo_ptr = bo; > > > return 0; > > > > > > -- > > > 2.30.0.284.gd98b1dd5eaa7-goog > > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > Thanks for your reply! Android Cuttlefish virtual platform is using > the virtio-gpu driver, and we currently are carrying this small patch > at the downstream side. This is essential for us because: > (1) Android has deprecated debugfs on production devices already > (2) Android GPU drivers are not DRM based, and this won't change in a > short term. > > Android relies on this tracepoint + eBPF to make the GPU memory totals > available at runtime on production devices, which has been enforced > already. Not only game developers can have a reliable kernel total GPU > memory to look at, but also Android leverages this to take GPU memory > usage out from the system lost ram. > > I'm not sure whether the other DRM drivers would like to integrate > this tracepoint(maybe upstream drivers will move away from debugfs > later as well?), but at least we hope virtio-gpu can take this. There's already another proposal from Android people for tracking dma-buf (in dma-buf heaps/ion) usage. I think we need something which is overall integrated, otherwise we have a complete mess of partial solutions. Also there's work going on to add cgroups support to gpu drivers (pushed by amd and intel folks, latest rfc have been quite old), so that's another proposal for gpu memory usage tracking. Also for upstream we need something which works with upstream gpu drivers (even if you don't end up using that in shipping products). So that's another reason maybe why a quick hack in the virtio gpu driver isn't the best approach here. I guess a good approach would be if Android at least can get to something unified (gpu driver, virtio-gpu, dma-buf heaps), and then we need to figure out how to mesh that with the cgroups side somehow. Also note that at least on dma-buf we already have some other debug features (for android), so an overall "how does this all fit together" would be good. -Daniel > > Many thanks! > Yiwei -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-20 9:11 ` Daniel Vetter @ 2021-01-20 9:51 ` Yiwei Zhang 2021-01-20 12:37 ` Daniel Vetter 2021-01-20 13:33 ` Gerd Hoffmann 0 siblings, 2 replies; 15+ messages in thread From: Yiwei Zhang @ 2021-01-20 9:51 UTC (permalink / raw) To: Yiwei Zhang, David Airlie, Gerd Hoffmann, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, Android Kernel Team Cc: Daniel Vetter On Wed, Jan 20, 2021 at 1:11 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Tue, Jan 19, 2021 at 11:08:12AM -0800, Yiwei Zhang wrote: > > On Mon, Jan 18, 2021 at 11:03 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > > On Tue, Jan 19, 2021 at 12:41 AM Yiwei Zhang <zzyiwei@android.com> wrote: > > > > > > > > On the success of virtio_gpu_object_create, add size of newly allocated > > > > bo to the tracled total_mem. In drm_gem_object_funcs.free, after the gem > > > > bo lost its last refcount, subtract the bo size from the tracked > > > > total_mem if the original underlying memory allocation is successful. > > > > > > > > Signed-off-by: Yiwei Zhang <zzyiwei@android.com> > > > > > > Isn't this something that ideally we'd for everyone? Also tracepoint > > > for showing the total feels like tracepoint abuse, usually we show > > > totals somewhere in debugfs or similar, and tracepoint just for what's > > > happening (i.e. which object got deleted/created). > > > > > > What is this for exactly? > > > -Daniel > > > > > > > --- > > > > drivers/gpu/drm/virtio/Kconfig | 1 + > > > > drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++++ > > > > drivers/gpu/drm/virtio/virtgpu_object.c | 19 +++++++++++++++++++ > > > > 3 files changed, 24 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig > > > > index b925b8b1da16..e103b7e883b1 100644 > > > > --- a/drivers/gpu/drm/virtio/Kconfig > > > > +++ b/drivers/gpu/drm/virtio/Kconfig > > > > @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU > > > > select DRM_KMS_HELPER > > > > select DRM_GEM_SHMEM_HELPER > > > > select VIRTIO_DMA_SHARED_BUFFER > > > > + select TRACE_GPU_MEM > > > > help > > > > This is the virtual GPU driver for virtio. It can be used with > > > > QEMU based VMMs (like KVM or Xen). > > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > > > > index 6a232553c99b..7c60e7486bc4 100644 > > > > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > > > > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > > > > @@ -249,6 +249,10 @@ struct virtio_gpu_device { > > > > spinlock_t resource_export_lock; > > > > /* protects map state and host_visible_mm */ > > > > spinlock_t host_visible_lock; > > > > + > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > + atomic64_t total_mem; > > > > +#endif > > > > }; > > > > > > > > struct virtio_gpu_fpriv { > > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > > > > index d69a5b6da553..1e16226cebbe 100644 > > > > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > > > > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > > > > @@ -25,12 +25,29 @@ > > > > > > > > #include <linux/dma-mapping.h> > > > > #include <linux/moduleparam.h> > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > +#include <trace/events/gpu_mem.h> > > > > +#endif > > > > > > > > #include "virtgpu_drv.h" > > > > > > > > static int virtio_gpu_virglrenderer_workaround = 1; > > > > module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); > > > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > > > > + s64 delta) > > > > +{ > > > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > > > > + > > > > + trace_gpu_mem_total(0, 0, total_mem); > > > > +} > > > > +#else > > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *, s64) > > > > +{ > > > > +} > > > > +#endif > > > > + > > > > int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) > > > > { > > > > if (virtio_gpu_virglrenderer_workaround) { > > > > @@ -104,6 +121,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) > > > > struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; > > > > > > > > if (bo->created) { > > > > + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); > > > > virtio_gpu_cmd_unref_resource(vgdev, bo); > > > > virtio_gpu_notify(vgdev); > > > > /* completion handler calls virtio_gpu_cleanup_object() */ > > > > @@ -265,6 +283,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > > > > virtio_gpu_object_attach(vgdev, bo, ents, nents); > > > > } > > > > > > > > + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); > > > > *bo_ptr = bo; > > > > return 0; > > > > > > > > -- > > > > 2.30.0.284.gd98b1dd5eaa7-goog > > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > Thanks for your reply! Android Cuttlefish virtual platform is using > > the virtio-gpu driver, and we currently are carrying this small patch > > at the downstream side. This is essential for us because: > > (1) Android has deprecated debugfs on production devices already > > (2) Android GPU drivers are not DRM based, and this won't change in a > > short term. > > > > Android relies on this tracepoint + eBPF to make the GPU memory totals > > available at runtime on production devices, which has been enforced > > already. Not only game developers can have a reliable kernel total GPU > > memory to look at, but also Android leverages this to take GPU memory > > usage out from the system lost ram. > > > > I'm not sure whether the other DRM drivers would like to integrate > > this tracepoint(maybe upstream drivers will move away from debugfs > > later as well?), but at least we hope virtio-gpu can take this. > > There's already another proposal from Android people for tracking dma-buf > (in dma-buf heaps/ion) usage. I think we need something which is overall > integrated, otherwise we have a complete mess of partial solutions. > > Also there's work going on to add cgroups support to gpu drivers (pushed > by amd and intel folks, latest rfc have been quite old), so that's another > proposal for gpu memory usage tracking. > > Also for upstream we need something which works with upstream gpu drivers > (even if you don't end up using that in shipping products). So that's > another reason maybe why a quick hack in the virtio gpu driver isn't the > best approach here. > > I guess a good approach would be if Android at least can get to something > unified (gpu driver, virtio-gpu, dma-buf heaps), and then we need to > figure out how to mesh that with the cgroups side somehow. > > Also note that at least on dma-buf we already have some other debug > features (for android), so an overall "how does this all fit together" > would be good. > -Daniel > > > > > Many thanks! > > Yiwei > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch The entire story is to better explain Android system memory usage. They fit together so that the dma-bufs overlap can be removed. Android GPU vendors have integrated this tracepoint to track gpu memory usage total(mapped into the gpu address space), which consists of below: (1) directly allocated via physical page allocator (2) imported external memory backed by dma-bufs (3) allocated exportable memory backed by dma-bufs Our Android kernel team is leading the other side of effort to help remove the dma-bufs overlap(those mapped into a gpu device) as a joint effort, so that we can accurately explain the memory usage of the entire Android system. For virtio-gpu, since that's used by our reference platform Cuttlefish(Cloud Android), we have to integrate the same tracepoint as well to enforce the use of this tracepoint and the eBPF stuff built on top to support runtime query of gpu memory on production devices. For virtio-gpu at this moment, we only want to track GEM allocations since PRIME import is currently not supported/used in Cuttlefish. That's all we are doing in this small patch. Best, Yiwei ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-20 9:51 ` Yiwei Zhang @ 2021-01-20 12:37 ` Daniel Vetter 2021-01-20 13:33 ` Gerd Hoffmann 1 sibling, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2021-01-20 12:37 UTC (permalink / raw) To: Yiwei Zhang Cc: David Airlie, Gerd Hoffmann, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, Android Kernel Team On Wed, Jan 20, 2021 at 10:51 AM Yiwei Zhang <zzyiwei@android.com> wrote: > > On Wed, Jan 20, 2021 at 1:11 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Tue, Jan 19, 2021 at 11:08:12AM -0800, Yiwei Zhang wrote: > > > On Mon, Jan 18, 2021 at 11:03 PM Daniel Vetter <daniel@ffwll.ch> wrote: > > > > > > > > On Tue, Jan 19, 2021 at 12:41 AM Yiwei Zhang <zzyiwei@android.com> wrote: > > > > > > > > > > On the success of virtio_gpu_object_create, add size of newly allocated > > > > > bo to the tracled total_mem. In drm_gem_object_funcs.free, after the gem > > > > > bo lost its last refcount, subtract the bo size from the tracked > > > > > total_mem if the original underlying memory allocation is successful. > > > > > > > > > > Signed-off-by: Yiwei Zhang <zzyiwei@android.com> > > > > > > > > Isn't this something that ideally we'd for everyone? Also tracepoint > > > > for showing the total feels like tracepoint abuse, usually we show > > > > totals somewhere in debugfs or similar, and tracepoint just for what's > > > > happening (i.e. which object got deleted/created). > > > > > > > > What is this for exactly? > > > > -Daniel > > > > > > > > > --- > > > > > drivers/gpu/drm/virtio/Kconfig | 1 + > > > > > drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++++ > > > > > drivers/gpu/drm/virtio/virtgpu_object.c | 19 +++++++++++++++++++ > > > > > 3 files changed, 24 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig > > > > > index b925b8b1da16..e103b7e883b1 100644 > > > > > --- a/drivers/gpu/drm/virtio/Kconfig > > > > > +++ b/drivers/gpu/drm/virtio/Kconfig > > > > > @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU > > > > > select DRM_KMS_HELPER > > > > > select DRM_GEM_SHMEM_HELPER > > > > > select VIRTIO_DMA_SHARED_BUFFER > > > > > + select TRACE_GPU_MEM > > > > > help > > > > > This is the virtual GPU driver for virtio. It can be used with > > > > > QEMU based VMMs (like KVM or Xen). > > > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > > > > > index 6a232553c99b..7c60e7486bc4 100644 > > > > > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > > > > > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > > > > > @@ -249,6 +249,10 @@ struct virtio_gpu_device { > > > > > spinlock_t resource_export_lock; > > > > > /* protects map state and host_visible_mm */ > > > > > spinlock_t host_visible_lock; > > > > > + > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > > + atomic64_t total_mem; > > > > > +#endif > > > > > }; > > > > > > > > > > struct virtio_gpu_fpriv { > > > > > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > > > > > index d69a5b6da553..1e16226cebbe 100644 > > > > > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > > > > > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > > > > > @@ -25,12 +25,29 @@ > > > > > > > > > > #include <linux/dma-mapping.h> > > > > > #include <linux/moduleparam.h> > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > > +#include <trace/events/gpu_mem.h> > > > > > +#endif > > > > > > > > > > #include "virtgpu_drv.h" > > > > > > > > > > static int virtio_gpu_virglrenderer_workaround = 1; > > > > > module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); > > > > > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > > > > > + s64 delta) > > > > > +{ > > > > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > > > > > + > > > > > + trace_gpu_mem_total(0, 0, total_mem); > > > > > +} > > > > > +#else > > > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *, s64) > > > > > +{ > > > > > +} > > > > > +#endif > > > > > + > > > > > int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) > > > > > { > > > > > if (virtio_gpu_virglrenderer_workaround) { > > > > > @@ -104,6 +121,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) > > > > > struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; > > > > > > > > > > if (bo->created) { > > > > > + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); > > > > > virtio_gpu_cmd_unref_resource(vgdev, bo); > > > > > virtio_gpu_notify(vgdev); > > > > > /* completion handler calls virtio_gpu_cleanup_object() */ > > > > > @@ -265,6 +283,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > > > > > virtio_gpu_object_attach(vgdev, bo, ents, nents); > > > > > } > > > > > > > > > > + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); > > > > > *bo_ptr = bo; > > > > > return 0; > > > > > > > > > > -- > > > > > 2.30.0.284.gd98b1dd5eaa7-goog > > > > > > > > > > > > > > > > > -- > > > > Daniel Vetter > > > > Software Engineer, Intel Corporation > > > > http://blog.ffwll.ch > > > > > > Thanks for your reply! Android Cuttlefish virtual platform is using > > > the virtio-gpu driver, and we currently are carrying this small patch > > > at the downstream side. This is essential for us because: > > > (1) Android has deprecated debugfs on production devices already > > > (2) Android GPU drivers are not DRM based, and this won't change in a > > > short term. > > > > > > Android relies on this tracepoint + eBPF to make the GPU memory totals > > > available at runtime on production devices, which has been enforced > > > already. Not only game developers can have a reliable kernel total GPU > > > memory to look at, but also Android leverages this to take GPU memory > > > usage out from the system lost ram. > > > > > > I'm not sure whether the other DRM drivers would like to integrate > > > this tracepoint(maybe upstream drivers will move away from debugfs > > > later as well?), but at least we hope virtio-gpu can take this. > > > > There's already another proposal from Android people for tracking dma-buf > > (in dma-buf heaps/ion) usage. I think we need something which is overall > > integrated, otherwise we have a complete mess of partial solutions. > > > > Also there's work going on to add cgroups support to gpu drivers (pushed > > by amd and intel folks, latest rfc have been quite old), so that's another > > proposal for gpu memory usage tracking. > > > > Also for upstream we need something which works with upstream gpu drivers > > (even if you don't end up using that in shipping products). So that's > > another reason maybe why a quick hack in the virtio gpu driver isn't the > > best approach here. > > > > I guess a good approach would be if Android at least can get to something > > unified (gpu driver, virtio-gpu, dma-buf heaps), and then we need to > > figure out how to mesh that with the cgroups side somehow. > > > > Also note that at least on dma-buf we already have some other debug > > features (for android), so an overall "how does this all fit together" > > would be good. > > -Daniel > > > > > > > > Many thanks! > > > Yiwei > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > The entire story is to better explain Android system memory usage. > They fit together so that the dma-bufs overlap can be removed. > > Android GPU vendors have integrated this tracepoint to track gpu > memory usage total(mapped into the gpu address space), which consists > of below: > (1) directly allocated via physical page allocator > (2) imported external memory backed by dma-bufs > (3) allocated exportable memory backed by dma-bufs > > Our Android kernel team is leading the other side of effort to help > remove the dma-bufs overlap(those mapped into a gpu device) as a joint > effort, so that we can accurately explain the memory usage of the > entire Android system. > > For virtio-gpu, since that's used by our reference platform > Cuttlefish(Cloud Android), we have to integrate the same tracepoint as > well to enforce the use of this tracepoint and the eBPF stuff built on > top to support runtime query of gpu memory on production devices. For > virtio-gpu at this moment, we only want to track GEM allocations since > PRIME import is currently not supported/used in Cuttlefish. That's all > we are doing in this small patch. Ok if the plan is to have that as a hard requirement for android across all android uapi drivers, then - this needs to be done across all upstream drivers too (otherwise we don't have that uapi) - usual open source requirements for new uapi (but I don't think that should be a problem, these parts of android are all open I think) - figuring out the overlap with the dma-buf account, before we merge either Otherwise I don't see how this can work and be backed with upstreams "never break uapi" guarantee. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-20 9:51 ` Yiwei Zhang 2021-01-20 12:37 ` Daniel Vetter @ 2021-01-20 13:33 ` Gerd Hoffmann 2021-01-20 18:52 ` Yiwei Zhang 1 sibling, 1 reply; 15+ messages in thread From: Gerd Hoffmann @ 2021-01-20 13:33 UTC (permalink / raw) To: Yiwei Zhang Cc: David Airlie, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, Android Kernel Team, Daniel Vetter Hi, > > > > > + select TRACE_GPU_MEM > > > > > +#ifdef CONFIG_TRACE_GPU_MEM That doesn't make sense btw. > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > > > > > + s64 delta) > > > > > +{ > > > > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > > > > > + > > > > > + trace_gpu_mem_total(0, 0, total_mem); Hmm, so no per process tracking (pid arg hard-coded to zero)? Any plans for that? The cgroups patches mentioned by Daniel should address that btw. The gpu_id is hardcoded to zero too. Shouldn't that be something like the minor number of the drm device? Or maybe something else in case you need drm and non-drm gpu devices work side-by-side? > > > Thanks for your reply! Android Cuttlefish virtual platform is using > > > the virtio-gpu driver, and we currently are carrying this small patch > > > at the downstream side. This is essential for us because: > > > (1) Android has deprecated debugfs on production devices already IIRC there have been discussions about a statfs, so you can export stats with a sane interface without also enabling all the power provided by debugfs, exactly because of the concerns to do that on production systems. Not sure what the state is, seems to not be upstream yet. That would be (beside cgroups) another thing to look at. > > > Android relies on this tracepoint + eBPF to make the GPU memory totals > > > available at runtime on production devices, which has been enforced > > > already. Not only game developers can have a reliable kernel total GPU > > > memory to look at, but also Android leverages this to take GPU memory > > > usage out from the system lost ram. Sounds like you define "gpu memory" as "system memory used to store gpu data". Is that correct? What about device memory? > > > I'm not sure whether the other DRM drivers would like to integrate > > > this tracepoint(maybe upstream drivers will move away from debugfs > > > later as well?), but at least we hope virtio-gpu can take this. Well, it is basically the same for all drivers using the gem shmem helpers. So I see little reason why we should do that at virtio-gpu level. > Android GPU vendors have integrated this tracepoint to track gpu > memory usage total(mapped into the gpu address space), which consists > of below: > (1) directly allocated via physical page allocator > (2) imported external memory backed by dma-bufs > (3) allocated exportable memory backed by dma-bufs Hmm, the tracepoint doesn't track which of the three groups the memory belongs to. Which I think is important, specifically group (2) because that might already be accounted for by the exporting driver ... > Our Android kernel team is leading the other side of effort to help > remove the dma-bufs overlap(those mapped into a gpu device) as a joint > effort, so that we can accurately explain the memory usage of the > entire Android system. I suspect once you figured that you'll notice that this little hack is rather incomplete. > For virtio-gpu, since that's used by our reference platform > Cuttlefish(Cloud Android), we have to integrate the same tracepoint as > well to enforce the use of this tracepoint and the eBPF stuff built on > top to support runtime query of gpu memory on production devices. For > virtio-gpu at this moment, we only want to track GEM allocations since > PRIME import is currently not supported/used in Cuttlefish. That's all > we are doing in this small patch. take care, Gerd ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-20 13:33 ` Gerd Hoffmann @ 2021-01-20 18:52 ` Yiwei Zhang 2021-01-21 9:10 ` Gerd Hoffmann 0 siblings, 1 reply; 15+ messages in thread From: Yiwei Zhang @ 2021-01-20 18:52 UTC (permalink / raw) To: Gerd Hoffmann Cc: David Airlie, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, Android Kernel Team, Daniel Vetter On Wed, Jan 20, 2021 at 5:33 AM Gerd Hoffmann <kraxel@redhat.com> wrote: > > Hi, > > > > > > > + select TRACE_GPU_MEM > > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > That doesn't make sense btw. Do you recommend we just select it or leave it an option? > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > > > > > > + s64 delta) > > > > > > +{ > > > > > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > > > > > > + > > > > > > + trace_gpu_mem_total(0, 0, total_mem); > > Hmm, so no per process tracking (pid arg hard-coded to zero)? > Any plans for that? > The cgroups patches mentioned by Daniel should address that btw. Android GPU vendors do report the totals for each process as well. For Cuttlefish virtual platform, we haven't yet required that, and want to get the global total in place first. > The gpu_id is hardcoded to zero too. Shouldn't that be something like > the minor number of the drm device? Or maybe something else in case you > need drm and non-drm gpu devices work side-by-side? For drm driver integration, I'd definitely vote for using the primary->index. Non-drm gpu devices, if co-exists, need to avoid using the duplicate id if possible(though there's no system wide id). I'll send an update later. > > > > Thanks for your reply! Android Cuttlefish virtual platform is using > > > > the virtio-gpu driver, and we currently are carrying this small patch > > > > at the downstream side. This is essential for us because: > > > > (1) Android has deprecated debugfs on production devices already > > IIRC there have been discussions about a statfs, so you can export stats > with a sane interface without also enabling all the power provided by > debugfs, exactly because of the concerns to do that on production > systems. > > Not sure what the state is, seems to not be upstream yet. That would be > (beside cgroups) another thing to look at. > > > > > Android relies on this tracepoint + eBPF to make the GPU memory totals > > > > available at runtime on production devices, which has been enforced > > > > already. Not only game developers can have a reliable kernel total GPU > > > > memory to look at, but also Android leverages this to take GPU memory > > > > usage out from the system lost ram. > > Sounds like you define "gpu memory" as "system memory used to store gpu > data". Is that correct? What about device memory? The total definition does include all device memory being used as well for numa devices.(If my understanding of your question is correct.) > > > > I'm not sure whether the other DRM drivers would like to integrate > > > > this tracepoint(maybe upstream drivers will move away from debugfs > > > > later as well?), but at least we hope virtio-gpu can take this. > > Well, it is basically the same for all drivers using the gem shmem > helpers. So I see little reason why we should do that at virtio-gpu > level. This can be a starting point. Another reason would be I'm fearing that this tracepoint approach might be more difficult to get upstreamed at drm layer level, since later we may want to get to per-process total tracking, which would be making more sense at device driver level. > > Android GPU vendors have integrated this tracepoint to track gpu > > memory usage total(mapped into the gpu address space), which consists > > of below: > > (1) directly allocated via physical page allocator > > (2) imported external memory backed by dma-bufs > > (3) allocated exportable memory backed by dma-bufs > > Hmm, the tracepoint doesn't track which of the three groups the memory > belongs to. Which I think is important, specifically group (2) because > that might already be accounted for by the exporting driver ... The tracepoint only cares about a total number, but I'm not against the idea to extend the tracepoint with categorization. However, I believe the dma-bufs core can track which dma-buf gets attached/mapped to some devices. So that those overlap between dma-buf heaps and the gpu memory total we are tracking here can be canceled out. > > Our Android kernel team is leading the other side of effort to help > > remove the dma-bufs overlap(those mapped into a gpu device) as a joint > > effort, so that we can accurately explain the memory usage of the > > entire Android system. > > I suspect once you figured that you'll notice that this little hack is > rather incomplete. Despite the dma-buf side effort, we still wish to have this tracepoint integrated in virtio-gpu just for a global total at this moment. So we can have all enforcement in place for Android given the downstream drivers are still not drm-based. Upon system tracing, event based update via tracepoint is much more efficient and accurate than polling the fs nodes(too much overhead on SeLinux). > > For virtio-gpu, since that's used by our reference platform > > Cuttlefish(Cloud Android), we have to integrate the same tracepoint as > > well to enforce the use of this tracepoint and the eBPF stuff built on > > top to support runtime query of gpu memory on production devices. For > > virtio-gpu at this moment, we only want to track GEM allocations since > > PRIME import is currently not supported/used in Cuttlefish. That's all > > we are doing in this small patch. > > take care, > Gerd > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver 2021-01-20 18:52 ` Yiwei Zhang @ 2021-01-21 9:10 ` Gerd Hoffmann 2021-01-22 5:31 ` [PATCH v3] drm/virtio: trace total gem bo for virtio Yiwei Zhang 0 siblings, 1 reply; 15+ messages in thread From: Gerd Hoffmann @ 2021-01-21 9:10 UTC (permalink / raw) To: Yiwei Zhang Cc: David Airlie, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, Android Kernel Team, Daniel Vetter On Wed, Jan 20, 2021 at 10:52:11AM -0800, Yiwei Zhang wrote: > On Wed, Jan 20, 2021 at 5:33 AM Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > Hi, > > > > > > > > > + select TRACE_GPU_MEM > > > > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > That doesn't make sense btw. > > Do you recommend we just select it or leave it an option? The patch selects it (which makes sense given the small size). The #ifdef is pointless then ... > > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM > > > > > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > > > > > > > + s64 delta) > > > > > > > +{ > > > > > > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > > > > > > > + > > > > > > > + trace_gpu_mem_total(0, 0, total_mem); > > > > Hmm, so no per process tracking (pid arg hard-coded to zero)? > > Any plans for that? > > The cgroups patches mentioned by Daniel should address that btw. > > Android GPU vendors do report the totals for each process as well. For > Cuttlefish virtual platform, we haven't yet required that, and want to > get the global total in place first. That means no plans yet? > > > > > Android relies on this tracepoint + eBPF to make the GPU memory totals > > > > > available at runtime on production devices, which has been enforced > > > > > already. Not only game developers can have a reliable kernel total GPU > > > > > memory to look at, but also Android leverages this to take GPU memory > > > > > usage out from the system lost ram. > > > > Sounds like you define "gpu memory" as "system memory used to store gpu > > data". Is that correct? What about device memory? > > The total definition does include all device memory being used as well > for numa devices.(If my understanding of your question is correct.) device memory == gpu-owned memory, typically exposed to as pci memory bar. qemu stdvga for example stores gem objects in device memory (unless it runs out of vram, then ttm allocates from / moves into main memory). > > > > > I'm not sure whether the other DRM drivers would like to integrate > > > > > this tracepoint(maybe upstream drivers will move away from debugfs > > > > > later as well?), but at least we hope virtio-gpu can take this. > > > > Well, it is basically the same for all drivers using the gem shmem > > helpers. So I see little reason why we should do that at virtio-gpu > > level. > > This can be a starting point. Another reason would be I'm fearing that > this tracepoint approach might be more difficult to get upstreamed at > drm layer level, since later we may want to get to per-process total > tracking, which would be making more sense at device driver level. Tracking in __drm_gem_shmem_create + drm_gem_shmem_free_object should give you pretty much the same results, with the major difference being that it works for all shmem-based drivers. Of course just moving the trace points doesn't solve the other issues discussed. > > > Android GPU vendors have integrated this tracepoint to track gpu > > > memory usage total(mapped into the gpu address space), which consists > > > of below: > > > (1) directly allocated via physical page allocator > > > (2) imported external memory backed by dma-bufs > > > (3) allocated exportable memory backed by dma-bufs > > > > Hmm, the tracepoint doesn't track which of the three groups the memory > > belongs to. Which I think is important, specifically group (2) because > > that might already be accounted for by the exporting driver ... > > The tracepoint only cares about a total number, but I'm not against > the idea to extend the tracepoint with categorization. However, I > believe the dma-bufs core can track which dma-buf gets attached/mapped > to some devices. So that those overlap between dma-buf heaps and the > gpu memory total we are tracking here can be canceled out. Yep, maybe. Which is *exactly* why Daniel keeps asking for the big picture and how this integrates/interacts with the dma-buf accounting which seems to be in the works too. Note that dma-bufs are not only used for cross-device sharing. They are also used to pass handles from one application to another (application to wayland compositor or x server for example). Which doesn't matter much for the totals, but for per-process accounting you need a plan how to account these shared buffers. > > I suspect once you figured that you'll notice that this little hack is > > rather incomplete. > > Despite the dma-buf side effort, we still wish to have this tracepoint > integrated in virtio-gpu just for a global total at this moment. I don't feel like merging patches with obvious shortcomings which have a high chance to end up as technical dept. The question how this interacts with dma-buf accounting must be clarified. I'd also suggest to join forces with the cgroups people. The problem space has alot of overlap. Even if we end up with multiple ways to export the accounting data the spots you have to hook into to actually do the accounting should be largely identical. take care, Gerd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3] drm/virtio: trace total gem bo for virtio 2021-01-21 9:10 ` Gerd Hoffmann @ 2021-01-22 5:31 ` Yiwei Zhang 2021-01-22 5:40 ` [PATCH v4] drm/virtio: Track total GPU memory for virtio driver Yiwei Zhang 0 siblings, 1 reply; 15+ messages in thread From: Yiwei Zhang @ 2021-01-22 5:31 UTC (permalink / raw) To: David Airlie, Gerd Hoffmann, Daniel Vetter Cc: dri-devel, virtualization, linux-kernel, kernel-team, Yiwei Zhang, Yiwei Zhang From: Yiwei Zhang <zzyiwei@google.com> On the success of virtio_gpu_object_create, add size of newly allocated bo to the tracked total_mem. In drm_gem_object_funcs.free, after the gem bo lost its last refcount, subtract the bo size from the tracked total_mem if the original underlying memory allocation is successful. It's more accurate to do this in device driver layer to best match when the underlying resource gets allocated and destroyed during tracing. Signed-off-by: Yiwei Zhang <zzyiwei@android.com> --- drivers/gpu/drm/virtio/Kconfig | 1 + drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++ drivers/gpu/drm/virtio/virtgpu_object.c | 11 +++++++++++ 3 files changed, 14 insertions(+) diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig index b925b8b1da16..e103b7e883b1 100644 --- a/drivers/gpu/drm/virtio/Kconfig +++ b/drivers/gpu/drm/virtio/Kconfig @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU select DRM_KMS_HELPER select DRM_GEM_SHMEM_HELPER select VIRTIO_DMA_SHARED_BUFFER + select TRACE_GPU_MEM help This is the virtual GPU driver for virtio. It can be used with QEMU based VMMs (like KVM or Xen). diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 6a232553c99b..7ab63ce9c6a9 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -249,6 +249,8 @@ struct virtio_gpu_device { spinlock_t resource_export_lock; /* protects map state and host_visible_mm */ spinlock_t host_visible_lock; + /* total memory backing gem bos */ + atomic64_t total_mem; }; struct virtio_gpu_fpriv { diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index d69a5b6da553..e2251fc41509 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -25,12 +25,21 @@ #include <linux/dma-mapping.h> #include <linux/moduleparam.h> +#include <trace/events/gpu_mem.h> #include "virtgpu_drv.h" static int virtio_gpu_virglrenderer_workaround = 1; module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, + s64 delta) +{ + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); + + trace_gpu_mem_total(vgdev->ddev->primary->index, 0, total_mem); +} + int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) { if (virtio_gpu_virglrenderer_workaround) { @@ -104,6 +113,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; if (bo->created) { + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); virtio_gpu_cmd_unref_resource(vgdev, bo); virtio_gpu_notify(vgdev); /* completion handler calls virtio_gpu_cleanup_object() */ @@ -265,6 +275,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, virtio_gpu_object_attach(vgdev, bo, ents, nents); } + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); *bo_ptr = bo; return 0; -- 2.30.0.280.ga3ce27912f-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4] drm/virtio: Track total GPU memory for virtio driver 2021-01-22 5:31 ` [PATCH v3] drm/virtio: trace total gem bo for virtio Yiwei Zhang @ 2021-01-22 5:40 ` Yiwei Zhang 2021-01-22 7:58 ` Yiwei Zhang 0 siblings, 1 reply; 15+ messages in thread From: Yiwei Zhang @ 2021-01-22 5:40 UTC (permalink / raw) To: David Airlie, Gerd Hoffmann, Daniel Vetter Cc: dri-devel, virtualization, linux-kernel, kernel-team, Yiwei Zhang On the success of virtio_gpu_object_create, add size of newly allocated bo to the tracked total_mem. In drm_gem_object_funcs.free, after the gem bo loses its last refcount, subtract the bo size from the tracked total_mem if the original underlying memory allocation is successful. It's more accurate to do this in device driver layer to best match when the underlying resource gets allocated and destroyed during tracing. Signed-off-by: Yiwei Zhang <zzyiwei@android.com> --- drivers/gpu/drm/virtio/Kconfig | 1 + drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++ drivers/gpu/drm/virtio/virtgpu_object.c | 11 +++++++++++ 3 files changed, 14 insertions(+) diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig index b925b8b1da16..e103b7e883b1 100644 --- a/drivers/gpu/drm/virtio/Kconfig +++ b/drivers/gpu/drm/virtio/Kconfig @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU select DRM_KMS_HELPER select DRM_GEM_SHMEM_HELPER select VIRTIO_DMA_SHARED_BUFFER + select TRACE_GPU_MEM help This is the virtual GPU driver for virtio. It can be used with QEMU based VMMs (like KVM or Xen). diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 6a232553c99b..c5622f9b591f 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -249,6 +249,8 @@ struct virtio_gpu_device { spinlock_t resource_export_lock; /* protects map state and host_visible_mm */ spinlock_t host_visible_lock; + + atomic64_t total_mem; }; struct virtio_gpu_fpriv { diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index d69a5b6da553..e2251fc41509 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -25,12 +25,21 @@ #include <linux/dma-mapping.h> #include <linux/moduleparam.h> +#include <trace/events/gpu_mem.h> #include "virtgpu_drv.h" static int virtio_gpu_virglrenderer_workaround = 1; module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, + s64 delta) +{ + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); + + trace_gpu_mem_total(vgdev->ddev->primary->index, 0, total_mem); +} + int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) { if (virtio_gpu_virglrenderer_workaround) { @@ -104,6 +113,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; if (bo->created) { + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); virtio_gpu_cmd_unref_resource(vgdev, bo); virtio_gpu_notify(vgdev); /* completion handler calls virtio_gpu_cleanup_object() */ @@ -265,6 +275,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, virtio_gpu_object_attach(vgdev, bo, ents, nents); } + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); *bo_ptr = bo; return 0; -- 2.30.0.280.ga3ce27912f-goog ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4] drm/virtio: Track total GPU memory for virtio driver 2021-01-22 5:40 ` [PATCH v4] drm/virtio: Track total GPU memory for virtio driver Yiwei Zhang @ 2021-01-22 7:58 ` Yiwei Zhang 2021-01-22 10:03 ` Daniel Vetter 0 siblings, 1 reply; 15+ messages in thread From: Yiwei Zhang @ 2021-01-22 7:58 UTC (permalink / raw) To: David Airlie, Gerd Hoffmann, Daniel Vetter Cc: dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, kernel-team On Thu, Jan 21, 2021 at 9:40 PM Yiwei Zhang <zzyiwei@android.com> wrote: > > On the success of virtio_gpu_object_create, add size of newly allocated > bo to the tracked total_mem. In drm_gem_object_funcs.free, after the gem > bo loses its last refcount, subtract the bo size from the tracked > total_mem if the original underlying memory allocation is successful. > > It's more accurate to do this in device driver layer to best match when > the underlying resource gets allocated and destroyed during tracing. > > Signed-off-by: Yiwei Zhang <zzyiwei@android.com> > --- > drivers/gpu/drm/virtio/Kconfig | 1 + > drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++ > drivers/gpu/drm/virtio/virtgpu_object.c | 11 +++++++++++ > 3 files changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig > index b925b8b1da16..e103b7e883b1 100644 > --- a/drivers/gpu/drm/virtio/Kconfig > +++ b/drivers/gpu/drm/virtio/Kconfig > @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU > select DRM_KMS_HELPER > select DRM_GEM_SHMEM_HELPER > select VIRTIO_DMA_SHARED_BUFFER > + select TRACE_GPU_MEM > help > This is the virtual GPU driver for virtio. It can be used with > QEMU based VMMs (like KVM or Xen). > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > index 6a232553c99b..c5622f9b591f 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -249,6 +249,8 @@ struct virtio_gpu_device { > spinlock_t resource_export_lock; > /* protects map state and host_visible_mm */ > spinlock_t host_visible_lock; > + > + atomic64_t total_mem; > }; > > struct virtio_gpu_fpriv { > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > index d69a5b6da553..e2251fc41509 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > @@ -25,12 +25,21 @@ > > #include <linux/dma-mapping.h> > #include <linux/moduleparam.h> > +#include <trace/events/gpu_mem.h> > > #include "virtgpu_drv.h" > > static int virtio_gpu_virglrenderer_workaround = 1; > module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > + s64 delta) > +{ > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > + > + trace_gpu_mem_total(vgdev->ddev->primary->index, 0, total_mem); > +} > + > int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) > { > if (virtio_gpu_virglrenderer_workaround) { > @@ -104,6 +113,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) > struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; > > if (bo->created) { > + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); > virtio_gpu_cmd_unref_resource(vgdev, bo); > virtio_gpu_notify(vgdev); > /* completion handler calls virtio_gpu_cleanup_object() */ > @@ -265,6 +275,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > virtio_gpu_object_attach(vgdev, bo, ents, nents); > } > > + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); > *bo_ptr = bo; > return 0; > > -- > 2.30.0.280.ga3ce27912f-goog > Re Gerd and Daniel: I'm not sure why we want to couple this patch too much with the dma-bufs tracking. The tracepoint added here itself is pretty useful for tracking gem bo total usage in virtio gpu upon tracing. The original purpose for integrating this tracepoint in all Android gpu kernel drivers is to just track total gpu memory usage and serve the accurate data to game developers in a much easier way. It's something they can rely on for robust testing and regression monitoring. The only overlap with the dma-buf side is when we export a bo via prime to a dma-buf. But still, the total here is already useful for this particular device. Using which approach to account for the overlap wouldn't block this small integration from my understanding. Besides, there's no plan for adding per-process gem total tracking in virtio-gpu at this moment. This patch should be light enough to carry without worrying about tech debt I believe. Many thanks! Yiwei ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] drm/virtio: Track total GPU memory for virtio driver 2021-01-22 7:58 ` Yiwei Zhang @ 2021-01-22 10:03 ` Daniel Vetter 0 siblings, 0 replies; 15+ messages in thread From: Daniel Vetter @ 2021-01-22 10:03 UTC (permalink / raw) To: Yiwei Zhang Cc: David Airlie, Gerd Hoffmann, Daniel Vetter, dri-devel, open list:VIRTIO CORE, NET..., Linux Kernel Mailing List, kernel-team On Thu, Jan 21, 2021 at 11:58:22PM -0800, Yiwei Zhang wrote: > On Thu, Jan 21, 2021 at 9:40 PM Yiwei Zhang <zzyiwei@android.com> wrote: > > > > On the success of virtio_gpu_object_create, add size of newly allocated > > bo to the tracked total_mem. In drm_gem_object_funcs.free, after the gem > > bo loses its last refcount, subtract the bo size from the tracked > > total_mem if the original underlying memory allocation is successful. > > > > It's more accurate to do this in device driver layer to best match when > > the underlying resource gets allocated and destroyed during tracing. > > > > Signed-off-by: Yiwei Zhang <zzyiwei@android.com> > > --- > > drivers/gpu/drm/virtio/Kconfig | 1 + > > drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++ > > drivers/gpu/drm/virtio/virtgpu_object.c | 11 +++++++++++ > > 3 files changed, 14 insertions(+) > > > > diff --git a/drivers/gpu/drm/virtio/Kconfig b/drivers/gpu/drm/virtio/Kconfig > > index b925b8b1da16..e103b7e883b1 100644 > > --- a/drivers/gpu/drm/virtio/Kconfig > > +++ b/drivers/gpu/drm/virtio/Kconfig > > @@ -5,6 +5,7 @@ config DRM_VIRTIO_GPU > > select DRM_KMS_HELPER > > select DRM_GEM_SHMEM_HELPER > > select VIRTIO_DMA_SHARED_BUFFER > > + select TRACE_GPU_MEM > > help > > This is the virtual GPU driver for virtio. It can be used with > > QEMU based VMMs (like KVM or Xen). > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > > index 6a232553c99b..c5622f9b591f 100644 > > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > > @@ -249,6 +249,8 @@ struct virtio_gpu_device { > > spinlock_t resource_export_lock; > > /* protects map state and host_visible_mm */ > > spinlock_t host_visible_lock; > > + > > + atomic64_t total_mem; > > }; > > > > struct virtio_gpu_fpriv { > > diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c > > index d69a5b6da553..e2251fc41509 100644 > > --- a/drivers/gpu/drm/virtio/virtgpu_object.c > > +++ b/drivers/gpu/drm/virtio/virtgpu_object.c > > @@ -25,12 +25,21 @@ > > > > #include <linux/dma-mapping.h> > > #include <linux/moduleparam.h> > > +#include <trace/events/gpu_mem.h> > > > > #include "virtgpu_drv.h" > > > > static int virtio_gpu_virglrenderer_workaround = 1; > > module_param_named(virglhack, virtio_gpu_virglrenderer_workaround, int, 0400); > > > > +static inline void virtio_gpu_trace_total_mem(struct virtio_gpu_device *vgdev, > > + s64 delta) > > +{ > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem); > > + > > + trace_gpu_mem_total(vgdev->ddev->primary->index, 0, total_mem); > > +} > > + > > int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev, uint32_t *resid) > > { > > if (virtio_gpu_virglrenderer_workaround) { > > @@ -104,6 +113,7 @@ static void virtio_gpu_free_object(struct drm_gem_object *obj) > > struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private; > > > > if (bo->created) { > > + virtio_gpu_trace_total_mem(vgdev, -(obj->size)); > > virtio_gpu_cmd_unref_resource(vgdev, bo); > > virtio_gpu_notify(vgdev); > > /* completion handler calls virtio_gpu_cleanup_object() */ > > @@ -265,6 +275,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev, > > virtio_gpu_object_attach(vgdev, bo, ents, nents); > > } > > > > + virtio_gpu_trace_total_mem(vgdev, shmem_obj->base.size); > > *bo_ptr = bo; > > return 0; > > > > -- > > 2.30.0.280.ga3ce27912f-goog > > > > Re Gerd and Daniel: > > I'm not sure why we want to couple this patch too much with the > dma-bufs tracking. The tracepoint added here itself is pretty useful > for tracking gem bo total usage in virtio gpu upon tracing. The > original purpose for integrating this tracepoint in all Android gpu > kernel drivers is to just track total gpu memory usage and serve the > accurate data to game developers in a much easier way. It's something > they can rely on for robust testing and regression monitoring. > > The only overlap with the dma-buf side is when we export a bo via > prime to a dma-buf. But still, the total here is already useful for > this particular device. Using which approach to account for the > overlap wouldn't block this small integration from my understanding. > > Besides, there's no plan for adding per-process gem total tracking in > virtio-gpu at this moment. This patch should be light enough to carry > without worrying about tech debt I believe. The tracepoint is clearly more generic than just what you implement here, to support the full use cases on Android's closed stacks. And it is uapi. Tech debt isn't measured in lines of code, but in how expensive it's going to be to fix up the mess in the future. uapi is expensive no matter how few lines are used to implement it. So yeah this needs to be properly thought out, properly implemented (not just on the virtual demo stack but something that looks like actual production stack), with open drivers, proper alignment with other efforts like tracking memory with cgroups, and the interactions with dma-buf tracking resolved, and igt testcases (this is meant to be generic after all), and at least solid proposals for rolling this out across the drm drivers, and ... In other words, new uapi needs to be done right. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-01-22 10:25 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-18 22:46 [PATCH] Track total GPU memory for virtio driver Yiwei Zhang 2021-01-18 23:23 ` Yiwei Zhang 2021-01-18 23:40 ` [PATCH v2] drm/virtio: " Yiwei Zhang 2021-01-19 7:03 ` Daniel Vetter 2021-01-19 19:08 ` Yiwei Zhang 2021-01-20 9:11 ` Daniel Vetter 2021-01-20 9:51 ` Yiwei Zhang 2021-01-20 12:37 ` Daniel Vetter 2021-01-20 13:33 ` Gerd Hoffmann 2021-01-20 18:52 ` Yiwei Zhang 2021-01-21 9:10 ` Gerd Hoffmann 2021-01-22 5:31 ` [PATCH v3] drm/virtio: trace total gem bo for virtio Yiwei Zhang 2021-01-22 5:40 ` [PATCH v4] drm/virtio: Track total GPU memory for virtio driver Yiwei Zhang 2021-01-22 7:58 ` Yiwei Zhang 2021-01-22 10:03 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).