| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0a233d0d-17a2-4f5b-85bb-a33451ec6e9f@ursulin.net>
Date: Tue, 2 Dec 2025 11:01:04 +0000
From: Tvrtko Ursulin <tursulin@...ulin.net>
To: Loïc Molinari <loic.molinari@...labora.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Boris Brezillon <boris.brezillon@...labora.com>,
Rob Herring <robh@...nel.org>, Steven Price <steven.price@....com>,
Liviu Dudau <liviu.dudau@....com>, Melissa Wen <mwen@...lia.com>,
Maíra Canal <mcanal@...lia.com>,
Hugh Dickins <hughd@...gle.com>, Baolin Wang
<baolin.wang@...ux.alibaba.com>, Andrew Morton <akpm@...ux-foundation.org>,
Al Viro <viro@...iv.linux.org.uk>, Mikołaj Wasiak
<mikolaj.wasiak@...el.com>, Christian Brauner <brauner@...nel.org>,
Nitin Gote <nitin.r.gote@...el.com>, Andi Shyti
<andi.shyti@...ux.intel.com>, Jonathan Corbet <corbet@....net>,
Christopher Healy <healych@...zon.com>, Matthew Wilcox
<willy@...radead.org>, Bagas Sanjaya <bagasdotme@...il.com>
Cc: linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
intel-gfx@...ts.freedesktop.org, linux-mm@...ck.org,
linux-doc@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH v9 05/11] drm/i915: Use huge tmpfs mountpoint helpers
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 <loic.molinari@...labora.com>
>>> ---
>>> 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;
>
Powered by blists - more mailing lists