[Intel-gfx] [RFC 3/6] drm: Add fdinfo memory stats

[Tvrtko Ursulin]

On 18/04/2023 17:13, Rob Clark wrote:
> On Tue, Apr 18, 2023 at 7:46 AM Tvrtko Ursulin
> <tvrtko.ursulin at linux.intel.com> wrote:
>> On 18/04/2023 15:36, Rob Clark wrote:
>>> On Tue, Apr 18, 2023 at 7:19 AM Tvrtko Ursulin
>>> <tvrtko.ursulin at linux.intel.com> wrote:
>>>>
>>>>
>>>> On 18/04/2023 14:49, Rob Clark wrote:
>>>>> On Tue, Apr 18, 2023 at 2:00 AM Tvrtko Ursulin
>>>>> <tvrtko.ursulin at linux.intel.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 17/04/2023 20:39, Rob Clark wrote:
>>>>>>> On Mon, Apr 17, 2023 at 8:56 AM Tvrtko Ursulin
>>>>>>> <tvrtko.ursulin at linux.intel.com> wrote:
>>>>>>>>
>>>>>>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>>>>>
>>>>>>>> Add support to dump GEM stats to fdinfo.
>>>>>>>>
>>>>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>>>>>> ---
>>>>>>>>      Documentation/gpu/drm-usage-stats.rst | 12 +++++++
>>>>>>>>      drivers/gpu/drm/drm_file.c            | 52 +++++++++++++++++++++++++++
>>>>>>>>      include/drm/drm_drv.h                 |  7 ++++
>>>>>>>>      include/drm/drm_file.h                |  8 +++++
>>>>>>>>      4 files changed, 79 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst
>>>>>>>> index 2ab32c40e93c..8273a41b2fb0 100644
>>>>>>>> --- a/Documentation/gpu/drm-usage-stats.rst
>>>>>>>> +++ b/Documentation/gpu/drm-usage-stats.rst
>>>>>>>> @@ -21,6 +21,7 @@ File format specification
>>>>>>>>
>>>>>>>>      - File shall contain one key value pair per one line of text.
>>>>>>>>      - Colon character (`:`) must be used to delimit keys and values.
>>>>>>>> +- Caret (`^`) is also a reserved character.
>>>>>>>
>>>>>>> this doesn't solve the problem that led me to drm-$CATEGORY-memory... ;-)
>>>>>>
>>>>>> Could you explain or remind me with a link to a previous explanation?
>>>>>
>>>>> How is userspace supposed to know that "drm-memory-foo" is a memory
>>>>> type "foo" but drm-memory-foo^size is not memory type "foo^size"?
>>>>
>>>> Are you referring to nvtop?
>>>
>>> I'm not referring to any particular app.  It could even be some app
>>> that isn't even written yet but started with an already existing
>>> kernel without this change.  It is just a general point about forwards
>>> compatibility of old userspace with new kernel.  And it doesn't really
>>> matter what special character you use.  You can't retroactively define
>>> some newly special characters.
>>
>> What you see does not work if we output both legacy and new key with
>> extra category? Userspace which hardcode the name keep working, and
>> userspace which parses region names as opaque strings also keeps working
>> just shows more entries.
> 
> well, it shows nonsense entries.. I'd not call that "working"
> 
> But honestly we are wasting too many words on this.. we just can't
> re-use the "drm-memory-<anything>" namespace, it is already burnt.
> Full stop.
> 
> If you don't like the "drm-$CATEGORY-$REGION" workaround then we can
> shorten to "drm-mem-$REGION-$CATEGORY" since that won't accidentally
> match the existing "drm-memory-" pattern.

I can live with that token reversal, it was not the primary motivation 
for my RFC as we have discussed that side of things already before I 
sketched my version up.

But I also still don't get what doesn't work with what I described and 
you did not really address my specific questions with more than a 
"doesn't work" with not much details.

Unless for you it starts and ends with "nonsense entries". If so then it 
seems there is no option than to disagree and move on. Again, I can 
accept the drm-$category-memory-$region.

Regards,

Tvrtko