On 28/11/2025 18:41, Loïc Molinari wrote:
On 20/11/2025 10:31, Tvrtko Ursulin wrote:

On 14/11/2025 17:02, Loïc Molinari wrote:
Make use of the new drm_gem_huge_mnt_create() and
drm_gem_get_huge_mnt() helpers to avoid code duplication. Now that
it's just a few lines long, the single function in i915_gemfs.c is
moved into v3d_gem_shmem.c.

v3:
- use huge tmpfs mountpoint in drm_device
- move i915_gemfs.c into i915_gem_shmem.c

v4:
- clean up mountpoint creation error handling

v5:
- use drm_gem_has_huge_mnt() helper

v7:
- include <drm/drm_print.h> in i915_gem_shmem.c

v8:
- keep logging notice message with CONFIG_TRANSPARENT_HUGEPAGE=n
- don't access huge_mnt field with CONFIG_TRANSPARENT_HUGEPAGE=n

v9:
- replace drm_gem_has_huge_mnt() by drm_gem_get_huge_mnt()
- remove useless ternary op test in selftests/huge_pages.c

Signed-off-by: Loïc Molinari <[email protected]>
---
  drivers/gpu/drm/i915/Makefile                 |  3 +-
  drivers/gpu/drm/i915/gem/i915_gem_shmem.c     | 48 +++++++++----
  drivers/gpu/drm/i915/gem/i915_gemfs.c         | 71 -------------------
  drivers/gpu/drm/i915/gem/i915_gemfs.h         | 14 ----
  .../gpu/drm/i915/gem/selftests/huge_pages.c   | 16 +++--
  drivers/gpu/drm/i915/i915_drv.h               |  5 --
  6 files changed, 47 insertions(+), 110 deletions(-)
  delete mode 100644 drivers/gpu/drm/i915/gem/i915_gemfs.c
  delete mode 100644 drivers/gpu/drm/i915/gem/i915_gemfs.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/ Makefile
index 84ec79b64960..b5a8c0a6b747 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -169,8 +169,7 @@ gem-y += \
      gem/i915_gem_ttm_move.o \
      gem/i915_gem_ttm_pm.o \
      gem/i915_gem_userptr.o \
-    gem/i915_gem_wait.o \
-    gem/i915_gemfs.o
+    gem/i915_gem_wait.o
  i915-y += \
      $(gem-y) \
      i915_active.o \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/ drm/i915/gem/i915_gem_shmem.c
index 26dda55a07ff..15c2c6fde2ac 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -9,14 +9,16 @@
  #include <linux/uio.h>
  #include <drm/drm_cache.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_print.h>
  #include "gem/i915_gem_region.h"
  #include "i915_drv.h"
  #include "i915_gem_object.h"
  #include "i915_gem_tiling.h"
-#include "i915_gemfs.h"
  #include "i915_scatterlist.h"
  #include "i915_trace.h"
+#include "i915_utils.h"
  /*
   * Move folios to appropriate lru and release the batch, decrementing the @@ -497,6 +499,7 @@ static int __create_shmem(struct drm_i915_private *i915,
                resource_size_t size)
  {
      unsigned long flags = VM_NORESERVE;
+    struct vfsmount *huge_mnt;
      struct file *filp;
      drm_gem_private_object_init(&i915->drm, obj, size);
@@ -515,9 +518,9 @@ static int __create_shmem(struct drm_i915_private *i915,
      if (BITS_PER_LONG == 64 && size > MAX_LFS_FILESIZE)
          return -E2BIG;
-    if (i915->mm.gemfs)
-        filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size,
-                         flags);
+    huge_mnt = drm_gem_get_huge_mnt(&i915->drm);
+    if (huge_mnt)
+        filp = shmem_file_setup_with_mnt(huge_mnt, "i915", size, flags);
      else
          filp = shmem_file_setup("i915", size, flags);
      if (IS_ERR(filp))
@@ -644,21 +647,40 @@ i915_gem_object_create_shmem_from_data(struct drm_i915_private *i915,
  static int init_shmem(struct intel_memory_region *mem)
  {
-    i915_gemfs_init(mem->i915);
-    intel_memory_region_set_name(mem, "system");
+    struct drm_i915_private *i915 = mem->i915;
-    return 0; /* We have fallback to the kernel mnt if gemfs init failed. */
-}
+    /*
+     * By creating our own shmemfs mountpoint, we can pass in
+     * mount flags that better match our usecase.
+     *
+     * One example, although it is probably better with a per-file
+     * control, is selecting huge page allocations ("huge=within_size").
+     * However, we only do so on platforms which benefit from it, or to
+     * offset the overhead of iommu lookups, where with latter it is a net
+     * win even on platforms which would otherwise see some performance
+     * regressions such a slow reads issue on Broadwell and Skylake.
+     */
-static int release_shmem(struct intel_memory_region *mem)
-{
-    i915_gemfs_fini(mem->i915);
-    return 0;
+    if (GRAPHICS_VER(i915) < 11 && !i915_vtd_active(i915))
+        goto no_thp;
+
+    drm_gem_huge_mnt_create(&i915->drm, "within_size");
+    if (drm_gem_get_huge_mnt(&i915->drm))
+        drm_info(&i915->drm, "Using Transparent Hugepages\n");
+    else
+        drm_notice(&i915->drm,
+               "Transparent Hugepage support is recommended for optimal performance%s\n",
+               GRAPHICS_VER(i915) >= 11 ? " on this platform!" :
+                              " when IOMMU is enabled!");
+
+ no_thp:
+    intel_memory_region_set_name(mem, "system");
+
+    return 0; /* We have fallback to the kernel mnt if huge mnt failed. */
  }
  static const struct intel_memory_region_ops shmem_region_ops = {
      .init = init_shmem,
-    .release = release_shmem,
      .init_object = shmem_object_init,
  };
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/ i915/gem/i915_gemfs.c
deleted file mode 100644
index 1f1290214031..000000000000
--- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
+++ /dev/null
@@ -1,71 +0,0 @@
-// SPDX-License-Identifier: MIT
-/*
- * Copyright © 2017 Intel Corporation
- */
-
-#include <linux/fs.h>
-#include <linux/mount.h>
-#include <linux/fs_context.h>
-
-#include <drm/drm_print.h>
-
-#include "i915_drv.h"
-#include "i915_gemfs.h"
-#include "i915_utils.h"
-
-void i915_gemfs_init(struct drm_i915_private *i915)
-{
-    struct file_system_type *type;
-    struct fs_context *fc;
-    struct vfsmount *gemfs;
-    int ret;
-
-    /*
-     * By creating our own shmemfs mountpoint, we can pass in
-     * mount flags that better match our usecase.
-     *
-     * One example, although it is probably better with a per-file
-     * control, is selecting huge page allocations ("huge=within_size").
-     * However, we only do so on platforms which benefit from it, or to
-     * offset the overhead of iommu lookups, where with latter it is a net
-     * win even on platforms which would otherwise see some performance
-     * regressions such a slow reads issue on Broadwell and Skylake.
-     */
-
-    if (GRAPHICS_VER(i915) < 11 && !i915_vtd_active(i915))
-        return;
-
-    if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
-        goto err;
-
-    type = get_fs_type("tmpfs");
-    if (!type)
-        goto err;
-
-    fc = fs_context_for_mount(type, SB_KERNMOUNT);
-    if (IS_ERR(fc))
-        goto err;
-    ret = vfs_parse_fs_string(fc, "source", "tmpfs");
-    if (!ret)
-        ret = vfs_parse_fs_string(fc, "huge", "within_size");
-    if (!ret)
-        gemfs = fc_mount_longterm(fc);
-    put_fs_context(fc);
-    if (ret)
-        goto err;
-
-    i915->mm.gemfs = gemfs;
-    drm_info(&i915->drm, "Using Transparent Hugepages\n");
-    return;
-
-err:
-    drm_notice(&i915->drm,
-           "Transparent Hugepage support is recommended for optimal performance%s\n",
-           GRAPHICS_VER(i915) >= 11 ? " on this platform!" :
-                          " when IOMMU is enabled!");
-}
-
-void i915_gemfs_fini(struct drm_i915_private *i915)
-{
-    kern_unmount(i915->mm.gemfs);
-}
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.h b/drivers/gpu/drm/ i915/gem/i915_gemfs.h
deleted file mode 100644
index 16d4333c9a4e..000000000000
--- a/drivers/gpu/drm/i915/gem/i915_gemfs.h
+++ /dev/null
@@ -1,14 +0,0 @@
-/* SPDX-License-Identifier: MIT */
-/*
- * Copyright © 2017 Intel Corporation
- */
-
-#ifndef __I915_GEMFS_H__
-#define __I915_GEMFS_H__
-
-struct drm_i915_private;
-
-void i915_gemfs_init(struct drm_i915_private *i915);
-void i915_gemfs_fini(struct drm_i915_private *i915);
-
-#endif
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/ drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index bd08605a1611..28aef75630a2 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1316,7 +1316,7 @@ typedef struct drm_i915_gem_object *
  static inline bool igt_can_allocate_thp(struct drm_i915_private *i915)
  {
-    return i915->mm.gemfs && has_transparent_hugepage();
+    return !!drm_gem_get_huge_mnt(&i915->drm);
  }
  static struct drm_i915_gem_object *
@@ -1761,7 +1761,9 @@ static int igt_tmpfs_fallback(void *arg)
      struct drm_i915_private *i915 = arg;
      struct i915_address_space *vm;
      struct i915_gem_context *ctx;
-    struct vfsmount *gemfs = i915->mm.gemfs;
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+    struct vfsmount *huge_mnt = i915->drm.huge_mnt;
+#endif
      struct drm_i915_gem_object *obj;
      struct i915_vma *vma;
      struct file *file;
@@ -1782,10 +1784,12 @@ static int igt_tmpfs_fallback(void *arg)
      /*
       * Make sure that we don't burst into a ball of flames upon falling back        * to tmpfs, which we rely on if on the off-chance we encounter a failure
-     * when setting up gemfs.
+     * when setting up a huge mountpoint.
       */
-    i915->mm.gemfs = NULL;
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+    i915->drm.huge_mnt = NULL;
+#endif
      obj = i915_gem_object_create_shmem(i915, PAGE_SIZE);
      if (IS_ERR(obj)) {
@@ -1819,7 +1823,9 @@ static int igt_tmpfs_fallback(void *arg)
  out_put:
      i915_gem_object_put(obj);
  out_restore:
-    i915->mm.gemfs = gemfs;
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+    i915->drm.huge_mnt = huge_mnt;
+#endif

Apart from this layering violation in the selftest, this version looks good to me. I am just wondering if we could somehow improve this aspect. I was thinking a self-test builds only special version of i915_gem_object_create_shmem. Call chain is deep but there are flags passed on:

i915_gem_object_create_shmem
   i915_gem_object_create_region
     __i915_gem_object_create_region
       err = mem->ops->init_object(

So we could add a new helper like:

selftests_create_shmem
   i915_gem_object_create_region(...flags = I915_BO_ALLOC_SELFTESTS_NOTHP...)

And in __create_shmem we just make it:

...
huge_mnt = drm_gem_get_huge_mnt(&i915->drm) &&
if (IS_ENABLED(..SELFTESTS..) &&
     (flags & I915_BO_ALLOC_SELFTESTS_NOTHP))
     huge_mnt = NULL;
...

It would avoid the ifdef and needing to play games with the DRM internals.

How does that sound to you?

That sounds better to me but I'm not very familiar with the i915 testing process. Would you be ready to accept the currect ifdef'd version for now and let me take a better look at that proposal later for a follow-up patch series?

I would rather we do it in one go. I assume you are compile testing the i915 part? I so, would you be happy to integrate something like this in your patch (adjusted for your changes):

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index 465ce94aee76..4dbd61280c93 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -348,12 +348,14 @@ struct drm_i915_gem_object {
  */
 #define I915_BO_ALLOC_GPU_ONLY   BIT(6)
 #define I915_BO_ALLOC_CCS_AUX    BIT(7)
+#define I915_BO_ALLOC_NOTHP      BIT(8)
 /*
* Object is allowed to retain its initial data and will not be cleared on first
  * access if used along with I915_BO_ALLOC_USER. This is mainly to keep
* preallocated framebuffer data intact while transitioning it to i915drmfb.
  */
-#define I915_BO_PREALLOC         BIT(8)
+#define I915_BO_PREALLOC         BIT(9)
+
 #define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS | \
                             I915_BO_ALLOC_VOLATILE | \
                             I915_BO_ALLOC_CPU_CLEAR | \
@@ -362,10 +364,12 @@ struct drm_i915_gem_object {
                             I915_BO_ALLOC_PM_EARLY | \
                             I915_BO_ALLOC_GPU_ONLY | \
                             I915_BO_ALLOC_CCS_AUX | \
+                            I915_BO_ALLOC_NOTHP | \
                             I915_BO_PREALLOC)
-#define I915_BO_READONLY          BIT(9)
-#define I915_TILING_QUIRK_BIT 10 /* unknown swizzling; do not release! */
-#define I915_BO_PROTECTED         BIT(11)
+#define I915_BO_READONLY          BIT(10)
+#define I915_TILING_QUIRK_BIT 11 /* unknown swizzling; do not release! */
+#define I915_BO_PROTECTED         BIT(12)
+
        /**
         * @mem_flags - Mutable placement-related flags
         *
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 26dda55a07ff..a1e876ce7bb9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -494,7 +494,8 @@ const struct drm_i915_gem_object_ops i915_gem_shmem_ops = {

 static int __create_shmem(struct drm_i915_private *i915,
                          struct drm_gem_object *obj,
-                         resource_size_t size)
+                         resource_size_t size,
+                         unsigned int flags)
 {
        unsigned long flags = VM_NORESERVE;
        struct file *filp;
@@ -515,7 +516,7 @@ static int __create_shmem(struct drm_i915_private *i915,
        if (BITS_PER_LONG == 64 && size > MAX_LFS_FILESIZE)
                return -E2BIG;

-       if (i915->mm.gemfs)
+       if (!(flags & I915_BO_ALLOC_NOTHP) && i915->mm.gemfs)
                filp = shmem_file_setup_with_mnt(i915->mm.gemfs, "i915", size,
                                                 flags);
        else
@@ -548,7 +549,7 @@ static int shmem_object_init(struct intel_memory_region *mem,
        gfp_t mask;
        int ret;

-       ret = __create_shmem(i915, &obj->base, size);
+       ret = __create_shmem(i915, &obj->base, size, flags);
        if (ret)
                return ret;

diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index bd08605a1611..c296af381007 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1787,7 +1787,8 @@ static int igt_tmpfs_fallback(void *arg)

        i915->mm.gemfs = NULL;

-       obj = i915_gem_object_create_shmem(i915, PAGE_SIZE);
+       obj = i915_gem_object_create_region(i915->mm.regions[INTEL_REGION_SMEM],
+                                           PAGE_SIZE, 0, I915_BO_ALLOC_NOTHP);
        if (IS_ERR(obj)) {
                err = PTR_ERR(obj);
                goto out_restore;


If it compiles I would take that and will handle any CI fall out.

Regards,

Tvrtko


Regards,

Loïc

Regards,

Tvrtko

      i915_vm_put(vm);
  out:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/ i915_drv.h
index 95f9ddf22ce4..93a5af3de334 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -141,11 +141,6 @@ struct i915_gem_mm {
       */
      atomic_t free_count;
-    /**
-     * tmpfs instance used for shmem backed objects
-     */
-    struct vfsmount *gemfs;
-
      struct intel_memory_region *regions[INTEL_REGION_UNKNOWN];
      struct notifier_block oom_notifier;


Reply via email to