[RFC 0/2] DRM standardized memory stats

[RFC 0/2] DRM standardized memory stats

[Tvrtko Ursulin]

Re-floating a simple idea (this time with more Cc) born from an user interest to
be able to correctly size the swap space during OS installation, based on the
size of the VRAM a discrete GPU might have. And to be able to do it in a vendor
agnostic way.

Idea is to expose a standardized scheme in sysfs, under the DRM class card, and
under a new 'memstat' directory. Such as, example from amdgpu:

/sys/class/drm/card1/memstat/
├── gtt
│   ├── total_mb
│   └── used_mb
└── vram
    ├── total_mb
    └── used_mb

Or with concrete numbers:

$ grep -Hr . /sys/class/drm/card1/memstat/
/sys/class/drm/card1/memstat/vram/total_mb:1024
/sys/class/drm/card1/memstat/vram/used_mb:445
/sys/class/drm/card1/memstat/gtt/total_mb:7394
/sys/class/drm/card1/memstat/gtt/used_mb:71

Drivers need to implement a simple DRM driver level callback which needs to
report a stable list of interesting memory regions and their respective stats.
The region names then become sub-directory names under the new 'memstat'
directory, with each region exposing the total size and the current usage.

Similar data can already be queried if the dmem cgroup controller is enabled,
also only for the participating drivers, by querying the root cgroup. But
perhaps sysfs is easier, or perhaps it is too much code for too little benefit.
I am curious to hear any opinions.

Cc: Maíra Canal <mcanal@igalia.com>
Cc: Ludovico de Nittis <ludovico.denittis@collabora.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>

Tvrtko Ursulin (2):
  drm: Allow drivers to report standardized memory stats
  drm/amdgpu: Wire up DRM memory stats reporting

 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |   6 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c |  40 +++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  37 +++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |   3 +
 drivers/gpu/drm/drm_drv.c                  |  10 ++
 drivers/gpu/drm/drm_sysfs.c                | 123 +++++++++++++++++++++
 include/drm/drm_device.h                   |  19 ++++
 include/drm/drm_drv.h                      |   8 ++
 include/drm/drm_file.h                     |   9 ++
 include/drm/drm_sysfs.h                    |   4 +
 11 files changed, 248 insertions(+), 13 deletions(-)

-- 
2.55.0

[RFC 1/2] drm: Allow drivers to report standardized memory stats

[Tvrtko Ursulin]

Add a DRM driver level vfunc which allows drivers to opt-in into
reporting standardized memory stats via sysfs.

Via the vfunc drivers can report a list of memory regions in a stable
order, with the names following the memory region names used for fdinfo
memory stats. The region names become directory names under the parent
'memstat' directory.

Each region reports the total size of the region and its current usage,
represented respectively in the total_mb and used_mb files present under
the aforementioned memory region named directories.

The card level 'memstat' directory is only created for drivers who report
at least one memory region.

Amdgpu example:

/sys/class/drm/card1/memstat/
├── gtt
│   ├── total_mb
│   └── used_mb
└── vram
    ├── total_mb
    └── used_mb

Or with concrete numbers:

$ grep -Hr . /sys/class/drm/card1/memstat/
/sys/class/drm/card1/memstat/vram/total_mb:1024
/sys/class/drm/card1/memstat/vram/used_mb:445
/sys/class/drm/card1/memstat/gtt/total_mb:7394
/sys/class/drm/card1/memstat/gtt/used_mb:71

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Maíra Canal <mcanal@igalia.com>
Cc: Ludovico de Nittis <ludovico.denittis@collabora.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
---
 drivers/gpu/drm/drm_drv.c   |  10 +++
 drivers/gpu/drm/drm_sysfs.c | 123 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_device.h    |  19 ++++++
 include/drm/drm_drv.h       |   8 +++
 include/drm/drm_file.h      |   9 +++
 include/drm/drm_sysfs.h     |   4 ++
 6 files changed, 173 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 0cdc606af8d1..ab26cfd73002 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -54,6 +54,7 @@
 #include <drm/drm_print.h>
 #include <drm/drm_privacy_screen_machine.h>
 #include <drm/drm_ras_genl_family.h>
+#include <drm/drm_sysfs.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -197,10 +198,15 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
 	if (ret)
 		goto err_debugfs;
 
+	ret = drm_sysfs_register_memstat(minor);
+	if (ret)
+		goto err_kdev;
+
 	/* replace NULL with @minor so lookups will succeed from now on */
 	entry = xa_store(drm_minor_get_xa(type), minor->index, minor, GFP_KERNEL);
 	if (xa_is_err(entry)) {
 		ret = xa_err(entry);
+		drm_sysfs_unregister_memstat(minor);
 		goto err_debugfs;
 	}
 	WARN_ON(entry);
@@ -208,6 +214,9 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
 	DRM_DEBUG("new minor registered %d\n", minor->index);
 	return 0;
 
+err_kdev:
+	device_del(minor->kdev);
+
 err_debugfs:
 	drm_debugfs_unregister(minor);
 	return ret;
@@ -224,6 +233,7 @@ static void drm_minor_unregister(struct drm_device *dev, enum drm_minor_type typ
 	/* replace @minor with NULL so lookups will fail from now on */
 	xa_store(drm_minor_get_xa(type), minor->index, NULL, GFP_KERNEL);
 
+	drm_sysfs_unregister_memstat(minor);
 	device_del(minor->kdev);
 	dev_set_drvdata(minor->kdev, NULL); /* safety belt */
 	drm_debugfs_unregister(minor);
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 7b16d32b6ff6..4a8c5b0b458d 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -25,6 +25,7 @@
 #include <drm/drm_accel.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_device.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_file.h>
 #include <drm/drm_modes.h>
 #include <drm/drm_print.h>
@@ -589,3 +590,125 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
 	put_device(kdev);
 	return ERR_PTR(r);
 }
+
+static ssize_t total_mb_show(struct kobject *kobj, struct kobj_attribute *attr,
+			     char *buf)
+{
+	struct drm_memstat_attr *mattr = container_of(kobj, typeof(*mattr), kobj);
+	struct drm_device *drm = mattr->drm;
+	const struct drm_memory_info *info;
+
+	if (drm->driver->get_memory_info) {
+		info = drm->driver->get_memory_info(drm);
+		if (info)
+			return sysfs_emit(buf, "%u\n",
+					  info->region[mattr->region].total_mb);
+	}
+
+	return 0;
+}
+
+static ssize_t used_mb_show(struct kobject *kobj, struct kobj_attribute *attr,
+			    char *buf)
+{
+	struct drm_memstat_attr *mattr = container_of(kobj, typeof(*mattr), kobj);
+	struct drm_device *drm = mattr->drm;
+	const struct drm_memory_info *info;
+
+	if (drm->driver->get_memory_info) {
+		info = drm->driver->get_memory_info(drm);
+		if (info)
+			return sysfs_emit(buf, "%u\n",
+					  info->region[mattr->region].used_mb);
+	}
+
+	return 0;
+}
+
+static struct kobj_attribute total_attr = __ATTR_RO(total_mb);
+static struct kobj_attribute used_attr = __ATTR_RO(used_mb);
+
+static struct attribute *memstat_attrs[] = {
+	&total_attr.attr,
+	&used_attr.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(memstat);
+
+static void kobj_memstat_release(struct kobject *kobj)
+{
+}
+
+static const struct kobj_type memstat_ktype = {
+	.release = kobj_memstat_release,
+	.sysfs_ops = &kobj_sysfs_ops,
+	.default_groups = memstat_groups,
+};
+
+int drm_sysfs_register_memstat(struct drm_minor *minor)
+{
+	const struct drm_memory_info *info = NULL;
+	struct drm_device *drm = minor->dev;
+	struct drm_memstat_attr *regions;
+	struct kobject *root;
+	int i, ret = -ENOMEM;
+
+	if (drm->driver->get_memory_info)
+		info = drm->driver->get_memory_info(drm);
+
+	if (!info || !info->num_regions)
+		return 0;
+
+	regions = kmalloc_objs(*regions, info->num_regions + 1);
+	if (!regions)
+		return -ENOMEM;
+
+	root = kobject_create_and_add("memstat", &minor->kdev->kobj);
+	if (!root)
+		goto err_root;
+
+	for (i = 0; i < info->num_regions; i++) {
+		regions[i].region = i;
+		regions[i].drm = drm;
+		ret = kobject_init_and_add(&regions[i].kobj, &memstat_ktype,
+					   root, "%s", info->region[i].name);
+		if (ret)
+			goto err_regions;
+	}
+
+	minor->sysfs_memstat = root;
+	minor->sysfs_memstat_regions = regions;
+
+	return 0;
+
+err_regions:
+	while (i-- > 0) {
+		kobject_del(&regions[i].kobj);
+		kobject_put(&regions[i].kobj);
+	}
+
+	kobject_del(root);
+	kobject_put(root);
+
+err_root:
+	kfree(regions);
+
+	return ret;
+}
+
+void drm_sysfs_unregister_memstat(struct drm_minor *minor)
+{
+	struct drm_memstat_attr *region = minor->sysfs_memstat_regions;
+
+	while (region && region->kobj.state_initialized) {
+		kobject_del(&region->kobj);
+		kobject_put(&region->kobj);
+		region++;
+	}
+
+	kfree(minor->sysfs_memstat_regions);
+
+	kobject_del(minor->sysfs_memstat);
+	kobject_put(minor->sysfs_memstat);
+}
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 75f030d027ee..0c9260c85b30 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -67,6 +67,25 @@ enum switch_power_state {
 	DRM_SWITCH_POWER_DYNAMIC_OFF = 3,
 };
 
+/**
+ * struct drm_memory_region_info - Memory name and stats for a region
+ * @name: Name of the memory region
+ * @total_mb: Total available memory in mebibytes
+ * @used_mb: Used memory in mebibytes
+ */
+struct drm_memory_region_info {
+	char	name[24];
+
+	u32	total_mb;
+	u32	used_mb;
+};
+
+struct drm_memory_info {
+	unsigned int num_regions;
+
+	struct drm_memory_region_info region[];
+};
+
 /**
  * struct drm_device - DRM device structure
  *
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index b23830494ed4..a3f1906961ac 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -45,6 +45,7 @@ struct drm_minor;
 struct dma_buf;
 struct dma_buf_attachment;
 struct drm_display_mode;
+struct drm_device_memory_info;
 struct drm_mode_create_dumb;
 struct drm_printer;
 struct sg_table;
@@ -387,6 +388,13 @@ struct drm_driver {
 	 */
 	void (*show_fdinfo)(struct drm_printer *p, struct drm_file *f);
 
+	/**
+	 * @get_memory_info:
+	 *
+	 * Get device specific memory info.  See Documentation/gpu/drm-memory-info.rst.
+	 */
+	const struct drm_memory_info * (*get_memory_info)(struct drm_device *dev);
+
 	/** @major: driver major number */
 	int major;
 	/** @minor: driver minor number */
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 6ee70ad65e1f..ebb227a91f60 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -66,6 +66,12 @@ enum drm_minor_type {
 	DRM_MINOR_ACCEL = 32,
 };
 
+struct drm_memstat_attr {
+	struct kobject kobj;
+	unsigned int region;
+	struct drm_device *drm;
+};
+
 /**
  * struct drm_minor - DRM device minor structure
  *
@@ -82,6 +88,9 @@ struct drm_minor {
 	struct device *kdev;		/* Linux device */
 	struct drm_device *dev;
 
+	struct kobject	*sysfs_memstat;
+	struct drm_memstat_attr  *sysfs_memstat_regions;
+
 	struct dentry *debugfs_symlink;
 	struct dentry *debugfs_root;
 };
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index 7695873a9456..10ed468965b0 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -4,10 +4,14 @@
 
 struct drm_device;
 struct drm_connector;
+struct drm_minor;
 struct drm_property;
 
 void drm_sysfs_hotplug_event(struct drm_device *dev);
 void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
 void drm_sysfs_connector_property_event(struct drm_connector *connector,
 					struct drm_property *property);
+
+int drm_sysfs_register_memstat(struct drm_minor *minor);
+void drm_sysfs_unregister_memstat(struct drm_minor *minor);
 #endif
-- 
2.55.0

Re: [RFC 1/2] drm: Allow drivers to report standardized memory stats

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] sysfs: UAF due to empty kobj release function and synchronous kfree
- [High] sysfs: uninitialized memory read in unregister loop leads to out-of-bounds access
- [High] drm: double-free of memstat regions on xa_store failure
- [Medium] sysfs: kobject leak in drm_sysfs_register_memstat on error path
--

commit d035e68d523234171ccd6d024a36f5efdbe545f4
Author: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Subject: drm: Allow drivers to report standardized memory stats

Adds a DRM driver level vfunc to opt-in into reporting standardized memory
stats via sysfs. Each region reports the total size and its current usage in
the total_mb and used_mb files.

> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 8c0879c336e5f..6302a72631005 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -197,10 +198,15 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
>  	if (ret)
>  		goto err_debugfs;
>  
> +	ret = drm_sysfs_register_memstat(minor);
> +	if (ret)
> +		goto err_kdev;
> +
>  	/* replace NULL with @minor so lookups will succeed from now on */
>  	entry = xa_store(drm_minor_get_xa(type), minor->index, minor, GFP_KERNEL);
>  	if (xa_is_err(entry)) {
>  		ret = xa_err(entry);
> +		drm_sysfs_unregister_memstat(minor);
>  		goto err_debugfs;
>  	}

[Severity: High]
Does this error path introduce a double-free?

If xa_store() fails, this jumps to err_debugfs, which bypasses
device_del(minor->kdev) and leaves the minor device registered.

When the caller eventually cleans up by calling drm_minor_unregister(), it
will call drm_sysfs_unregister_memstat(minor) a second time.

Since the first call doesn't set the sysfs_memstat_regions pointer to NULL,
won't this second call attempt to free the same regions array again?

> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 7b16d32b6ff65..4a8c5b0b458d5 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
[ ... ]
> +static void kobj_memstat_release(struct kobject *kobj)
> +{
> +}

[Severity: Critical]
Is it safe to use an empty release callback here?

An empty release function breaks the kobject lifecycle guarantees. If a
userspace process holds a sysfs file open (like total_mb) when the device is
unregistered, the sysfs core retains an elevated reference to this kobject.

> +
> +static const struct kobj_type memstat_ktype = {
> +	.release = kobj_memstat_release,
> +	.sysfs_ops = &kobj_sysfs_ops,
> +	.default_groups = memstat_groups,
> +};
> +
> +int drm_sysfs_register_memstat(struct drm_minor *minor)
> +{
[ ... ]
> +	regions = kmalloc_objs(*regions, info->num_regions + 1);
> +	if (!regions)
> +		return -ENOMEM;

[Severity: High]
Should this use kzalloc_objs() instead?

This allocation leaves the sentinel element at index info->num_regions
uninitialized.

> +
> +	root = kobject_create_and_add("memstat", &minor->kdev->kobj);
> +	if (!root)
> +		goto err_root;
> +
> +	for (i = 0; i < info->num_regions; i++) {
> +		regions[i].region = i;
> +		regions[i].drm = drm;
> +		ret = kobject_init_and_add(&regions[i].kobj, &memstat_ktype,
> +					   root, "%s", info->region[i].name);
> +		if (ret)
> +			goto err_regions;
> +	}
[ ... ]
> +err_regions:
> +	while (i-- > 0) {
> +		kobject_del(&regions[i].kobj);
> +		kobject_put(&regions[i].kobj);
> +	}

[Severity: Medium]
Does this cleanup loop leak the failing kobject?

If kobject_init_and_add() fails at index i, the code jumps to err_regions.
The while (i-- > 0) loop post-decrements i, skipping the cleanup for the
kobject at index i.

Since the kobject API requires calling kobject_put() even if
kobject_init_and_add() fails, wouldn't this leak the partially initialized
kobject and its allocated name?

[ ... ]
> +void drm_sysfs_unregister_memstat(struct drm_minor *minor)
> +{
> +	struct drm_memstat_attr *region = minor->sysfs_memstat_regions;
> +
> +	while (region && region->kobj.state_initialized) {

[Severity: High]
When iterating here, won't the loop read garbage memory for state_initialized
when it reaches the uninitialized sentinel element?

If the garbage memory happens to be non-zero, this could proceed to call
kobject_del() and kobject_put() on arbitrary memory.

> +		kobject_del(&region->kobj);
> +		kobject_put(&region->kobj);
> +		region++;
> +	}
> +
> +	kfree(minor->sysfs_memstat_regions);

[Severity: Critical]
Since kfree() is called synchronously right after kobject_put(), won't the
entire array be deleted even if the kobjects inside still have active
references?

This seems like it could lead to a use-after-free when userspace later reads
from the open sysfs file, since the empty release function above doesn't
delay the freeing of the memory until all references are dropped.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917120444.86471-1-tvrtko.ursulin@igalia.com?part=1

Re: [RFC 1/2] drm: Allow drivers to report standardized memory stats

[Thomas Zimmermann]

Hi

Am 17.09.26 um 14:04 schrieb Tvrtko Ursulin:
> Add a DRM driver level vfunc which allows drivers to opt-in into
> reporting standardized memory stats via sysfs.
>
> Via the vfunc drivers can report a list of memory regions in a stable
> order, with the names following the memory region names used for fdinfo
> memory stats. The region names become directory names under the parent
> 'memstat' directory.
>
> Each region reports the total size of the region and its current usage,
> represented respectively in the total_mb and used_mb files present under
> the aforementioned memory region named directories.
>
> The card level 'memstat' directory is only created for drivers who report
> at least one memory region.
>
> Amdgpu example:
>
> /sys/class/drm/card1/memstat/
> ├── gtt
> │   ├── total_mb
> │   └── used_mb
> └── vram
>      ├── total_mb
>      └── used_mb

This looks simple enough to be generally useful. A-b me.

>
> Or with concrete numbers:
>
> $ grep -Hr . /sys/class/drm/card1/memstat/
> /sys/class/drm/card1/memstat/vram/total_mb:1024
> /sys/class/drm/card1/memstat/vram/used_mb:445
> /sys/class/drm/card1/memstat/gtt/total_mb:7394
> /sys/class/drm/card1/memstat/gtt/used_mb:71

These seem easy to track.  What about other, shared regions?

IIRC some systems (RasPi) use CMA areas for allocating BOs and run out 
of that quickly. Could this be represented here?

Is there a representation of 'unknown value' ?

Best regards
Thomas

>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Maíra Canal <mcanal@igalia.com>
> Cc: Ludovico de Nittis <ludovico.denittis@collabora.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> ---
>   drivers/gpu/drm/drm_drv.c   |  10 +++
>   drivers/gpu/drm/drm_sysfs.c | 123 ++++++++++++++++++++++++++++++++++++
>   include/drm/drm_device.h    |  19 ++++++
>   include/drm/drm_drv.h       |   8 +++
>   include/drm/drm_file.h      |   9 +++
>   include/drm/drm_sysfs.h     |   4 ++
>   6 files changed, 173 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 0cdc606af8d1..ab26cfd73002 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -54,6 +54,7 @@
>   #include <drm/drm_print.h>
>   #include <drm/drm_privacy_screen_machine.h>
>   #include <drm/drm_ras_genl_family.h>
> +#include <drm/drm_sysfs.h>
>   
>   #include "drm_crtc_internal.h"
>   #include "drm_internal.h"
> @@ -197,10 +198,15 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
>   	if (ret)
>   		goto err_debugfs;
>   
> +	ret = drm_sysfs_register_memstat(minor);
> +	if (ret)
> +		goto err_kdev;
> +
>   	/* replace NULL with @minor so lookups will succeed from now on */
>   	entry = xa_store(drm_minor_get_xa(type), minor->index, minor, GFP_KERNEL);
>   	if (xa_is_err(entry)) {
>   		ret = xa_err(entry);
> +		drm_sysfs_unregister_memstat(minor);
>   		goto err_debugfs;
>   	}
>   	WARN_ON(entry);
> @@ -208,6 +214,9 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
>   	DRM_DEBUG("new minor registered %d\n", minor->index);
>   	return 0;
>   
> +err_kdev:
> +	device_del(minor->kdev);
> +
>   err_debugfs:
>   	drm_debugfs_unregister(minor);
>   	return ret;
> @@ -224,6 +233,7 @@ static void drm_minor_unregister(struct drm_device *dev, enum drm_minor_type typ
>   	/* replace @minor with NULL so lookups will fail from now on */
>   	xa_store(drm_minor_get_xa(type), minor->index, NULL, GFP_KERNEL);
>   
> +	drm_sysfs_unregister_memstat(minor);
>   	device_del(minor->kdev);
>   	dev_set_drvdata(minor->kdev, NULL); /* safety belt */
>   	drm_debugfs_unregister(minor);
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 7b16d32b6ff6..4a8c5b0b458d 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -25,6 +25,7 @@
>   #include <drm/drm_accel.h>
>   #include <drm/drm_connector.h>
>   #include <drm/drm_device.h>
> +#include <drm/drm_drv.h>
>   #include <drm/drm_file.h>
>   #include <drm/drm_modes.h>
>   #include <drm/drm_print.h>
> @@ -589,3 +590,125 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
>   	put_device(kdev);
>   	return ERR_PTR(r);
>   }
> +
> +static ssize_t total_mb_show(struct kobject *kobj, struct kobj_attribute *attr,
> +			     char *buf)
> +{
> +	struct drm_memstat_attr *mattr = container_of(kobj, typeof(*mattr), kobj);
> +	struct drm_device *drm = mattr->drm;
> +	const struct drm_memory_info *info;
> +
> +	if (drm->driver->get_memory_info) {
> +		info = drm->driver->get_memory_info(drm);
> +		if (info)
> +			return sysfs_emit(buf, "%u\n",
> +					  info->region[mattr->region].total_mb);
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t used_mb_show(struct kobject *kobj, struct kobj_attribute *attr,
> +			    char *buf)
> +{
> +	struct drm_memstat_attr *mattr = container_of(kobj, typeof(*mattr), kobj);
> +	struct drm_device *drm = mattr->drm;
> +	const struct drm_memory_info *info;
> +
> +	if (drm->driver->get_memory_info) {
> +		info = drm->driver->get_memory_info(drm);
> +		if (info)
> +			return sysfs_emit(buf, "%u\n",
> +					  info->region[mattr->region].used_mb);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct kobj_attribute total_attr = __ATTR_RO(total_mb);
> +static struct kobj_attribute used_attr = __ATTR_RO(used_mb);
> +
> +static struct attribute *memstat_attrs[] = {
> +	&total_attr.attr,
> +	&used_attr.attr,
> +	NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(memstat);
> +
> +static void kobj_memstat_release(struct kobject *kobj)
> +{
> +}
> +
> +static const struct kobj_type memstat_ktype = {
> +	.release = kobj_memstat_release,
> +	.sysfs_ops = &kobj_sysfs_ops,
> +	.default_groups = memstat_groups,
> +};
> +
> +int drm_sysfs_register_memstat(struct drm_minor *minor)
> +{
> +	const struct drm_memory_info *info = NULL;
> +	struct drm_device *drm = minor->dev;
> +	struct drm_memstat_attr *regions;
> +	struct kobject *root;
> +	int i, ret = -ENOMEM;
> +
> +	if (drm->driver->get_memory_info)
> +		info = drm->driver->get_memory_info(drm);
> +
> +	if (!info || !info->num_regions)
> +		return 0;
> +
> +	regions = kmalloc_objs(*regions, info->num_regions + 1);
> +	if (!regions)
> +		return -ENOMEM;
> +
> +	root = kobject_create_and_add("memstat", &minor->kdev->kobj);
> +	if (!root)
> +		goto err_root;
> +
> +	for (i = 0; i < info->num_regions; i++) {
> +		regions[i].region = i;
> +		regions[i].drm = drm;
> +		ret = kobject_init_and_add(&regions[i].kobj, &memstat_ktype,
> +					   root, "%s", info->region[i].name);
> +		if (ret)
> +			goto err_regions;
> +	}
> +
> +	minor->sysfs_memstat = root;
> +	minor->sysfs_memstat_regions = regions;
> +
> +	return 0;
> +
> +err_regions:
> +	while (i-- > 0) {
> +		kobject_del(&regions[i].kobj);
> +		kobject_put(&regions[i].kobj);
> +	}
> +
> +	kobject_del(root);
> +	kobject_put(root);
> +
> +err_root:
> +	kfree(regions);
> +
> +	return ret;
> +}
> +
> +void drm_sysfs_unregister_memstat(struct drm_minor *minor)
> +{
> +	struct drm_memstat_attr *region = minor->sysfs_memstat_regions;
> +
> +	while (region && region->kobj.state_initialized) {
> +		kobject_del(&region->kobj);
> +		kobject_put(&region->kobj);
> +		region++;
> +	}
> +
> +	kfree(minor->sysfs_memstat_regions);
> +
> +	kobject_del(minor->sysfs_memstat);
> +	kobject_put(minor->sysfs_memstat);
> +}
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index 75f030d027ee..0c9260c85b30 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -67,6 +67,25 @@ enum switch_power_state {
>   	DRM_SWITCH_POWER_DYNAMIC_OFF = 3,
>   };
>   
> +/**
> + * struct drm_memory_region_info - Memory name and stats for a region
> + * @name: Name of the memory region
> + * @total_mb: Total available memory in mebibytes
> + * @used_mb: Used memory in mebibytes
> + */
> +struct drm_memory_region_info {
> +	char	name[24];
> +
> +	u32	total_mb;
> +	u32	used_mb;
> +};
> +
> +struct drm_memory_info {
> +	unsigned int num_regions;
> +
> +	struct drm_memory_region_info region[];
> +};
> +
>   /**
>    * struct drm_device - DRM device structure
>    *
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index b23830494ed4..a3f1906961ac 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -45,6 +45,7 @@ struct drm_minor;
>   struct dma_buf;
>   struct dma_buf_attachment;
>   struct drm_display_mode;
> +struct drm_device_memory_info;
>   struct drm_mode_create_dumb;
>   struct drm_printer;
>   struct sg_table;
> @@ -387,6 +388,13 @@ struct drm_driver {
>   	 */
>   	void (*show_fdinfo)(struct drm_printer *p, struct drm_file *f);
>   
> +	/**
> +	 * @get_memory_info:
> +	 *
> +	 * Get device specific memory info.  See Documentation/gpu/drm-memory-info.rst.
> +	 */
> +	const struct drm_memory_info * (*get_memory_info)(struct drm_device *dev);
> +
>   	/** @major: driver major number */
>   	int major;
>   	/** @minor: driver minor number */
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index 6ee70ad65e1f..ebb227a91f60 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -66,6 +66,12 @@ enum drm_minor_type {
>   	DRM_MINOR_ACCEL = 32,
>   };
>   
> +struct drm_memstat_attr {
> +	struct kobject kobj;
> +	unsigned int region;
> +	struct drm_device *drm;
> +};
> +
>   /**
>    * struct drm_minor - DRM device minor structure
>    *
> @@ -82,6 +88,9 @@ struct drm_minor {
>   	struct device *kdev;		/* Linux device */
>   	struct drm_device *dev;
>   
> +	struct kobject	*sysfs_memstat;
> +	struct drm_memstat_attr  *sysfs_memstat_regions;
> +
>   	struct dentry *debugfs_symlink;
>   	struct dentry *debugfs_root;
>   };
> diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
> index 7695873a9456..10ed468965b0 100644
> --- a/include/drm/drm_sysfs.h
> +++ b/include/drm/drm_sysfs.h
> @@ -4,10 +4,14 @@
>   
>   struct drm_device;
>   struct drm_connector;
> +struct drm_minor;
>   struct drm_property;
>   
>   void drm_sysfs_hotplug_event(struct drm_device *dev);
>   void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
>   void drm_sysfs_connector_property_event(struct drm_connector *connector,
>   					struct drm_property *property);
> +
> +int drm_sysfs_register_memstat(struct drm_minor *minor);
> +void drm_sysfs_unregister_memstat(struct drm_minor *minor);
>   #endif

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)

Re: [RFC 1/2] drm: Allow drivers to report standardized memory stats

[Tvrtko Ursulin]

On 17/09/2026 13:38, Thomas Zimmermann wrote:
> Hi
> 
> Am 17.09.26 um 14:04 schrieb Tvrtko Ursulin:
>> Add a DRM driver level vfunc which allows drivers to opt-in into
>> reporting standardized memory stats via sysfs.
>>
>> Via the vfunc drivers can report a list of memory regions in a stable
>> order, with the names following the memory region names used for fdinfo
>> memory stats. The region names become directory names under the parent
>> 'memstat' directory.
>>
>> Each region reports the total size of the region and its current usage,
>> represented respectively in the total_mb and used_mb files present under
>> the aforementioned memory region named directories.
>>
>> The card level 'memstat' directory is only created for drivers who report
>> at least one memory region.
>>
>> Amdgpu example:
>>
>> /sys/class/drm/card1/memstat/
>> ├── gtt
>> │   ├── total_mb
>> │   └── used_mb
>> └── vram
>>      ├── total_mb
>>      └── used_mb
> 
> This looks simple enough to be generally useful. A-b me.

Thanks!

>>
>> Or with concrete numbers:
>>
>> $ grep -Hr . /sys/class/drm/card1/memstat/
>> /sys/class/drm/card1/memstat/vram/total_mb:1024
>> /sys/class/drm/card1/memstat/vram/used_mb:445
>> /sys/class/drm/card1/memstat/gtt/total_mb:7394
>> /sys/class/drm/card1/memstat/gtt/used_mb:71
> 
> These seem easy to track.  What about other, shared regions?
> 
> IIRC some systems (RasPi) use CMA areas for allocating BOs and run out 
> of that quickly. Could this be represented here?

Yes, it is upto drivers to decide which memory regions they want to 
expose here.

For drivers that currently expose per client memory stats via fdinfo the 
idea is that the region names match for ABI consistency. The set does 
not have to be 1:1 but names should match.

Should we go a step further and try to standardize on some region names? 
So far fdinfo did not do it.

> Is there a representation of 'unknown value' ?

No, what use case do you have in mind? And you mean for the memory 
region name or the numerical value?

Regards,

Tvrtko

>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>> Cc: Maíra Canal <mcanal@igalia.com>
>> Cc: Ludovico de Nittis <ludovico.denittis@collabora.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Simona Vetter <simona@ffwll.ch>
>> ---
>>   drivers/gpu/drm/drm_drv.c   |  10 +++
>>   drivers/gpu/drm/drm_sysfs.c | 123 ++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_device.h    |  19 ++++++
>>   include/drm/drm_drv.h       |   8 +++
>>   include/drm/drm_file.h      |   9 +++
>>   include/drm/drm_sysfs.h     |   4 ++
>>   6 files changed, 173 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 0cdc606af8d1..ab26cfd73002 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -54,6 +54,7 @@
>>   #include <drm/drm_print.h>
>>   #include <drm/drm_privacy_screen_machine.h>
>>   #include <drm/drm_ras_genl_family.h>
>> +#include <drm/drm_sysfs.h>
>>   #include "drm_crtc_internal.h"
>>   #include "drm_internal.h"
>> @@ -197,10 +198,15 @@ static int drm_minor_register(struct drm_device 
>> *dev, enum drm_minor_type type)
>>       if (ret)
>>           goto err_debugfs;
>> +    ret = drm_sysfs_register_memstat(minor);
>> +    if (ret)
>> +        goto err_kdev;
>> +
>>       /* replace NULL with @minor so lookups will succeed from now on */
>>       entry = xa_store(drm_minor_get_xa(type), minor->index, minor, 
>> GFP_KERNEL);
>>       if (xa_is_err(entry)) {
>>           ret = xa_err(entry);
>> +        drm_sysfs_unregister_memstat(minor);
>>           goto err_debugfs;
>>       }
>>       WARN_ON(entry);
>> @@ -208,6 +214,9 @@ static int drm_minor_register(struct drm_device 
>> *dev, enum drm_minor_type type)
>>       DRM_DEBUG("new minor registered %d\n", minor->index);
>>       return 0;
>> +err_kdev:
>> +    device_del(minor->kdev);
>> +
>>   err_debugfs:
>>       drm_debugfs_unregister(minor);
>>       return ret;
>> @@ -224,6 +233,7 @@ static void drm_minor_unregister(struct drm_device 
>> *dev, enum drm_minor_type typ
>>       /* replace @minor with NULL so lookups will fail from now on */
>>       xa_store(drm_minor_get_xa(type), minor->index, NULL, GFP_KERNEL);
>> +    drm_sysfs_unregister_memstat(minor);
>>       device_del(minor->kdev);
>>       dev_set_drvdata(minor->kdev, NULL); /* safety belt */
>>       drm_debugfs_unregister(minor);
>> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
>> index 7b16d32b6ff6..4a8c5b0b458d 100644
>> --- a/drivers/gpu/drm/drm_sysfs.c
>> +++ b/drivers/gpu/drm/drm_sysfs.c
>> @@ -25,6 +25,7 @@
>>   #include <drm/drm_accel.h>
>>   #include <drm/drm_connector.h>
>>   #include <drm/drm_device.h>
>> +#include <drm/drm_drv.h>
>>   #include <drm/drm_file.h>
>>   #include <drm/drm_modes.h>
>>   #include <drm/drm_print.h>
>> @@ -589,3 +590,125 @@ struct device *drm_sysfs_minor_alloc(struct 
>> drm_minor *minor)
>>       put_device(kdev);
>>       return ERR_PTR(r);
>>   }
>> +
>> +static ssize_t total_mb_show(struct kobject *kobj, struct 
>> kobj_attribute *attr,
>> +                 char *buf)
>> +{
>> +    struct drm_memstat_attr *mattr = container_of(kobj, 
>> typeof(*mattr), kobj);
>> +    struct drm_device *drm = mattr->drm;
>> +    const struct drm_memory_info *info;
>> +
>> +    if (drm->driver->get_memory_info) {
>> +        info = drm->driver->get_memory_info(drm);
>> +        if (info)
>> +            return sysfs_emit(buf, "%u\n",
>> +                      info->region[mattr->region].total_mb);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static ssize_t used_mb_show(struct kobject *kobj, struct 
>> kobj_attribute *attr,
>> +                char *buf)
>> +{
>> +    struct drm_memstat_attr *mattr = container_of(kobj, 
>> typeof(*mattr), kobj);
>> +    struct drm_device *drm = mattr->drm;
>> +    const struct drm_memory_info *info;
>> +
>> +    if (drm->driver->get_memory_info) {
>> +        info = drm->driver->get_memory_info(drm);
>> +        if (info)
>> +            return sysfs_emit(buf, "%u\n",
>> +                      info->region[mattr->region].used_mb);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static struct kobj_attribute total_attr = __ATTR_RO(total_mb);
>> +static struct kobj_attribute used_attr = __ATTR_RO(used_mb);
>> +
>> +static struct attribute *memstat_attrs[] = {
>> +    &total_attr.attr,
>> +    &used_attr.attr,
>> +    NULL,
>> +};
>> +
>> +ATTRIBUTE_GROUPS(memstat);
>> +
>> +static void kobj_memstat_release(struct kobject *kobj)
>> +{
>> +}
>> +
>> +static const struct kobj_type memstat_ktype = {
>> +    .release = kobj_memstat_release,
>> +    .sysfs_ops = &kobj_sysfs_ops,
>> +    .default_groups = memstat_groups,
>> +};
>> +
>> +int drm_sysfs_register_memstat(struct drm_minor *minor)
>> +{
>> +    const struct drm_memory_info *info = NULL;
>> +    struct drm_device *drm = minor->dev;
>> +    struct drm_memstat_attr *regions;
>> +    struct kobject *root;
>> +    int i, ret = -ENOMEM;
>> +
>> +    if (drm->driver->get_memory_info)
>> +        info = drm->driver->get_memory_info(drm);
>> +
>> +    if (!info || !info->num_regions)
>> +        return 0;
>> +
>> +    regions = kmalloc_objs(*regions, info->num_regions + 1);
>> +    if (!regions)
>> +        return -ENOMEM;
>> +
>> +    root = kobject_create_and_add("memstat", &minor->kdev->kobj);
>> +    if (!root)
>> +        goto err_root;
>> +
>> +    for (i = 0; i < info->num_regions; i++) {
>> +        regions[i].region = i;
>> +        regions[i].drm = drm;
>> +        ret = kobject_init_and_add(&regions[i].kobj, &memstat_ktype,
>> +                       root, "%s", info->region[i].name);
>> +        if (ret)
>> +            goto err_regions;
>> +    }
>> +
>> +    minor->sysfs_memstat = root;
>> +    minor->sysfs_memstat_regions = regions;
>> +
>> +    return 0;
>> +
>> +err_regions:
>> +    while (i-- > 0) {
>> +        kobject_del(&regions[i].kobj);
>> +        kobject_put(&regions[i].kobj);
>> +    }
>> +
>> +    kobject_del(root);
>> +    kobject_put(root);
>> +
>> +err_root:
>> +    kfree(regions);
>> +
>> +    return ret;
>> +}
>> +
>> +void drm_sysfs_unregister_memstat(struct drm_minor *minor)
>> +{
>> +    struct drm_memstat_attr *region = minor->sysfs_memstat_regions;
>> +
>> +    while (region && region->kobj.state_initialized) {
>> +        kobject_del(&region->kobj);
>> +        kobject_put(&region->kobj);
>> +        region++;
>> +    }
>> +
>> +    kfree(minor->sysfs_memstat_regions);
>> +
>> +    kobject_del(minor->sysfs_memstat);
>> +    kobject_put(minor->sysfs_memstat);
>> +}
>> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
>> index 75f030d027ee..0c9260c85b30 100644
>> --- a/include/drm/drm_device.h
>> +++ b/include/drm/drm_device.h
>> @@ -67,6 +67,25 @@ enum switch_power_state {
>>       DRM_SWITCH_POWER_DYNAMIC_OFF = 3,
>>   };
>> +/**
>> + * struct drm_memory_region_info - Memory name and stats for a region
>> + * @name: Name of the memory region
>> + * @total_mb: Total available memory in mebibytes
>> + * @used_mb: Used memory in mebibytes
>> + */
>> +struct drm_memory_region_info {
>> +    char    name[24];
>> +
>> +    u32    total_mb;
>> +    u32    used_mb;
>> +};
>> +
>> +struct drm_memory_info {
>> +    unsigned int num_regions;
>> +
>> +    struct drm_memory_region_info region[];
>> +};
>> +
>>   /**
>>    * struct drm_device - DRM device structure
>>    *
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index b23830494ed4..a3f1906961ac 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -45,6 +45,7 @@ struct drm_minor;
>>   struct dma_buf;
>>   struct dma_buf_attachment;
>>   struct drm_display_mode;
>> +struct drm_device_memory_info;
>>   struct drm_mode_create_dumb;
>>   struct drm_printer;
>>   struct sg_table;
>> @@ -387,6 +388,13 @@ struct drm_driver {
>>        */
>>       void (*show_fdinfo)(struct drm_printer *p, struct drm_file *f);
>> +    /**
>> +     * @get_memory_info:
>> +     *
>> +     * Get device specific memory info.  See Documentation/gpu/drm- 
>> memory-info.rst.
>> +     */
>> +    const struct drm_memory_info * (*get_memory_info)(struct 
>> drm_device *dev);
>> +
>>       /** @major: driver major number */
>>       int major;
>>       /** @minor: driver minor number */
>> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
>> index 6ee70ad65e1f..ebb227a91f60 100644
>> --- a/include/drm/drm_file.h
>> +++ b/include/drm/drm_file.h
>> @@ -66,6 +66,12 @@ enum drm_minor_type {
>>       DRM_MINOR_ACCEL = 32,
>>   };
>> +struct drm_memstat_attr {
>> +    struct kobject kobj;
>> +    unsigned int region;
>> +    struct drm_device *drm;
>> +};
>> +
>>   /**
>>    * struct drm_minor - DRM device minor structure
>>    *
>> @@ -82,6 +88,9 @@ struct drm_minor {
>>       struct device *kdev;        /* Linux device */
>>       struct drm_device *dev;
>> +    struct kobject    *sysfs_memstat;
>> +    struct drm_memstat_attr  *sysfs_memstat_regions;
>> +
>>       struct dentry *debugfs_symlink;
>>       struct dentry *debugfs_root;
>>   };
>> diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
>> index 7695873a9456..10ed468965b0 100644
>> --- a/include/drm/drm_sysfs.h
>> +++ b/include/drm/drm_sysfs.h
>> @@ -4,10 +4,14 @@
>>   struct drm_device;
>>   struct drm_connector;
>> +struct drm_minor;
>>   struct drm_property;
>>   void drm_sysfs_hotplug_event(struct drm_device *dev);
>>   void drm_sysfs_connector_hotplug_event(struct drm_connector 
>> *connector);
>>   void drm_sysfs_connector_property_event(struct drm_connector 
>> *connector,
>>                       struct drm_property *property);
>> +
>> +int drm_sysfs_register_memstat(struct drm_minor *minor);
>> +void drm_sysfs_unregister_memstat(struct drm_minor *minor);
>>   #endif
> 

[RFC 2/2] drm/amdgpu: Wire up DRM memory stats reporting

[Tvrtko Ursulin]

Wire up the driver agnostic DRM memory reporting for the VRAM and GTT
memory regions.

As the list of regions and their stats needs to be returned to the DRM
core for inspection and remain valid after the callback had exited, the
persistent storage is kept at the device level and updated on each query.

The current list of memory region names as reported for fdinfo memory
stats is exported as a helper in order to achieve standardized names, as
required by the DRM contract.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Maíra Canal <mcanal@igalia.com>
Cc: Ludovico de Nittis <ludovico.denittis@collabora.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  6 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 40 +++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 37 ++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  3 ++
 5 files changed, 75 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 79b69d74eb2e..8c00f76cfc7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -965,6 +965,9 @@ struct amdgpu_device {
 
 	struct amdgpu_uma_carveout_info uma_info;
 
+	struct drm_memory_info		memory_info;
+	struct drm_memory_region_info	__memory_region_info[2]; /* Storage for memory_info */
+
 	/* KFD
 	 * Must be last --ends in a flexible-array member.
 	 */
@@ -1350,4 +1353,7 @@ void amdgpu_device_set_uid(struct amdgpu_uid *uid_info,
 			   uint64_t uid);
 uint64_t amdgpu_device_get_uid(struct amdgpu_uid *uid_info,
 			       enum amdgpu_uid_type type, uint8_t inst);
+
+const struct drm_memory_info *amdgpu_drm_memory_info(struct drm_device *drm);
+
 #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 9c5e93cd3ee6..9830497b9ed2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -3142,7 +3142,7 @@ static const struct drm_driver amdgpu_kms_driver = {
 #ifdef CONFIG_PROC_FS
 	.show_fdinfo = amdgpu_show_fdinfo,
 #endif
-
+	.get_memory_info = amdgpu_drm_memory_info,
 	.gem_prime_import = amdgpu_gem_prime_import,
 
 	.name = DRIVER_NAME,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
index b349bb3676d5..3bbd053cd9f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
@@ -55,14 +55,9 @@ static const char *amdgpu_ip_name[AMDGPU_HW_IP_NUM] = {
 	[AMDGPU_HW_IP_VPE]	=	"vpe",
 };
 
-void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
+const char *amdgpu_ttm_pl_to_name(unsigned int placement)
 {
-	struct amdgpu_fpriv *fpriv = file->driver_priv;
-	struct amdgpu_vm *vm = &fpriv->vm;
-
-	struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM];
-	ktime_t usage[AMDGPU_HW_IP_NUM];
-	const char *pl_name[] = {
+	static const char *names[] = {
 		[TTM_PL_VRAM] = "vram",
 		[TTM_PL_TT] = "gtt",
 		[TTM_PL_SYSTEM] = "cpu",
@@ -72,6 +67,30 @@ void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
 		[AMDGPU_PL_DOORBELL] = "doorbell",
 		[AMDGPU_PL_MMIO_REMAP] = "mmioremap",
 	};
+
+	if (WARN_ON_ONCE(placement >= ARRAY_SIZE(names)))
+		return "unknown";
+
+	return names[placement];
+}
+
+void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
+{
+	struct amdgpu_fpriv *fpriv = file->driver_priv;
+	struct amdgpu_vm *vm = &fpriv->vm;
+
+	struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM];
+	ktime_t usage[AMDGPU_HW_IP_NUM];
+	static const unsigned int regions[] = {
+		TTM_PL_VRAM,
+		TTM_PL_TT,
+		TTM_PL_SYSTEM,
+		AMDGPU_PL_GDS,
+		AMDGPU_PL_GWS,
+		AMDGPU_PL_OA,
+		AMDGPU_PL_DOORBELL,
+		AMDGPU_PL_MMIO_REMAP,
+	};
 	unsigned int hw_ip, i;
 
 	amdgpu_vm_get_memory(vm, stats);
@@ -85,15 +104,12 @@ void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
 
 	drm_printf(p, "pasid:\t%u\n", fpriv->vm.pasid);
 
-	for (i = 0; i < ARRAY_SIZE(pl_name); i++) {
-		if (!pl_name[i])
-			continue;
-
+	for (i = 0; i < ARRAY_SIZE(regions); i++) {
 		drm_print_memory_stats(p,
 				       &stats[i].drm,
 				       DRM_GEM_OBJECT_RESIDENT |
 				       DRM_GEM_OBJECT_PURGEABLE,
-				       pl_name[i]);
+				       amdgpu_ttm_pl_to_name(regions[i]));
 	}
 
 	/* Legacy amdgpu keys, alias to drm-resident-memory-: */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index c509dc45ba19..2336ea1cd008 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -3027,3 +3027,40 @@ void amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
 
 #endif
 }
+
+const struct drm_memory_info *amdgpu_drm_memory_info(struct drm_device *drm)
+{
+	struct amdgpu_device *adev = drm_to_adev(drm);
+	struct drm_memory_info *info = &adev->memory_info;
+	static const unsigned int regions[] = {
+		TTM_PL_VRAM,
+		TTM_PL_TT,
+	};
+	struct drm_memory_region_info *region;
+	struct ttm_resource_manager *man;
+	unsigned int i;
+
+	if (WARN_ON_ONCE(ARRAY_SIZE(adev->__memory_region_info) !=
+			 ARRAY_SIZE(regions)))
+		return NULL;
+
+	if (!info->num_regions) {
+		for (i = 0; i < ARRAY_SIZE(regions); i++) {
+			region = &info->region[i];
+			strscpy(region->name,
+				amdgpu_ttm_pl_to_name(regions[i]),
+				sizeof(region->name));
+			man = ttm_manager_type(&adev->mman.bdev, regions[i]);
+			region->total_mb = man->size >> 20;
+			info->num_regions++;
+		}
+	}
+
+	for (i = 0; i < ARRAY_SIZE(regions); i++) {
+		region = &info->region[i];
+		man = ttm_manager_type(&adev->mman.bdev, regions[i]);
+		region->used_mb = ttm_resource_manager_usage(man) >> 20;
+	}
+
+	return &adev->memory_info;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index adb6cfae5052..1ed07fb4a755 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -271,4 +271,7 @@ void amdgpu_ttm_mmio_remap_free_sgt(struct device *dev,
 				    enum dma_data_direction dir,
 				    struct sg_table *sgt);
 
+const char *amdgpu_ttm_pl_to_name(unsigned int placement);
+const struct drm_memory_info *amdgpu_drm_memory_info(struct drm_device *drm);
+
 #endif
-- 
2.55.0

Re: [RFC 2/2] drm/amdgpu: Wire up DRM memory stats reporting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] amdgpu: data race and OOB read in amdgpu_drm_memory_info lazy init
- [High] amdgpu: embedded flexible array member causes UB and FORTIFY panics
- [Medium] amdgpu: array index mismatch in fdinfo mixes up memory stats
- [Medium] amdgpu: unprotected concurrent updates to region->used_mb cause data race
--

commit 0821285be07eaef79323cee5db70d046b94e519f
Author: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
drm/amdgpu: Wire up DRM memory stats reporting

This commit wires up the driver agnostic DRM memory reporting for the VRAM
and GTT memory regions. It keeps the persistent storage at the device level
and exports a helper for standardized region names.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 79b69d74eb2e5..8c00f76cfc7b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -965,6 +965,9 @@ struct amdgpu_device {
>  
>  	struct amdgpu_uma_carveout_info uma_info;
>  
> +	struct drm_memory_info		memory_info;
> +	struct drm_memory_region_info	__memory_region_info[2]; /* Storage for memory_info */
> +

[Severity: High]
Does embedding a structure with a flexible array member in the middle
of another struct cause undefined behavior and compiler warnings?

struct drm_memory_info ends with a flexible array member (region[]).
Embedding it here before __memory_region_info[2] means the compiler
evaluates the size of memory_info as only its fixed members (4 bytes).

When writing to info->region[i] later, FORTIFY/UBSAN checks may panic
as the access exceeds the recognized object bounds.

[ ... ]

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> index b349bb3676d5f..3bbd053cd9f7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
[ ... ]
> @@ -85,15 +104,12 @@ void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
>  
>  	drm_printf(p, "pasid:\t%u\n", fpriv->vm.pasid);
>  
> -	for (i = 0; i < ARRAY_SIZE(pl_name); i++) {
> -		if (!pl_name[i])
> -			continue;
> -
> +	for (i = 0; i < ARRAY_SIZE(regions); i++) {
>  		drm_print_memory_stats(p,
> -				       &stats[i].drm,
> +				       &stats[i].drm,
>  				       DRM_GEM_OBJECT_RESIDENT |
>  				       DRM_GEM_OBJECT_PURGEABLE,
> -				       pl_name[i]);
> +				       amdgpu_ttm_pl_to_name(regions[i]));
>  	}

[Severity: Medium]
Is there an array index mismatch here?

The stats array is populated by amdgpu_vm_get_memory() which maps
placement IDs directly to indices (e.g., TTM_PL_SYSTEM is 0,
TTM_PL_VRAM is 2).

However, the loop iterates using a sequential index i (0 to 7) and
fetches &stats[i].drm, but labels it with amdgpu_ttm_pl_to_name(regions[i]).

For i=0, regions[0] is TTM_PL_VRAM (2), meaning it fetches stats[0]
(CPU memory) but labels it as VRAM.

Should the fetch use &stats[regions[i]].drm to correctly match the data
with the placement ID?

[ ... ]

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 0ca9b1669e0fe..d6da850eadc97 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -3026,3 +3026,40 @@ void amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
>  
>  #endif
>  }
> +
> +const struct drm_memory_info *amdgpu_drm_memory_info(struct drm_device *drm)
> +{
> +	struct amdgpu_device *adev = drm_to_adev(drm);
> +	struct drm_memory_info *info = &adev->memory_info;
[ ... ]
> +	if (!info->num_regions) {
> +		for (i = 0; i < ARRAY_SIZE(regions); i++) {
> +			region = &info->region[i];
> +			strscpy(region->name,
> +				amdgpu_ttm_pl_to_name(regions[i]),
> +				sizeof(region->name));
> +			man = ttm_manager_type(&adev->mman.bdev, regions[i]);
> +			region->total_mb = man->size >> 20;
> +			info->num_regions++;
> +		}
> +	}

[Severity: High]
Is this lockless lazy initialization safe against concurrent access?

If multiple threads query DRM memory info concurrently via fdinfo, they
can simultaneously enter the if (!info->num_regions) block. Both threads
could execute info->num_regions++ in a loop.

Since the backing array (__memory_region_info) only has 2 elements,
concurrent increments can push num_regions to 4. When the DRM core later
iterates up to num_regions, will this lead to out-of-bounds memory reads?

> +
> +	for (i = 0; i < ARRAY_SIZE(regions); i++) {
> +		region = &info->region[i];
> +		man = ttm_manager_type(&adev->mman.bdev, regions[i]);
> +		region->used_mb = ttm_resource_manager_usage(man) >> 20;
> +	}
> +
> +	return &adev->memory_info;
> +}

[Severity: Medium]
Can unprotected concurrent updates to region->used_mb cause data races?

Because amdgpu_drm_memory_info() updates region->used_mb directly within
the globally shared adev->memory_info structure without synchronization,
concurrent readers and writers might race, leading to torn reads or
KCSAN splats.

Would it be safer to allocate a drm_memory_info struct per-query instead
of caching it globally?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917120444.86471-1-tvrtko.ursulin@igalia.com?part=2

Re: [RFC 0/2] DRM standardized memory stats

[Christian König]

On 9/17/26 14:04, Tvrtko Ursulin wrote:
> Re-floating a simple idea (this time with more Cc) born from an user interest to
> be able to correctly size the swap space during OS installation, based on the
> size of the VRAM a discrete GPU might have. And to be able to do it in a vendor
> agnostic way.
> 
> Idea is to expose a standardized scheme in sysfs, under the DRM class card, and
> under a new 'memstat' directory. Such as, example from amdgpu:
> 
> /sys/class/drm/card1/memstat/
> ├── gtt
> │   ├── total_mb
> │   └── used_mb
> └── vram
>     ├── total_mb
>     └── used_mb
> 
> Or with concrete numbers:
> 
> $ grep -Hr . /sys/class/drm/card1/memstat/
> /sys/class/drm/card1/memstat/vram/total_mb:1024
> /sys/class/drm/card1/memstat/vram/used_mb:445
> /sys/class/drm/card1/memstat/gtt/total_mb:7394
> /sys/class/drm/card1/memstat/gtt/used_mb:71
> 
> Drivers need to implement a simple DRM driver level callback which needs to
> report a stable list of interesting memory regions and their respective stats.
> The region names then become sub-directory names under the new 'memstat'
> directory, with each region exposing the total size and the current usage.
> 
> Similar data can already be queried if the dmem cgroup controller is enabled,
> also only for the participating drivers, by querying the root cgroup. But
> perhaps sysfs is easier, or perhaps it is too much code for too little benefit.
> I am curious to hear any opinions.

The implementation can be polished a bit but I seriously like the idea to standardize that.

amdgpu already exposes the information as non-standard sysfs files and adds a bit more, e.g. CPU visible VRAM size, VRAM vendor etc...

I'm wondering if those shouldn't be added as well and the existing sysfs files then implemented as symlinks.

Regards,
Christian.

> 
> Cc: Maíra Canal <mcanal@igalia.com>
> Cc: Ludovico de Nittis <ludovico.denittis@collabora.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> 
> Tvrtko Ursulin (2):
>   drm: Allow drivers to report standardized memory stats
>   drm/amdgpu: Wire up DRM memory stats reporting
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |   6 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c |  40 +++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  37 +++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |   3 +
>  drivers/gpu/drm/drm_drv.c                  |  10 ++
>  drivers/gpu/drm/drm_sysfs.c                | 123 +++++++++++++++++++++
>  include/drm/drm_device.h                   |  19 ++++
>  include/drm/drm_drv.h                      |   8 ++
>  include/drm/drm_file.h                     |   9 ++
>  include/drm/drm_sysfs.h                    |   4 +
>  11 files changed, 248 insertions(+), 13 deletions(-)
> 

Re: [RFC 0/2] DRM standardized memory stats

[Tvrtko Ursulin]

On 21/09/2026 10:19, Christian König wrote:
> On 9/17/26 14:04, Tvrtko Ursulin wrote:
>> Re-floating a simple idea (this time with more Cc) born from an user interest to
>> be able to correctly size the swap space during OS installation, based on the
>> size of the VRAM a discrete GPU might have. And to be able to do it in a vendor
>> agnostic way.
>>
>> Idea is to expose a standardized scheme in sysfs, under the DRM class card, and
>> under a new 'memstat' directory. Such as, example from amdgpu:
>>
>> /sys/class/drm/card1/memstat/
>> ├── gtt
>> │   ├── total_mb
>> │   └── used_mb
>> └── vram
>>      ├── total_mb
>>      └── used_mb
>>
>> Or with concrete numbers:
>>
>> $ grep -Hr . /sys/class/drm/card1/memstat/
>> /sys/class/drm/card1/memstat/vram/total_mb:1024
>> /sys/class/drm/card1/memstat/vram/used_mb:445
>> /sys/class/drm/card1/memstat/gtt/total_mb:7394
>> /sys/class/drm/card1/memstat/gtt/used_mb:71
>>
>> Drivers need to implement a simple DRM driver level callback which needs to
>> report a stable list of interesting memory regions and their respective stats.
>> The region names then become sub-directory names under the new 'memstat'
>> directory, with each region exposing the total size and the current usage.
>>
>> Similar data can already be queried if the dmem cgroup controller is enabled,
>> also only for the participating drivers, by querying the root cgroup. But
>> perhaps sysfs is easier, or perhaps it is too much code for too little benefit.
>> I am curious to hear any opinions.
> 
> The implementation can be polished a bit but I seriously like the idea to standardize that.

Indeed. I think I've fixed all the kobject related flaring bugs locally.

> amdgpu already exposes the information as non-standard sysfs files and adds a bit more, e.g. CPU visible VRAM size, VRAM vendor etc...
> 
> I'm wondering if those shouldn't be added as well and the existing sysfs files then implemented as symlinks.

CPU visible I guess could be added with a small addition to the 2nd 
(amdgpu) patch. So far the implementation is driven trivially from TTM 
placements / range managers, but I could easily add a different stat 
since drivers define what they export.

Existing files as symlinks I do not know who I would implement it. File 
to file is not possible AFAIU, right?

Allowing drivers to put custom files in there might be complicated and 
could create a standardization problem. If vendor is strongly desired I 
could add it as DRM owned attribute perhaps. Like 
/sys/class/drm/card1/memstat/vram/hw_vendor or something.

Regards,

Tvrtko

>> Cc: Maíra Canal <mcanal@igalia.com>
>> Cc: Ludovico de Nittis <ludovico.denittis@collabora.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Maxime Ripard <mripard@kernel.org>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Simona Vetter <simona@ffwll.ch>
>>
>> Tvrtko Ursulin (2):
>>    drm: Allow drivers to report standardized memory stats
>>    drm/amdgpu: Wire up DRM memory stats reporting
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |   6 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |   2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c |  40 +++++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  37 +++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |   3 +
>>   drivers/gpu/drm/drm_drv.c                  |  10 ++
>>   drivers/gpu/drm/drm_sysfs.c                | 123 +++++++++++++++++++++
>>   include/drm/drm_device.h                   |  19 ++++
>>   include/drm/drm_drv.h                      |   8 ++
>>   include/drm/drm_file.h                     |   9 ++
>>   include/drm/drm_sysfs.h                    |   4 +
>>   11 files changed, 248 insertions(+), 13 deletions(-)
>>
> 

[RFC 0/2] DRM standardized memory stats

[Tvrtko Ursulin]

Floating a simple idea born from an user interest to be able to correctly size
the swap space during OS installation, based on the size of the VRAM a discrete
GPU might have. And to be able to do it in a vendor agnostic way.

Idea is to expose a standardized scheme in sysfs, under the DRM class card, and
under a new 'memstat' directory. Such as, example from amdgpu:

/sys/class/drm/card1/memstat/
├── gtt
│   ├── total_mb
│   └── used_mb
└── vram
    ├── total_mb
    └── used_mb

Or with concrete numbers:

$ grep -Hr . /sys/class/drm/card1/memstat/
/sys/class/drm/card1/memstat/vram/total_mb:1024
/sys/class/drm/card1/memstat/vram/used_mb:445
/sys/class/drm/card1/memstat/gtt/total_mb:7394
/sys/class/drm/card1/memstat/gtt/used_mb:71

Drivers need to implement a simple DRM driver level callback which needs to
report a stable list of interesting memory regions and their respective stats.
The region names then become sub-directory names under the new 'memstat'
directory, with each region exposing the total size and the current usage.

Similar data can already be queried if the dmem cgroup controller is enabled,
also only for the participating drivers, by querying the root cgroup. But
perhaps sysfs is easier, or perhaps it is too much code for too little benefit.
I am curious to hear any opinions.

Tvrtko Ursulin (2):
  drm: Allow drivers to report standardized memory stats
  drm/amdgpu: Wire up DRM memory stats reporting

 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |   6 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c |  40 +++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  37 +++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |   3 +
 drivers/gpu/drm/drm_drv.c                  |  10 ++
 drivers/gpu/drm/drm_sysfs.c                | 123 +++++++++++++++++++++
 include/drm/drm_device.h                   |  19 ++++
 include/drm/drm_drv.h                      |   8 ++
 include/drm/drm_file.h                     |   9 ++
 include/drm/drm_sysfs.h                    |   4 +
 11 files changed, 248 insertions(+), 13 deletions(-)

-- 
2.52.0

[RFC 1/2] drm: Allow drivers to report standardized memory stats

[Tvrtko Ursulin]

Add a DRM driver level vfunc which allows drivers to opt-in into
reporting standardized memory stats via sysfs.

Via the vfunc drivers can report a list of memory regions in a stable
order, with the names following the memory region names used for fdinfo
memory stats. The region names become directory names under the parent
'memstat' directory.

Each region reports the total size of the region and its current usage,
represented respectively in the total_mb and used_mb files present under
the aforementioned memory region named directories.

The card level 'memstat' directory is only created for drivers who report
at least one memory region.

Amdgpu example:

/sys/class/drm/card1/memstat/
├── gtt
│   ├── total_mb
│   └── used_mb
└── vram
    ├── total_mb
    └── used_mb

Or with concrete numbers:

$ grep -Hr . /sys/class/drm/card1/memstat/
/sys/class/drm/card1/memstat/vram/total_mb:1024
/sys/class/drm/card1/memstat/vram/used_mb:445
/sys/class/drm/card1/memstat/gtt/total_mb:7394
/sys/class/drm/card1/memstat/gtt/used_mb:71

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
---
 drivers/gpu/drm/drm_drv.c   |  10 +++
 drivers/gpu/drm/drm_sysfs.c | 123 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_device.h    |  19 ++++++
 include/drm/drm_drv.h       |   8 +++
 include/drm/drm_file.h      |   9 +++
 include/drm/drm_sysfs.h     |   4 ++
 6 files changed, 173 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 985c283cf59f..9b350775f132 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -54,6 +54,7 @@
 #include <drm/drm_print.h>
 #include <drm/drm_privacy_screen_machine.h>
 #include <drm/drm_ras_genl_family.h>
+#include <drm/drm_sysfs.h>
 
 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -196,10 +197,15 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
 	if (ret)
 		goto err_debugfs;
 
+	ret = drm_sysfs_register_memstat(minor);
+	if (ret)
+		goto err_kdev;
+
 	/* replace NULL with @minor so lookups will succeed from now on */
 	entry = xa_store(drm_minor_get_xa(type), minor->index, minor, GFP_KERNEL);
 	if (xa_is_err(entry)) {
 		ret = xa_err(entry);
+		drm_sysfs_unregister_memstat(minor);
 		goto err_debugfs;
 	}
 	WARN_ON(entry);
@@ -207,6 +213,9 @@ static int drm_minor_register(struct drm_device *dev, enum drm_minor_type type)
 	DRM_DEBUG("new minor registered %d\n", minor->index);
 	return 0;
 
+err_kdev:
+	device_del(minor->kdev);
+
 err_debugfs:
 	drm_debugfs_unregister(minor);
 	return ret;
@@ -223,6 +232,7 @@ static void drm_minor_unregister(struct drm_device *dev, enum drm_minor_type typ
 	/* replace @minor with NULL so lookups will fail from now on */
 	xa_store(drm_minor_get_xa(type), minor->index, NULL, GFP_KERNEL);
 
+	drm_sysfs_unregister_memstat(minor);
 	device_del(minor->kdev);
 	dev_set_drvdata(minor->kdev, NULL); /* safety belt */
 	drm_debugfs_unregister(minor);
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ef4e923a8728..a7217d2280d3 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -25,6 +25,7 @@
 #include <drm/drm_accel.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_device.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_file.h>
 #include <drm/drm_modes.h>
 #include <drm/drm_print.h>
@@ -621,3 +622,125 @@ void drm_class_device_unregister(struct device *dev)
 	return device_unregister(dev);
 }
 EXPORT_SYMBOL_GPL(drm_class_device_unregister);
+
+static ssize_t total_mb_show(struct kobject *kobj, struct kobj_attribute *attr,
+			     char *buf)
+{
+	struct drm_memstat_attr *mattr = container_of(kobj, typeof(*mattr), kobj);
+	struct drm_device *drm = mattr->drm;
+	const struct drm_memory_info *info;
+
+	if (drm->driver->get_memory_info) {
+		info = drm->driver->get_memory_info(drm);
+		if (info)
+			return sysfs_emit(buf, "%u\n",
+					  info->region[mattr->region].total_mb);
+	}
+
+	return 0;
+}
+
+static ssize_t used_mb_show(struct kobject *kobj, struct kobj_attribute *attr,
+			    char *buf)
+{
+	struct drm_memstat_attr *mattr = container_of(kobj, typeof(*mattr), kobj);
+	struct drm_device *drm = mattr->drm;
+	const struct drm_memory_info *info;
+
+	if (drm->driver->get_memory_info) {
+		info = drm->driver->get_memory_info(drm);
+		if (info)
+			return sysfs_emit(buf, "%u\n",
+					  info->region[mattr->region].used_mb);
+	}
+
+	return 0;
+}
+
+static struct kobj_attribute total_attr = __ATTR_RO(total_mb);
+static struct kobj_attribute used_attr = __ATTR_RO(used_mb);
+
+static struct attribute *memstat_attrs[] = {
+	&total_attr.attr,
+	&used_attr.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(memstat);
+
+static void kobj_memstat_release(struct kobject *kobj)
+{
+}
+
+static const struct kobj_type memstat_ktype = {
+	.release = kobj_memstat_release,
+	.sysfs_ops = &kobj_sysfs_ops,
+	.default_groups = memstat_groups,
+};
+
+int drm_sysfs_register_memstat(struct drm_minor *minor)
+{
+	const struct drm_memory_info *info = NULL;
+	struct drm_device *drm = minor->dev;
+	struct drm_memstat_attr *regions;
+	struct kobject *root;
+	int i, ret = -ENOMEM;
+
+	if (drm->driver->get_memory_info)
+		info = drm->driver->get_memory_info(drm);
+
+	if (!info || !info->num_regions)
+		return 0;
+
+	regions = kmalloc_objs(*regions, info->num_regions + 1);
+	if (!regions)
+		return -ENOMEM;
+
+	root = kobject_create_and_add("memstat", &minor->kdev->kobj);
+	if (!root)
+		goto err_root;
+
+	for (i = 0; i < info->num_regions; i++) {
+		regions[i].region = i;
+		regions[i].drm = drm;
+		ret = kobject_init_and_add(&regions[i].kobj, &memstat_ktype,
+					   root, "%s", info->region[i].name);
+		if (ret)
+			goto err_regions;
+	}
+
+	minor->sysfs_memstat = root;
+	minor->sysfs_memstat_regions = regions;
+
+	return 0;
+
+err_regions:
+	while (i-- > 0) {
+		kobject_del(&regions[i].kobj);
+		kobject_put(&regions[i].kobj);
+	}
+
+	kobject_del(root);
+	kobject_put(root);
+
+err_root:
+	kfree(regions);
+
+	return ret;
+}
+
+void drm_sysfs_unregister_memstat(struct drm_minor *minor)
+{
+	struct drm_memstat_attr *region = minor->sysfs_memstat_regions;
+
+	while (region && region->kobj.state_initialized) {
+		kobject_del(&region->kobj);
+		kobject_put(&region->kobj);
+		region++;
+	}
+
+	kfree(minor->sysfs_memstat_regions);
+
+	kobject_del(minor->sysfs_memstat);
+	kobject_put(minor->sysfs_memstat);
+}
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index bc78fb77cc27..8c4f1fb436af 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -66,6 +66,25 @@ enum switch_power_state {
 	DRM_SWITCH_POWER_DYNAMIC_OFF = 3,
 };
 
+/**
+ * struct drm_memory_region_info - Memory name and stats for a region
+ * @name: Name of the memory region
+ * @total_mb: Total available memory in mebibytes
+ * @used_mb: Used memory in mebibytes
+ */
+struct drm_memory_region_info {
+	char	name[24];
+
+	u32	total_mb;
+	u32	used_mb;
+};
+
+struct drm_memory_info {
+	unsigned int num_regions;
+
+	struct drm_memory_region_info region[];
+};
+
 /**
  * struct drm_device - DRM device structure
  *
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 42fc085f986d..431d2e8e641a 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -44,6 +44,7 @@ struct drm_minor;
 struct dma_buf;
 struct dma_buf_attachment;
 struct drm_display_mode;
+struct drm_device_memory_info;
 struct drm_mode_create_dumb;
 struct drm_printer;
 struct sg_table;
@@ -392,6 +393,13 @@ struct drm_driver {
 	 */
 	void (*show_fdinfo)(struct drm_printer *p, struct drm_file *f);
 
+	/**
+	 * @get_memory_info:
+	 *
+	 * Get device specific memory info.  See Documentation/gpu/drm-memory-info.rst.
+	 */
+	const struct drm_memory_info * (*get_memory_info)(struct drm_device *dev);
+
 	/** @major: driver major number */
 	int major;
 	/** @minor: driver minor number */
diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 6ee70ad65e1f..ebb227a91f60 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -66,6 +66,12 @@ enum drm_minor_type {
 	DRM_MINOR_ACCEL = 32,
 };
 
+struct drm_memstat_attr {
+	struct kobject kobj;
+	unsigned int region;
+	struct drm_device *drm;
+};
+
 /**
  * struct drm_minor - DRM device minor structure
  *
@@ -82,6 +88,9 @@ struct drm_minor {
 	struct device *kdev;		/* Linux device */
 	struct drm_device *dev;
 
+	struct kobject	*sysfs_memstat;
+	struct drm_memstat_attr  *sysfs_memstat_regions;
+
 	struct dentry *debugfs_symlink;
 	struct dentry *debugfs_root;
 };
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index 96a5d858404b..dec9d7ecdec0 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -5,6 +5,7 @@
 struct drm_device;
 struct device;
 struct drm_connector;
+struct drm_minor;
 struct drm_property;
 
 int drm_class_device_register(struct device *dev);
@@ -14,4 +15,7 @@ void drm_sysfs_hotplug_event(struct drm_device *dev);
 void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
 void drm_sysfs_connector_property_event(struct drm_connector *connector,
 					struct drm_property *property);
+
+int drm_sysfs_register_memstat(struct drm_minor *minor);
+void drm_sysfs_unregister_memstat(struct drm_minor *minor);
 #endif
-- 
2.52.0