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

[Rob Clark]

On Tue, Apr 18, 2023 at 9:44 AM Tvrtko Ursulin
<tvrtko.ursulin at linux.intel.com> wrote:
>
>
> 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.

Yeah, it's about "nonsense entries".. and I am perhaps being a bit
pedantic about compatibility with old userspace (not like there are
100's of apps parsing drm fdinfo), but it is the principle of the
matter ;-)

BR,
-R