| Message ID | 20240813135712.82611-2-tursulin@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DRM fdinfo legacy drm-memory- clarification and amdgpu update | expand |
On Tue, Aug 13, 2024 at 6:57 AM Tvrtko Ursulin <tursulin@igalia.com> wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > > Currently it is not well defined what is drm-memory- compared to other > categories. > > In practice the only driver which emits these keys is amdgpu and in them > exposes the current resident buffer object memory (including shared). > > To prevent any confusion, document that drm-memory- is deprecated and an > alias for drm-resident-memory-. > > While at it also clarify that the reserved sub-string 'memory' refers to > the memory region component, and also clarify the intended semantics of > other memory categories. > > v2: > * Also mark drm-memory- as deprecated. > * Add some more text describing memory categories. (Alex) > > v3: > * Semantics of the amdgpu drm-memory is actually as drm-resident. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Christian König <christian.keonig@amd.com> > Cc: Rob Clark <robdclark@chromium.org> Reviewed-by: Rob Clark <robdclark@gmail.com> > --- > Documentation/gpu/drm-usage-stats.rst | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst > index a80f95ca1b2f..ff964c707754 100644 > --- a/Documentation/gpu/drm-usage-stats.rst > +++ b/Documentation/gpu/drm-usage-stats.rst > @@ -144,7 +144,9 @@ Memory > > Each possible memory type which can be used to store buffer objects by the > GPU in question shall be given a stable and unique name to be returned as the > -string here. The name "memory" is reserved to refer to normal system memory. > +string here. > + > +The region name "memory" is reserved to refer to normal system memory. > > Value shall reflect the amount of storage currently consumed by the buffer > objects belong to this client, in the respective memory region. > @@ -152,6 +154,9 @@ objects belong to this client, in the respective memory region. > Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' > indicating kibi- or mebi-bytes. > > +This key is deprecated and is an alias for drm-resident-<region>. Only one of > +the two should be present in the output. > + > - drm-shared-<region>: <uint> [KiB|MiB] > > The total size of buffers that are shared with another file (e.g., have more > @@ -159,20 +164,34 @@ than a single handle). > > - drm-total-<region>: <uint> [KiB|MiB] > > -The total size of buffers that including shared and private memory. > +The total size of all created buffers including shared and private memory. The > +backing store for the buffers does not have to be currently instantiated to be > +counted under this category. > > - drm-resident-<region>: <uint> [KiB|MiB] > > -The total size of buffers that are resident in the specified region. > +The total size of buffers that are resident (have their backing store present or > +instantiated) in the specified region. > + > +This is an alias for drm-memory-<region> and only one of the two should be > +present in the output. > > - drm-purgeable-<region>: <uint> [KiB|MiB] > > The total size of buffers that are purgeable. > > +For example drivers which implement a form of 'madvise' like functionality can > +here count buffers which have instantiated backing store, but have been marked > +with an equivalent of MADV_DONTNEED. > + > - drm-active-<region>: <uint> [KiB|MiB] > > The total size of buffers that are active on one or more engines. > > +One practical example of this can be presence of unsignaled fences in an GEM > +buffer reservation object. Therefore the active category is a subset of > +resident. > + > Implementation Details > ====================== > > -- > 2.44.0 >
On 13/08/2024 19:47, Rob Clark wrote: > On Tue, Aug 13, 2024 at 6:57 AM Tvrtko Ursulin <tursulin@igalia.com> wrote: >> >> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> >> >> Currently it is not well defined what is drm-memory- compared to other >> categories. >> >> In practice the only driver which emits these keys is amdgpu and in them >> exposes the current resident buffer object memory (including shared). >> >> To prevent any confusion, document that drm-memory- is deprecated and an >> alias for drm-resident-memory-. >> >> While at it also clarify that the reserved sub-string 'memory' refers to >> the memory region component, and also clarify the intended semantics of >> other memory categories. >> >> v2: >> * Also mark drm-memory- as deprecated. >> * Add some more text describing memory categories. (Alex) >> >> v3: >> * Semantics of the amdgpu drm-memory is actually as drm-resident. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> >> Cc: Alex Deucher <alexander.deucher@amd.com> >> Cc: Christian König <christian.keonig@amd.com> >> Cc: Rob Clark <robdclark@chromium.org> > > Reviewed-by: Rob Clark <robdclark@gmail.com> Thanks! So this one is stand alone and could be pushed to drm-misc-next. 2/2 can wait for AMD to give a verdict. Regards, Tvrtko > >> --- >> Documentation/gpu/drm-usage-stats.rst | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst >> index a80f95ca1b2f..ff964c707754 100644 >> --- a/Documentation/gpu/drm-usage-stats.rst >> +++ b/Documentation/gpu/drm-usage-stats.rst >> @@ -144,7 +144,9 @@ Memory >> >> Each possible memory type which can be used to store buffer objects by the >> GPU in question shall be given a stable and unique name to be returned as the >> -string here. The name "memory" is reserved to refer to normal system memory. >> +string here. >> + >> +The region name "memory" is reserved to refer to normal system memory. >> >> Value shall reflect the amount of storage currently consumed by the buffer >> objects belong to this client, in the respective memory region. >> @@ -152,6 +154,9 @@ objects belong to this client, in the respective memory region. >> Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' >> indicating kibi- or mebi-bytes. >> >> +This key is deprecated and is an alias for drm-resident-<region>. Only one of >> +the two should be present in the output. >> + >> - drm-shared-<region>: <uint> [KiB|MiB] >> >> The total size of buffers that are shared with another file (e.g., have more >> @@ -159,20 +164,34 @@ than a single handle). >> >> - drm-total-<region>: <uint> [KiB|MiB] >> >> -The total size of buffers that including shared and private memory. >> +The total size of all created buffers including shared and private memory. The >> +backing store for the buffers does not have to be currently instantiated to be >> +counted under this category. >> >> - drm-resident-<region>: <uint> [KiB|MiB] >> >> -The total size of buffers that are resident in the specified region. >> +The total size of buffers that are resident (have their backing store present or >> +instantiated) in the specified region. >> + >> +This is an alias for drm-memory-<region> and only one of the two should be >> +present in the output. >> >> - drm-purgeable-<region>: <uint> [KiB|MiB] >> >> The total size of buffers that are purgeable. >> >> +For example drivers which implement a form of 'madvise' like functionality can >> +here count buffers which have instantiated backing store, but have been marked >> +with an equivalent of MADV_DONTNEED. >> + >> - drm-active-<region>: <uint> [KiB|MiB] >> >> The total size of buffers that are active on one or more engines. >> >> +One practical example of this can be presence of unsignaled fences in an GEM >> +buffer reservation object. Therefore the active category is a subset of >> +resident. >> + >> Implementation Details >> ====================== >> >> -- >> 2.44.0 >>
On Thu, Aug 15, 2024 at 09:37:31AM +0100, Tvrtko Ursulin wrote: > > On 13/08/2024 19:47, Rob Clark wrote: > > On Tue, Aug 13, 2024 at 6:57 AM Tvrtko Ursulin <tursulin@igalia.com> wrote: > > > > > > From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > > > > > > Currently it is not well defined what is drm-memory- compared to other > > > categories. > > > > > > In practice the only driver which emits these keys is amdgpu and in them > > > exposes the current resident buffer object memory (including shared). > > > > > > To prevent any confusion, document that drm-memory- is deprecated and an > > > alias for drm-resident-memory-. > > > > > > While at it also clarify that the reserved sub-string 'memory' refers to > > > the memory region component, and also clarify the intended semantics of > > > other memory categories. > > > > > > v2: > > > * Also mark drm-memory- as deprecated. > > > * Add some more text describing memory categories. (Alex) > > > > > > v3: > > > * Semantics of the amdgpu drm-memory is actually as drm-resident. > > > > > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > > > Cc: Alex Deucher <alexander.deucher@amd.com> > > > Cc: Christian König <christian.keonig@amd.com> > > > Cc: Rob Clark <robdclark@chromium.org> > > > > Reviewed-by: Rob Clark <robdclark@gmail.com> > > Thanks! > > So this one is stand alone and could be pushed to drm-misc-next. fwiw on the series Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > 2/2 can wait for AMD to give a verdict. Imo best to wait a bit, unless Alex takes a while to get around to this. -Sima > > Regards, > > Tvrtko > > > > > > --- > > > Documentation/gpu/drm-usage-stats.rst | 25 ++++++++++++++++++++++--- > > > 1 file changed, 22 insertions(+), 3 deletions(-) > > > > > > diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst > > > index a80f95ca1b2f..ff964c707754 100644 > > > --- a/Documentation/gpu/drm-usage-stats.rst > > > +++ b/Documentation/gpu/drm-usage-stats.rst > > > @@ -144,7 +144,9 @@ Memory > > > > > > Each possible memory type which can be used to store buffer objects by the > > > GPU in question shall be given a stable and unique name to be returned as the > > > -string here. The name "memory" is reserved to refer to normal system memory. > > > +string here. > > > + > > > +The region name "memory" is reserved to refer to normal system memory. > > > > > > Value shall reflect the amount of storage currently consumed by the buffer > > > objects belong to this client, in the respective memory region. > > > @@ -152,6 +154,9 @@ objects belong to this client, in the respective memory region. > > > Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' > > > indicating kibi- or mebi-bytes. > > > > > > +This key is deprecated and is an alias for drm-resident-<region>. Only one of > > > +the two should be present in the output. > > > + > > > - drm-shared-<region>: <uint> [KiB|MiB] > > > > > > The total size of buffers that are shared with another file (e.g., have more > > > @@ -159,20 +164,34 @@ than a single handle). > > > > > > - drm-total-<region>: <uint> [KiB|MiB] > > > > > > -The total size of buffers that including shared and private memory. > > > +The total size of all created buffers including shared and private memory. The > > > +backing store for the buffers does not have to be currently instantiated to be > > > +counted under this category. > > > > > > - drm-resident-<region>: <uint> [KiB|MiB] > > > > > > -The total size of buffers that are resident in the specified region. > > > +The total size of buffers that are resident (have their backing store present or > > > +instantiated) in the specified region. > > > + > > > +This is an alias for drm-memory-<region> and only one of the two should be > > > +present in the output. > > > > > > - drm-purgeable-<region>: <uint> [KiB|MiB] > > > > > > The total size of buffers that are purgeable. > > > > > > +For example drivers which implement a form of 'madvise' like functionality can > > > +here count buffers which have instantiated backing store, but have been marked > > > +with an equivalent of MADV_DONTNEED. > > > + > > > - drm-active-<region>: <uint> [KiB|MiB] > > > > > > The total size of buffers that are active on one or more engines. > > > > > > +One practical example of this can be presence of unsignaled fences in an GEM > > > +buffer reservation object. Therefore the active category is a subset of > > > +resident. > > > + > > > Implementation Details > > > ====================== > > > > > > -- > > > 2.44.0 > > >
On Tue, Aug 13, 2024 at 9:57 AM Tvrtko Ursulin <tursulin@igalia.com> wrote: > > From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > > Currently it is not well defined what is drm-memory- compared to other > categories. > > In practice the only driver which emits these keys is amdgpu and in them > exposes the current resident buffer object memory (including shared). > > To prevent any confusion, document that drm-memory- is deprecated and an > alias for drm-resident-memory-. > > While at it also clarify that the reserved sub-string 'memory' refers to > the memory region component, and also clarify the intended semantics of > other memory categories. > > v2: > * Also mark drm-memory- as deprecated. > * Add some more text describing memory categories. (Alex) > > v3: > * Semantics of the amdgpu drm-memory is actually as drm-resident. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Christian König <christian.keonig@amd.com> > Cc: Rob Clark <robdclark@chromium.org> > --- > Documentation/gpu/drm-usage-stats.rst | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst > index a80f95ca1b2f..ff964c707754 100644 > --- a/Documentation/gpu/drm-usage-stats.rst > +++ b/Documentation/gpu/drm-usage-stats.rst > @@ -144,7 +144,9 @@ Memory > > Each possible memory type which can be used to store buffer objects by the > GPU in question shall be given a stable and unique name to be returned as the > -string here. The name "memory" is reserved to refer to normal system memory. > +string here. > + > +The region name "memory" is reserved to refer to normal system memory. > > Value shall reflect the amount of storage currently consumed by the buffer > objects belong to this client, in the respective memory region. > @@ -152,6 +154,9 @@ objects belong to this client, in the respective memory region. > Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' > indicating kibi- or mebi-bytes. > > +This key is deprecated and is an alias for drm-resident-<region>. Only one of > +the two should be present in the output. > + I'm not sure how best to handle this. What should amdgpu do? We have customers out in the field using these existing fields and then with patch 2, they go away. Arguably we'd want both for backwards compatibility. Alex > - drm-shared-<region>: <uint> [KiB|MiB] > > The total size of buffers that are shared with another file (e.g., have more > @@ -159,20 +164,34 @@ than a single handle). > > - drm-total-<region>: <uint> [KiB|MiB] > > -The total size of buffers that including shared and private memory. > +The total size of all created buffers including shared and private memory. The > +backing store for the buffers does not have to be currently instantiated to be > +counted under this category. > > - drm-resident-<region>: <uint> [KiB|MiB] > > -The total size of buffers that are resident in the specified region. > +The total size of buffers that are resident (have their backing store present or > +instantiated) in the specified region. > + > +This is an alias for drm-memory-<region> and only one of the two should be > +present in the output. > > - drm-purgeable-<region>: <uint> [KiB|MiB] > > The total size of buffers that are purgeable. > > +For example drivers which implement a form of 'madvise' like functionality can > +here count buffers which have instantiated backing store, but have been marked > +with an equivalent of MADV_DONTNEED. > + > - drm-active-<region>: <uint> [KiB|MiB] > > The total size of buffers that are active on one or more engines. > > +One practical example of this can be presence of unsignaled fences in an GEM > +buffer reservation object. Therefore the active category is a subset of > +resident. > + > Implementation Details > ====================== > > -- > 2.44.0 >
On 21/08/2024 21:47, Alex Deucher wrote: > On Tue, Aug 13, 2024 at 9:57 AM Tvrtko Ursulin <tursulin@igalia.com> wrote: >> >> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> >> >> Currently it is not well defined what is drm-memory- compared to other >> categories. >> >> In practice the only driver which emits these keys is amdgpu and in them >> exposes the current resident buffer object memory (including shared). >> >> To prevent any confusion, document that drm-memory- is deprecated and an >> alias for drm-resident-memory-. >> >> While at it also clarify that the reserved sub-string 'memory' refers to >> the memory region component, and also clarify the intended semantics of >> other memory categories. >> >> v2: >> * Also mark drm-memory- as deprecated. >> * Add some more text describing memory categories. (Alex) >> >> v3: >> * Semantics of the amdgpu drm-memory is actually as drm-resident. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> >> Cc: Alex Deucher <alexander.deucher@amd.com> >> Cc: Christian König <christian.keonig@amd.com> >> Cc: Rob Clark <robdclark@chromium.org> >> --- >> Documentation/gpu/drm-usage-stats.rst | 25 ++++++++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst >> index a80f95ca1b2f..ff964c707754 100644 >> --- a/Documentation/gpu/drm-usage-stats.rst >> +++ b/Documentation/gpu/drm-usage-stats.rst >> @@ -144,7 +144,9 @@ Memory >> >> Each possible memory type which can be used to store buffer objects by the >> GPU in question shall be given a stable and unique name to be returned as the >> -string here. The name "memory" is reserved to refer to normal system memory. >> +string here. >> + >> +The region name "memory" is reserved to refer to normal system memory. >> >> Value shall reflect the amount of storage currently consumed by the buffer >> objects belong to this client, in the respective memory region. >> @@ -152,6 +154,9 @@ objects belong to this client, in the respective memory region. >> Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' >> indicating kibi- or mebi-bytes. >> >> +This key is deprecated and is an alias for drm-resident-<region>. Only one of >> +the two should be present in the output. >> + > > I'm not sure how best to handle this. What should amdgpu do? We have > customers out in the field using these existing fields and then with > patch 2, they go away. Arguably we'd want both for backwards > compatibility. Exactly, so it looks you maybe missed that 2/2 is not removing the amdgpu "legacy" drm-memory-. It keeps outputting it and also duplicating under drm-resident-. This is mentioned in the commit paragraph: """ Legacy keys have been preserved, with the outlook of only potentially removing only the drm-memory- when the time gets right. """ Put differently, I don't think 2/2 should break the existing tools/parsers. Only if they have hardcoded assumptions about the order of keys perhaps? Regards, Tvrtko >> - drm-shared-<region>: <uint> [KiB|MiB] >> >> The total size of buffers that are shared with another file (e.g., have more >> @@ -159,20 +164,34 @@ than a single handle). >> >> - drm-total-<region>: <uint> [KiB|MiB] >> >> -The total size of buffers that including shared and private memory. >> +The total size of all created buffers including shared and private memory. The >> +backing store for the buffers does not have to be currently instantiated to be >> +counted under this category. >> >> - drm-resident-<region>: <uint> [KiB|MiB] >> >> -The total size of buffers that are resident in the specified region. >> +The total size of buffers that are resident (have their backing store present or >> +instantiated) in the specified region. >> + >> +This is an alias for drm-memory-<region> and only one of the two should be >> +present in the output. >> >> - drm-purgeable-<region>: <uint> [KiB|MiB] >> >> The total size of buffers that are purgeable. >> >> +For example drivers which implement a form of 'madvise' like functionality can >> +here count buffers which have instantiated backing store, but have been marked >> +with an equivalent of MADV_DONTNEED. >> + >> - drm-active-<region>: <uint> [KiB|MiB] >> >> The total size of buffers that are active on one or more engines. >> >> +One practical example of this can be presence of unsignaled fences in an GEM >> +buffer reservation object. Therefore the active category is a subset of >> +resident. >> + >> Implementation Details >> ====================== >> >> -- >> 2.44.0 >>
On Wed, Sep 4, 2024 at 4:36 AM Tvrtko Ursulin <tvrtko.ursulin@igalia.com> wrote: > > > On 21/08/2024 21:47, Alex Deucher wrote: > > On Tue, Aug 13, 2024 at 9:57 AM Tvrtko Ursulin <tursulin@igalia.com> wrote: > >> > >> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > >> > >> Currently it is not well defined what is drm-memory- compared to other > >> categories. > >> > >> In practice the only driver which emits these keys is amdgpu and in them > >> exposes the current resident buffer object memory (including shared). > >> > >> To prevent any confusion, document that drm-memory- is deprecated and an > >> alias for drm-resident-memory-. > >> > >> While at it also clarify that the reserved sub-string 'memory' refers to > >> the memory region component, and also clarify the intended semantics of > >> other memory categories. > >> > >> v2: > >> * Also mark drm-memory- as deprecated. > >> * Add some more text describing memory categories. (Alex) > >> > >> v3: > >> * Semantics of the amdgpu drm-memory is actually as drm-resident. > >> > >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > >> Cc: Alex Deucher <alexander.deucher@amd.com> > >> Cc: Christian König <christian.keonig@amd.com> > >> Cc: Rob Clark <robdclark@chromium.org> > >> --- > >> Documentation/gpu/drm-usage-stats.rst | 25 ++++++++++++++++++++++--- > >> 1 file changed, 22 insertions(+), 3 deletions(-) > >> > >> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst > >> index a80f95ca1b2f..ff964c707754 100644 > >> --- a/Documentation/gpu/drm-usage-stats.rst > >> +++ b/Documentation/gpu/drm-usage-stats.rst > >> @@ -144,7 +144,9 @@ Memory > >> > >> Each possible memory type which can be used to store buffer objects by the > >> GPU in question shall be given a stable and unique name to be returned as the > >> -string here. The name "memory" is reserved to refer to normal system memory. > >> +string here. > >> + > >> +The region name "memory" is reserved to refer to normal system memory. > >> > >> Value shall reflect the amount of storage currently consumed by the buffer > >> objects belong to this client, in the respective memory region. > >> @@ -152,6 +154,9 @@ objects belong to this client, in the respective memory region. > >> Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' > >> indicating kibi- or mebi-bytes. > >> > >> +This key is deprecated and is an alias for drm-resident-<region>. Only one of > >> +the two should be present in the output. > >> + > > > > I'm not sure how best to handle this. What should amdgpu do? We have > > customers out in the field using these existing fields and then with > > patch 2, they go away. Arguably we'd want both for backwards > > compatibility. > > Exactly, so it looks you maybe missed that 2/2 is not removing the > amdgpu "legacy" drm-memory-. It keeps outputting it and also duplicating > under drm-resident-. This is mentioned in the commit paragraph: > > """ > Legacy keys have been preserved, with the outlook of only potentially > removing only the drm-memory- when the time gets right. > """ > > Put differently, I don't think 2/2 should break the existing > tools/parsers. Only if they have hardcoded assumptions about the order > of keys perhaps? You're right. I totally missed that part. The series is: Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Happy to take the patches through my tree or patch 2 via mine and patch 1 via drm-misc. Alex > > Regards, > > Tvrtko > > >> - drm-shared-<region>: <uint> [KiB|MiB] > >> > >> The total size of buffers that are shared with another file (e.g., have more > >> @@ -159,20 +164,34 @@ than a single handle). > >> > >> - drm-total-<region>: <uint> [KiB|MiB] > >> > >> -The total size of buffers that including shared and private memory. > >> +The total size of all created buffers including shared and private memory. The > >> +backing store for the buffers does not have to be currently instantiated to be > >> +counted under this category. > >> > >> - drm-resident-<region>: <uint> [KiB|MiB] > >> > >> -The total size of buffers that are resident in the specified region. > >> +The total size of buffers that are resident (have their backing store present or > >> +instantiated) in the specified region. > >> + > >> +This is an alias for drm-memory-<region> and only one of the two should be > >> +present in the output. > >> > >> - drm-purgeable-<region>: <uint> [KiB|MiB] > >> > >> The total size of buffers that are purgeable. > >> > >> +For example drivers which implement a form of 'madvise' like functionality can > >> +here count buffers which have instantiated backing store, but have been marked > >> +with an equivalent of MADV_DONTNEED. > >> + > >> - drm-active-<region>: <uint> [KiB|MiB] > >> > >> The total size of buffers that are active on one or more engines. > >> > >> +One practical example of this can be presence of unsignaled fences in an GEM > >> +buffer reservation object. Therefore the active category is a subset of > >> +resident. > >> + > >> Implementation Details > >> ====================== > >> > >> -- > >> 2.44.0 > >>
On 06/09/2024 19:12, Alex Deucher wrote: > On Wed, Sep 4, 2024 at 4:36 AM Tvrtko Ursulin <tvrtko.ursulin@igalia.com> wrote: >> >> >> On 21/08/2024 21:47, Alex Deucher wrote: >>> On Tue, Aug 13, 2024 at 9:57 AM Tvrtko Ursulin <tursulin@igalia.com> wrote: >>>> >>>> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> >>>> >>>> Currently it is not well defined what is drm-memory- compared to other >>>> categories. >>>> >>>> In practice the only driver which emits these keys is amdgpu and in them >>>> exposes the current resident buffer object memory (including shared). >>>> >>>> To prevent any confusion, document that drm-memory- is deprecated and an >>>> alias for drm-resident-memory-. >>>> >>>> While at it also clarify that the reserved sub-string 'memory' refers to >>>> the memory region component, and also clarify the intended semantics of >>>> other memory categories. >>>> >>>> v2: >>>> * Also mark drm-memory- as deprecated. >>>> * Add some more text describing memory categories. (Alex) >>>> >>>> v3: >>>> * Semantics of the amdgpu drm-memory is actually as drm-resident. >>>> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> >>>> Cc: Alex Deucher <alexander.deucher@amd.com> >>>> Cc: Christian König <christian.keonig@amd.com> >>>> Cc: Rob Clark <robdclark@chromium.org> >>>> --- >>>> Documentation/gpu/drm-usage-stats.rst | 25 ++++++++++++++++++++++--- >>>> 1 file changed, 22 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst >>>> index a80f95ca1b2f..ff964c707754 100644 >>>> --- a/Documentation/gpu/drm-usage-stats.rst >>>> +++ b/Documentation/gpu/drm-usage-stats.rst >>>> @@ -144,7 +144,9 @@ Memory >>>> >>>> Each possible memory type which can be used to store buffer objects by the >>>> GPU in question shall be given a stable and unique name to be returned as the >>>> -string here. The name "memory" is reserved to refer to normal system memory. >>>> +string here. >>>> + >>>> +The region name "memory" is reserved to refer to normal system memory. >>>> >>>> Value shall reflect the amount of storage currently consumed by the buffer >>>> objects belong to this client, in the respective memory region. >>>> @@ -152,6 +154,9 @@ objects belong to this client, in the respective memory region. >>>> Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' >>>> indicating kibi- or mebi-bytes. >>>> >>>> +This key is deprecated and is an alias for drm-resident-<region>. Only one of >>>> +the two should be present in the output. >>>> + >>> >>> I'm not sure how best to handle this. What should amdgpu do? We have >>> customers out in the field using these existing fields and then with >>> patch 2, they go away. Arguably we'd want both for backwards >>> compatibility. >> >> Exactly, so it looks you maybe missed that 2/2 is not removing the >> amdgpu "legacy" drm-memory-. It keeps outputting it and also duplicating >> under drm-resident-. This is mentioned in the commit paragraph: >> >> """ >> Legacy keys have been preserved, with the outlook of only potentially >> removing only the drm-memory- when the time gets right. >> """ >> >> Put differently, I don't think 2/2 should break the existing >> tools/parsers. Only if they have hardcoded assumptions about the order >> of keys perhaps? > > You're right. I totally missed that part. The series is: > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > Happy to take the patches through my tree or patch 2 via mine and > patch 1 via drm-misc. Thanks! As to how to merge I don't have a strong preference. Adding Maxime and Thomas to check if they would want to take 1/2 via drm-misc-next? But IMO not splitting the patches between trees also kind of makes sense. Regards, Tvrtko > > Alex > >> >> Regards, >> >> Tvrtko >> >>>> - drm-shared-<region>: <uint> [KiB|MiB] >>>> >>>> The total size of buffers that are shared with another file (e.g., have more >>>> @@ -159,20 +164,34 @@ than a single handle). >>>> >>>> - drm-total-<region>: <uint> [KiB|MiB] >>>> >>>> -The total size of buffers that including shared and private memory. >>>> +The total size of all created buffers including shared and private memory. The >>>> +backing store for the buffers does not have to be currently instantiated to be >>>> +counted under this category. >>>> >>>> - drm-resident-<region>: <uint> [KiB|MiB] >>>> >>>> -The total size of buffers that are resident in the specified region. >>>> +The total size of buffers that are resident (have their backing store present or >>>> +instantiated) in the specified region. >>>> + >>>> +This is an alias for drm-memory-<region> and only one of the two should be >>>> +present in the output. >>>> >>>> - drm-purgeable-<region>: <uint> [KiB|MiB] >>>> >>>> The total size of buffers that are purgeable. >>>> >>>> +For example drivers which implement a form of 'madvise' like functionality can >>>> +here count buffers which have instantiated backing store, but have been marked >>>> +with an equivalent of MADV_DONTNEED. >>>> + >>>> - drm-active-<region>: <uint> [KiB|MiB] >>>> >>>> The total size of buffers that are active on one or more engines. >>>> >>>> +One practical example of this can be presence of unsignaled fences in an GEM >>>> +buffer reservation object. Therefore the active category is a subset of >>>> +resident. >>>> + >>>> Implementation Details >>>> ====================== >>>> >>>> -- >>>> 2.44.0 >>>>
diff --git a/Documentation/gpu/drm-usage-stats.rst b/Documentation/gpu/drm-usage-stats.rst index a80f95ca1b2f..ff964c707754 100644 --- a/Documentation/gpu/drm-usage-stats.rst +++ b/Documentation/gpu/drm-usage-stats.rst @@ -144,7 +144,9 @@ Memory Each possible memory type which can be used to store buffer objects by the GPU in question shall be given a stable and unique name to be returned as the -string here. The name "memory" is reserved to refer to normal system memory. +string here. + +The region name "memory" is reserved to refer to normal system memory. Value shall reflect the amount of storage currently consumed by the buffer objects belong to this client, in the respective memory region. @@ -152,6 +154,9 @@ objects belong to this client, in the respective memory region. Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB' indicating kibi- or mebi-bytes. +This key is deprecated and is an alias for drm-resident-<region>. Only one of +the two should be present in the output. + - drm-shared-<region>: <uint> [KiB|MiB] The total size of buffers that are shared with another file (e.g., have more @@ -159,20 +164,34 @@ than a single handle). - drm-total-<region>: <uint> [KiB|MiB] -The total size of buffers that including shared and private memory. +The total size of all created buffers including shared and private memory. The +backing store for the buffers does not have to be currently instantiated to be +counted under this category. - drm-resident-<region>: <uint> [KiB|MiB] -The total size of buffers that are resident in the specified region. +The total size of buffers that are resident (have their backing store present or +instantiated) in the specified region. + +This is an alias for drm-memory-<region> and only one of the two should be +present in the output. - drm-purgeable-<region>: <uint> [KiB|MiB] The total size of buffers that are purgeable. +For example drivers which implement a form of 'madvise' like functionality can +here count buffers which have instantiated backing store, but have been marked +with an equivalent of MADV_DONTNEED. + - drm-active-<region>: <uint> [KiB|MiB] The total size of buffers that are active on one or more engines. +One practical example of this can be presence of unsignaled fences in an GEM +buffer reservation object. Therefore the active category is a subset of +resident. + Implementation Details ======================