On 20/10/2025 11:10, Tvrtko Ursulin wrote:
On 15/10/2025 16:30, Loïc Molinari wrote:
Add the drm_gem_huge_mnt_create() helper to avoid code duplication in
the i915, V3D, Panfrost and Panthor drivers. It creates and mounts a
dedicated huge tmpfs mountpoint, for the lifetime of a DRM device,
used at GEM object initialization.
The next commits will port drivers to this helper.
v3:
- store huge tmpfs mountpoint in drm_device
v4:
- return 0 in builds with CONFIG_TRANSPARENT_HUGEPAGE=n
- return 0 when huge_mnt already exists
Signed-off-by: Loïc Molinari <[email protected]>
---
drivers/gpu/drm/drm_gem.c | 58 +++++++++++++++++++++++++++++++++++++++
include/drm/drm_device.h | 11 ++++++++
include/drm/drm_gem.h | 1 +
3 files changed, 70 insertions(+)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index a98d5744cc6c..db8c0a217add 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -29,6 +29,7 @@
#include <linux/export.h>
#include <linux/file.h>
#include <linux/fs.h>
+#include <linux/fs_context.h>
#include <linux/iosys-map.h>
#include <linux/mem_encrypt.h>
#include <linux/mm.h>
@@ -82,6 +83,63 @@
* up at a later date, and as our interface with shmfs for memory
allocation.
*/
+static void drm_gem_huge_mnt_free(struct drm_device *dev, void *data)
+{
+ drm_WARN_ON(dev, dev->huge_mnt == NULL);
I don't see a benefit of adding this check but maybe I am missing
something.
That was mostly to detect and warn drivers setting the drm_device's
huge_mnt pointer directly. I can remove that.
+
+ kern_unmount(dev->huge_mnt);
+ dev->huge_mnt = NULL;
Ditto - device is going away, no? So why bother clearing the pointer?
This one is necessary to let drivers tear down and reload.
drm_gem_huge_mnt_create() returns if the pointer isn't NULL.
Also, is the compiler smart enough to not compile or complain this
function is unused in the !CONFIG_TRANSPARENT_HUGEPAGE case?
No compiler warnings, but this might not be the case with different
compilers/versions so I'll ifdef it out.
+}
+
+/**
+ * drm_gem_huge_mnt_create - Create, mount and use a huge tmpfs
mountpoint
+ * @dev: drm_device a huge tmpfs mountpoint should be used with
+ * @value: huge tmpfs mount option value
+ *
+ * This function creates and mounts a dedicated huge tmpfs mountpoint
for the
+ * lifetime of the drm device @dev which is used at GEM object
initialization
+ * with drm_gem_object_init().
+ *
+ * The most common option value @value is "within_size" which only
allocates
+ * huge pages if the page will be fully within the GEM object size.
"always",
+ * "advise" and "never" are supported too but the latter would just
create a
+ * mountpoint similar to the default one (`shm_mnt`). See shmemfs and
+ * Transparent Hugepage for more information.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_huge_mnt_create(struct drm_device *dev, const char *value)
+{
+ struct file_system_type *type;
+ struct fs_context *fc;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
+ return 0;
Is there a specific reason why the !CONFIG_TRANSPARENT_HUGEPAGE path is
not implemented in the header as a static inline? That would enable
those builds to avoid the pointless function in text and function call
in the drivers.
Good point. I'll propose a new version with drm_gem_huge_mnt_create()
implemented as a static inline function that calls into
__drm_gem_huge_mnt_create() only for builds with
CONFIG_TRANSPARENT_HUGEPAGE=y.
+ if (unlikely(dev->huge_mnt))
+ return 0;
Any special reason why it is allowed to call it multiple times with
success?
That was initially returning -EEXIST in v3 but got changed after review
to simplify call sites.
+
+ type = get_fs_type("tmpfs");
+ if (unlikely(!type))
+ return -EOPNOTSUPP;
+ fc = fs_context_for_mount(type, SB_KERNMOUNT);
+ if (IS_ERR(fc))
+ return PTR_ERR(fc);
+ ret = vfs_parse_fs_string(fc, "source", "tmpfs");
+ if (unlikely(ret))
+ return -ENOPARAM;
+ ret = vfs_parse_fs_string(fc, "huge", value);
+ if (unlikely(ret))
+ return -ENOPARAM;
+
+ dev->huge_mnt = fc_mount_longterm(fc);
+ put_fs_context(fc);
+
+ return drmm_add_action_or_reset(dev, drm_gem_huge_mnt_free, NULL);
+}
+EXPORT_SYMBOL_GPL(drm_gem_huge_mnt_create);
+
static void
drm_gem_init_release(struct drm_device *dev, void *ptr)
{
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 778b2cca6c49..352e3db402d7 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -3,6 +3,7 @@
#include <linux/list.h>
#include <linux/kref.h>
+#include <linux/mount.h>
#include <linux/mutex.h>
#include <linux/idr.h>
#include <linux/sched.h>
@@ -168,6 +169,16 @@ struct drm_device {
*/
struct drm_master *master;
+ /**
+ * @huge_mnt:
+ *
+ * Huge tmpfs mountpoint used at GEM object initialization
+ * drm_gem_object_init(). Drivers can call
drm_gem_huge_mnt_create() to
+ * create a huge tmfps mountpoint. The default tmpfs mountpoint
+ * (`shm_mnt`) is used if NULL.
+ */
+ struct vfsmount *huge_mnt;
Maybe it would be nice to hide this in the !CONFIG_TRANSPARENT_HUGEPAGE
case? A bit ugly to add an ifdef but it is also a bit questionable to
force the member on everyone.
It was initially stored in drivers' data structures but, as mentioned
above for v3, got put in drm_device to simplify call sites.
Both V3D and i915 are testing for that pointer in a few places and that
would require adding ifdefs there too. This would also be the same for
any drivers adding support for huge pages. Is that really worth it?
Regards,
Tvrtko
+
/**
* @driver_features: per-device driver features
*
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 7c8bd67d087c..7285a62d9afc 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -492,6 +492,7 @@ struct drm_gem_object {
DRM_GEM_FOPS,\
}
+int drm_gem_huge_mnt_create(struct drm_device *dev, const char *value);
void drm_gem_object_release(struct drm_gem_object *obj);
void drm_gem_object_free(struct kref *kref);
int drm_gem_object_init(struct drm_device *dev,
