[PATCH RFC 4/4] drm/panfrost: Expose some memory related stats through fdinfo

[Tvrtko Ursulin]

Hi,

Chris was kind enough to bring my attention to this thread. Indeed this 
information was asked for by various people for many years so it sounds 
very useful to actually do attempt it.

On 04/01/2023 13:03, Boris Brezillon wrote:
> drm-memory-all: memory hold by this context. Not that all the memory is
> not necessarily resident: heap BO size is counted even though only part
> of the memory reserved for those BOs might be allocated.
> 
> drm-memory-resident: resident memory size. For normal BOs it's the same
> as drm-memory-all, but for heap BOs, only the memory actually allocated
> is counted.
> 
> drm-memory-purgeable: amount of memory that can be reclaimed by the
> system (madvise(DONT_NEED)).
> 
> drm-memory-shared: amount of memory shared through dma-buf.

A bunch of comments/questions..

First of all, lets please continue documenting the fdinfo content in 
Documentation/gpu/drm-usage-stats.rst as stuff is proposed to be added. 
Idea was to have as much commonality as reasonably possible, and so to 
have more usable user facing tools etc. And also please copy people 
involved with adding new stuff to that file.

(Half-digression - for some reason get_maintainers.pl does not play nice 
with this file, I was expecting it to count and sort contributors after 
maintainers but it doesn't for some reason. Perhaps it only does that 
for source code.)

For the actual key/fields name.. I suspect apart from category we will 
need a memory type part, at least with discrete GPUs (non unified/shared 
memory designs), that further breakdown will be required. We therefore 
need to discuss if we do that from the start, or start with your 
proposal and extend later.

In more practical terms I am talking about something this:

drm-memory-local-all: <ulong> <unit>
drm-memory-local-resident: <ulong> <unit>
drm-memory-system-all: <ulong> <unit>
drm-memory-system-resident: <ulong> <unit>

"All/resident/..." could then probably be standardized and the 
"local/system" could be partially standardized or left for drivers to 
use names they see fit (same as with engine names).

Also, I an not quite liking "all". Analogy from the CPU land for it 
would be "virtual", but does translate into the GPU world?

Finally, we also need to define the semantics of resident when we deal 
with shared objects. For instance process P / fd Fp creates a buffer and 
passes the handle to process Q / fd Fq. Whose fdinfo sees what, in what 
fields and when?

I suspect if Q is the first to start using the shared object it should 
show it under "resident". P should not, until it starts using it itself. 
For the "all" category both should see it.

For "shared" are you proposing to count imported or exported counts as 
well? I think it needs to be clearly documented. Is it "this fd is using 
this much shared buffers", or "this fd has exported this much shared 
buffers", or both?

I don't know your driver to quickly figure out what semantics you proposed?

Regards,

Tvrtko

> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>   drivers/gpu/drm/panfrost/panfrost_drv.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 6ee43559fc14..05d5d480df2a 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -519,9 +519,16 @@ static void panfrost_show_fdinfo(struct seq_file *m, struct file *f)
>   {
>   	struct drm_file *file = f->private_data;
>   	struct panfrost_file_priv *panfrost_priv = file->driver_priv;
> +	struct panfrost_mmu_stats mmu_stats; > +
> +	panfrost_mmu_get_stats(panfrost_priv->mmu, &mmu_stats);
>   
>   	seq_printf(m, "drm-driver:\t%s\n", file->minor->dev->driver->name);
>   	seq_printf(m, "drm-client-id:\t%llu\n", panfrost_priv->sched_entity[0].fence_context);
> +	seq_printf(m, "drm-memory-all:\t%llu KiB\n", mmu_stats.all >> 10);
> +	seq_printf(m, "drm-memory-resident:\t%llu KiB\n", mmu_stats.resident >> 10);
> +	seq_printf(m, "drm-memory-purgeable:\t%llu KiB\n", mmu_stats.purgeable >> 10);
> +	seq_printf(m, "drm-memory-shared:\t%llu KiB\n", mmu_stats.shared >> 10);
>   }
>   
>   static const struct file_operations panfrost_drm_driver_fops = {