2021-08-31 18:09:22

by Kalesh Singh

[permalink] [raw]
Subject: [RESEND v2] tracing/gpu: Add imported size to gpu_mem_imported tracepoint

The existing gpu_mem_total tracepoint provides GPU drivers a uniform way
to report the per-process and system-wide GPU memory usage. This
tracepoint reports a single total of the GPU private allocations and the
imported memory. [1]

To allow distinguishing GPU private vs imported memory, add an
imported_size field to the gpu_mem_total tracepoint. GPU drivers can use
this new field to report the per-process and global GPU-imported memory
in a uniform way.

User space tools can detect and handle the old vs new gpu_mem_total
format via the gpu_mem/gpu_mem_total/format file.

[1] https://lore.kernel.org/r/[email protected]/

Signed-off-by: Kalesh Singh <[email protected]>
---
include/trace/events/gpu_mem.h | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/trace/events/gpu_mem.h b/include/trace/events/gpu_mem.h
index 26d871f96e94..ae6fab6bab7b 100644
--- a/include/trace/events/gpu_mem.h
+++ b/include/trace/events/gpu_mem.h
@@ -15,7 +15,7 @@

/*
* The gpu_memory_total event indicates that there's an update to either the
- * global or process total gpu memory counters.
+ * global or process total and imported gpu memory counters.
*
* This event should be emitted whenever the kernel device driver allocates,
* frees, imports, unimports memory in the GPU addressable space.
@@ -24,31 +24,36 @@
*
* @pid: Put 0 for global total, while positive pid for process total.
*
- * @size: Size of the allocation in bytes.
+ * @size: Total size of allocated and imported memory in bytes.
+ *
+ * @imported_size: Total size of imported memory in bytes.
*
*/
TRACE_EVENT(gpu_mem_total,

- TP_PROTO(uint32_t gpu_id, uint32_t pid, uint64_t size),
+ TP_PROTO(uint32_t gpu_id, uint32_t pid, uint64_t size, uint64_t imported_size),

- TP_ARGS(gpu_id, pid, size),
+ TP_ARGS(gpu_id, pid, size, imported_size),

TP_STRUCT__entry(
__field(uint32_t, gpu_id)
__field(uint32_t, pid)
__field(uint64_t, size)
+ __field(uint64_t, imported_size)
),

TP_fast_assign(
__entry->gpu_id = gpu_id;
__entry->pid = pid;
__entry->size = size;
+ __entry->imported_size = imported_size;
),

- TP_printk("gpu_id=%u pid=%u size=%llu",
+ TP_printk("gpu_id=%u pid=%u size=%llu imported_size=%llu",
__entry->gpu_id,
__entry->pid,
- __entry->size)
+ __entry->size,
+ __entry->imported_size)
);

#endif /* _TRACE_GPU_MEM_H */

base-commit: 9c849ce86e0fa93a218614eac562ace44053d7ce
--
2.33.0.259.gc128427fd7-goog


2021-09-02 16:34:28

by Kalesh Singh

[permalink] [raw]
Subject: Re: [RESEND v2] tracing/gpu: Add imported size to gpu_mem_imported tracepoint

On Tue, Aug 31, 2021 at 10:02 AM Kalesh Singh <[email protected]> wrote:
>
> The existing gpu_mem_total tracepoint provides GPU drivers a uniform way
> to report the per-process and system-wide GPU memory usage. This
> tracepoint reports a single total of the GPU private allocations and the
> imported memory. [1]
>
> To allow distinguishing GPU private vs imported memory, add an
> imported_size field to the gpu_mem_total tracepoint. GPU drivers can use
> this new field to report the per-process and global GPU-imported memory
> in a uniform way.
>
> User space tools can detect and handle the old vs new gpu_mem_total
> format via the gpu_mem/gpu_mem_total/format file.
>
> [1] https://lore.kernel.org/r/[email protected]/
>
> Signed-off-by: Kalesh Singh <[email protected]>

Friendly ping on this one, since Steve was out of office :)

If there are no other concerns, I would like to have it considered for merge.

Thanks,
Kalesh

> ---
> include/trace/events/gpu_mem.h | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/trace/events/gpu_mem.h b/include/trace/events/gpu_mem.h
> index 26d871f96e94..ae6fab6bab7b 100644
> --- a/include/trace/events/gpu_mem.h
> +++ b/include/trace/events/gpu_mem.h
> @@ -15,7 +15,7 @@
>
> /*
> * The gpu_memory_total event indicates that there's an update to either the
> - * global or process total gpu memory counters.
> + * global or process total and imported gpu memory counters.
> *
> * This event should be emitted whenever the kernel device driver allocates,
> * frees, imports, unimports memory in the GPU addressable space.
> @@ -24,31 +24,36 @@
> *
> * @pid: Put 0 for global total, while positive pid for process total.
> *
> - * @size: Size of the allocation in bytes.
> + * @size: Total size of allocated and imported memory in bytes.
> + *
> + * @imported_size: Total size of imported memory in bytes.
> *
> */
> TRACE_EVENT(gpu_mem_total,
>
> - TP_PROTO(uint32_t gpu_id, uint32_t pid, uint64_t size),
> + TP_PROTO(uint32_t gpu_id, uint32_t pid, uint64_t size, uint64_t imported_size),
>
> - TP_ARGS(gpu_id, pid, size),
> + TP_ARGS(gpu_id, pid, size, imported_size),
>
> TP_STRUCT__entry(
> __field(uint32_t, gpu_id)
> __field(uint32_t, pid)
> __field(uint64_t, size)
> + __field(uint64_t, imported_size)
> ),
>
> TP_fast_assign(
> __entry->gpu_id = gpu_id;
> __entry->pid = pid;
> __entry->size = size;
> + __entry->imported_size = imported_size;
> ),
>
> - TP_printk("gpu_id=%u pid=%u size=%llu",
> + TP_printk("gpu_id=%u pid=%u size=%llu imported_size=%llu",
> __entry->gpu_id,
> __entry->pid,
> - __entry->size)
> + __entry->size,
> + __entry->imported_size)
> );
>
> #endif /* _TRACE_GPU_MEM_H */
>
> base-commit: 9c849ce86e0fa93a218614eac562ace44053d7ce
> --
> 2.33.0.259.gc128427fd7-goog
>

2021-09-03 20:32:33

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RESEND v2] tracing/gpu: Add imported size to gpu_mem_imported tracepoint

On Tue, 31 Aug 2021 17:02:29 +0000
Kalesh Singh <[email protected]> wrote:

> The existing gpu_mem_total tracepoint provides GPU drivers a uniform way
> to report the per-process and system-wide GPU memory usage. This
> tracepoint reports a single total of the GPU private allocations and the
> imported memory. [1]
>
> To allow distinguishing GPU private vs imported memory, add an
> imported_size field to the gpu_mem_total tracepoint. GPU drivers can use
> this new field to report the per-process and global GPU-imported memory
> in a uniform way.
>
> User space tools can detect and handle the old vs new gpu_mem_total
> format via the gpu_mem/gpu_mem_total/format file.
>
> [1] https://lore.kernel.org/r/[email protected]/
>
> Signed-off-by: Kalesh Singh <[email protected]>
> ---
> include/trace/events/gpu_mem.h | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>

This is that trace event that doesn't have any in tree callers, right? I
thought there was going to be some soon.

For the updates to the tracing side (besides not having any users), it
looks trivial to me.

Acked-by: Steven Rostedt (VMware) <[email protected]>

But this needs to be pulled in by one of the GPU maintainers.

-- Steve

2021-09-03 22:37:53

by Kalesh Singh

[permalink] [raw]
Subject: Re: [RESEND v2] tracing/gpu: Add imported size to gpu_mem_imported tracepoint

On Fri, Sep 3, 2021 at 1:30 PM Steven Rostedt <[email protected]> wrote:
>
> On Tue, 31 Aug 2021 17:02:29 +0000
> Kalesh Singh <[email protected]> wrote:
>
> > The existing gpu_mem_total tracepoint provides GPU drivers a uniform way
> > to report the per-process and system-wide GPU memory usage. This
> > tracepoint reports a single total of the GPU private allocations and the
> > imported memory. [1]
> >
> > To allow distinguishing GPU private vs imported memory, add an
> > imported_size field to the gpu_mem_total tracepoint. GPU drivers can use
> > this new field to report the per-process and global GPU-imported memory
> > in a uniform way.
> >
> > User space tools can detect and handle the old vs new gpu_mem_total
> > format via the gpu_mem/gpu_mem_total/format file.
> >
> > [1] https://lore.kernel.org/r/[email protected]/
> >
> > Signed-off-by: Kalesh Singh <[email protected]>
> > ---
> > include/trace/events/gpu_mem.h | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
>
> This is that trace event that doesn't have any in tree callers, right? I
> thought there was going to be some soon.

The trace event is currently used by the Android GPU drivers, and
there is some work ongoing to add this in drm core upstream but it's
not yet ready.

>
> For the updates to the tracing side (besides not having any users), it
> looks trivial to me.
>
> Acked-by: Steven Rostedt (VMware) <[email protected]>
>
> But this needs to be pulled in by one of the GPU maintainers.

Thanks for the review Steve. I'll resend adding the GPU maintainers.

>
> -- Steve

2021-09-07 17:54:41

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [RESEND v2] tracing/gpu: Add imported size to gpu_mem_imported tracepoint

On Tue, Sep 07, 2021 at 01:49:23PM -0400, Steven Rostedt wrote:
> On Fri, 3 Sep 2021 15:36:03 -0700
> Kalesh Singh <[email protected]> wrote:
>
> > On Fri, Sep 3, 2021 at 1:30 PM Steven Rostedt <[email protected]> wrote:
> > >
> > > On Tue, 31 Aug 2021 17:02:29 +0000
> > > Kalesh Singh <[email protected]> wrote:
> > >
> > > > The existing gpu_mem_total tracepoint provides GPU drivers a uniform way
> > > > to report the per-process and system-wide GPU memory usage. This
> > > > tracepoint reports a single total of the GPU private allocations and the
> > > > imported memory. [1]
> > > >
> > > > To allow distinguishing GPU private vs imported memory, add an
> > > > imported_size field to the gpu_mem_total tracepoint. GPU drivers can use
> > > > this new field to report the per-process and global GPU-imported memory
> > > > in a uniform way.
> > > >
> > > > User space tools can detect and handle the old vs new gpu_mem_total
> > > > format via the gpu_mem/gpu_mem_total/format file.
> > > >
> > > > [1] https://lore.kernel.org/r/[email protected]/
> > > >
> > > > Signed-off-by: Kalesh Singh <[email protected]>
> > > > ---
> > > > include/trace/events/gpu_mem.h | 17 +++++++++++------
> > > > 1 file changed, 11 insertions(+), 6 deletions(-)
> > > >
> > >
> > > This is that trace event that doesn't have any in tree callers, right? I
> > > thought there was going to be some soon.
> >
> > The trace event is currently used by the Android GPU drivers, and
> > there is some work ongoing to add this in drm core upstream but it's
> > not yet ready.
> >
> > >
> > > For the updates to the tracing side (besides not having any users), it
> > > looks trivial to me.
> > >
> > > Acked-by: Steven Rostedt (VMware) <[email protected]>
> > >
> > > But this needs to be pulled in by one of the GPU maintainers.
> >
> > Thanks for the review Steve. I'll resend adding the GPU maintainers.
>
> OK, so it was Greg that gave me the Ack to accept it.

Me? Hah, I'm not a gpu developer, they are the ones that need to ack
this thing...

greg k-h

2021-09-07 18:52:40

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RESEND v2] tracing/gpu: Add imported size to gpu_mem_imported tracepoint

On Fri, 3 Sep 2021 15:36:03 -0700
Kalesh Singh <[email protected]> wrote:

> On Fri, Sep 3, 2021 at 1:30 PM Steven Rostedt <[email protected]> wrote:
> >
> > On Tue, 31 Aug 2021 17:02:29 +0000
> > Kalesh Singh <[email protected]> wrote:
> >
> > > The existing gpu_mem_total tracepoint provides GPU drivers a uniform way
> > > to report the per-process and system-wide GPU memory usage. This
> > > tracepoint reports a single total of the GPU private allocations and the
> > > imported memory. [1]
> > >
> > > To allow distinguishing GPU private vs imported memory, add an
> > > imported_size field to the gpu_mem_total tracepoint. GPU drivers can use
> > > this new field to report the per-process and global GPU-imported memory
> > > in a uniform way.
> > >
> > > User space tools can detect and handle the old vs new gpu_mem_total
> > > format via the gpu_mem/gpu_mem_total/format file.
> > >
> > > [1] https://lore.kernel.org/r/[email protected]/
> > >
> > > Signed-off-by: Kalesh Singh <[email protected]>
> > > ---
> > > include/trace/events/gpu_mem.h | 17 +++++++++++------
> > > 1 file changed, 11 insertions(+), 6 deletions(-)
> > >
> >
> > This is that trace event that doesn't have any in tree callers, right? I
> > thought there was going to be some soon.
>
> The trace event is currently used by the Android GPU drivers, and
> there is some work ongoing to add this in drm core upstream but it's
> not yet ready.
>
> >
> > For the updates to the tracing side (besides not having any users), it
> > looks trivial to me.
> >
> > Acked-by: Steven Rostedt (VMware) <[email protected]>
> >
> > But this needs to be pulled in by one of the GPU maintainers.
>
> Thanks for the review Steve. I'll resend adding the GPU maintainers.

OK, so it was Greg that gave me the Ack to accept it.

I'll need his Ack again for the update.

-- Steve