linux-kernel - Re: [PATCH v5 2/2] Documentation/gpu: Add fdinfo meanings of drm-*-internal memory tags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2sb72aco2lc5hlvwn7hpc5k27naep7u2s64lc6qzk4ruy6jkhd@c2dfhvhe76yt>
Date: Thu, 2 Jan 2025 22:18:35 +0000
From: Adrián Martínez Larumbe <adrian.larumbe@...labora.com>
To: Tvrtko Ursulin <tursulin@...ulin.net>
Cc: Boris Brezillon <boris.brezillon@...labora.com>, 
	Steven Price <steven.price@....com>, Liviu Dudau <liviu.dudau@....com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, kernel@...labora.com, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, Mihail Atanassov <mihail.atanassov@....com>
Subject: Re: [PATCH v5 2/2] Documentation/gpu: Add fdinfo meanings of
 drm-*-internal memory tags

On 02.01.2025 21:59, Tvrtko Ursulin wrote:
>
>On 18/12/2024 18:18, Adrián Martínez Larumbe wrote:
>> From: Adrián Larumbe <adrian.larumbe@...labora.com>
>> 
>> A previous commit enabled display of driver-internal kernel BO sizes
>> through the device file's fdinfo interface.
>> 
>> Expand the description of the relevant driver-specific key:value pairs
>> with the definitions of the new drm-*-internal ones.
>> 
>> Signed-off-by: Adrián Larumbe <adrian.larumbe@...labora.com>
>> Reviewed-by: Mihail Atanassov <mihail.atanassov@....com>
>> ---
>>   Documentation/gpu/panthor.rst | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>> 
>> diff --git a/Documentation/gpu/panthor.rst b/Documentation/gpu/panthor.rst
>> index 3f8979fa2b86..23aa3d67c9d2 100644
>> --- a/Documentation/gpu/panthor.rst
>> +++ b/Documentation/gpu/panthor.rst
>> @@ -26,6 +26,10 @@ the currently possible format options:
>>        drm-cycles-panthor:     94439687187
>>        drm-maxfreq-panthor:    1000000000 Hz
>>        drm-curfreq-panthor:    1000000000 Hz
>> +     drm-total-internal:     10396 KiB
>> +     drm-shared-internal:    0
>> +     drm-active-internal:    10396 KiB
>> +     drm-resident-internal:  10396 KiB
>>        drm-total-memory:       16480 KiB
>>        drm-shared-memory:      0
>>        drm-active-memory:      16200 KiB
>> @@ -44,3 +48,13 @@ driver by writing into the appropriate sysfs node::
>>   Where `N` is a bit mask where cycle and timestamp sampling are respectively
>>   enabled by the first and second bits.
>> +
>> +Possible `drm-*-internal` keys are: `total`, `active`, `resident` and `shared`.
>> +These values convey the sizes of the internal driver-owned shmem BO's that
>> +aren't exposed to user-space through a DRM handle, like queue ring buffers,
>> +sync object arrays and heap chunks. Because they are all allocated and pinned
>> +at creation time, `drm-resident-internal` and `drm-total-internal` should always
>> +be equal. `drm-active-internal` shows the size of kernel BO's associated with
>> +VM's and groups currently being scheduled for execution by the GPU.
>> +`drm-shared-internal` is unused at present, but in the future it might stand for
>> +the size of executable FW regions, since they do not belong to an open file context.
>
>The description is way too specific, too tied to some of the implementations.

These are panthor-specific key:value pairs. I was in the belief that drivers
could define their own when it suits their interest beyond the DRM-wide ones
defined in the drm-fdinfo spec.

>I also don't remember that you ever explained why totting up the internal
>objects into existing regions isn't good enough. I keep asking, you keep not
>explaining. Or I missed your emails somehow.

It's not that it's not good enough, but rather that it cannot be done in the
current state of affairs. drm_show_memory_stats() defines its own
drm_memory_stats struct as an automatic variable so we don't have access to it
from anywhere else in the driver. In a previous revision of the patch series I
had come up with a workaround that would let drivers pass a function pointer to
drm_show_memory_stats() which would gather those numbers in a driver-specific
way, but it didn't seem to get any traction.

>And you keep not copying me on the thread. Copying people who expressed
>interest, gave past feedback, etc should be the norm.

I did not CC you on this series because these are all panthor-specific changes
which do not touch on any DRM fdinfo-wide code, and also because I didn't think
that driver-specific key:value pairs needed the approval of the drm-fdinfo core
maintainers.

>Until we can clarify the above points I don't think this can go in.
>
>Regards,
>
>Tvrtko

Adrian Larumbe