| 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: <240d0dce-bbc4-4c82-9f9f-7f4625da8315@collabora.com>
Date: Fri, 28 Nov 2025 19:41:07 +0100
From: Loïc Molinari <loic.molinari@...labora.com>
To: Tvrtko Ursulin <tursulin@...ulin.net>,
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 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?
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