disable large folios for shmem file used by xfs xfile

disable large folios for shmem file used by xfs xfile

[Christoph Hellwig]

Hi all,

Darrick reported that the fairly new XFS xfile code blows up when force
enabling large folio for shmem.  This series fixes this quickly by disabling
large folios for this particular shmem file for now until it can be fixed
properly, which will be a lot more invasive.

I've added most of you to the CC list as I suspect most other users of
shmem_file_setup and friends will have similar issues.

disable large folios for shmem file used by xfs xfile

[Christoph Hellwig]

Hi all,

Darrick reported that the fairly new XFS xfile code blows up when force
enabling large folio for shmem.  This series fixes this quickly by disabling
large folios for this particular shmem file for now until it can be fixed
properly, which will be a lot more invasive.

I've added most of you to the CC list as I suspect most other users of
shmem_file_setup and friends will have similar issues.

[PATCH 1/2] mm: add a mapping_clear_large_folios helper

[Christoph Hellwig]

Users of shmem_kernel_file_setup might not be able to deal with large
folios (yet).  Give them a way to disable large folio support on their
mapping.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/pagemap.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 06142ff7f9ce0e..352d1f8423292c 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -343,6 +343,20 @@ static inline void mapping_set_large_folios(struct address_space *mapping)
 	__set_bit(AS_LARGE_FOLIO_SUPPORT, &mapping->flags);
 }
 
+/**
+ * mapping_clear_large_folios() - Disable large folio support for a mapping
+ * @mapping: The mapping.
+ *
+ * This can be called to undo the effect of mapping_set_large_folios().
+ *
+ * Context: This should not be called while the inode is active as it
+ * is non-atomic.
+ */
+static inline void mapping_clear_large_folios(struct address_space *mapping)
+{
+	__clear_bit(AS_LARGE_FOLIO_SUPPORT, &mapping->flags);
+}
+
 /*
  * Large folio support currently depends on THP.  These dependencies are
  * being worked on but are not yet fixed.
-- 
2.39.2

[PATCH 1/2] mm: add a mapping_clear_large_folios helper

[Christoph Hellwig]

Users of shmem_kernel_file_setup might not be able to deal with large
folios (yet).  Give them a way to disable large folio support on their
mapping.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/pagemap.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 06142ff7f9ce0e..352d1f8423292c 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -343,6 +343,20 @@ static inline void mapping_set_large_folios(struct address_space *mapping)
 	__set_bit(AS_LARGE_FOLIO_SUPPORT, &mapping->flags);
 }
 
+/**
+ * mapping_clear_large_folios() - Disable large folio support for a mapping
+ * @mapping: The mapping.
+ *
+ * This can be called to undo the effect of mapping_set_large_folios().
+ *
+ * Context: This should not be called while the inode is active as it
+ * is non-atomic.
+ */
+static inline void mapping_clear_large_folios(struct address_space *mapping)
+{
+	__clear_bit(AS_LARGE_FOLIO_SUPPORT, &mapping->flags);
+}
+
 /*
  * Large folio support currently depends on THP.  These dependencies are
  * being worked on but are not yet fixed.
-- 
2.39.2

[PATCH 2/2] xfs: disable large folio support in xfile_create

[Christoph Hellwig]

The xfarray code will crash if large folios are force enabled using:

   echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled

Fixing this will require a bit of an API change, and prefeably sorting out
the hwpoison story for pages vs folio and where it is placed in the shmem
API.  For now use this one liner to disable large folios.

Reported-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/xfile.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
index 090c3ead43fdf1..1a8d1bedd0b0dc 100644
--- a/fs/xfs/scrub/xfile.c
+++ b/fs/xfs/scrub/xfile.c
@@ -94,6 +94,11 @@ xfile_create(
 
 	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
 
+	/*
+	 * We're not quite ready for large folios yet.
+	 */
+	mapping_clear_large_folios(inode->i_mapping);
+
 	trace_xfile_create(xf);
 
 	*xfilep = xf;
-- 
2.39.2

[PATCH 2/2] xfs: disable large folio support in xfile_create

[Christoph Hellwig]

The xfarray code will crash if large folios are force enabled using:

   echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled

Fixing this will require a bit of an API change, and prefeably sorting out
the hwpoison story for pages vs folio and where it is placed in the shmem
API.  For now use this one liner to disable large folios.

Reported-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/xfile.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
index 090c3ead43fdf1..1a8d1bedd0b0dc 100644
--- a/fs/xfs/scrub/xfile.c
+++ b/fs/xfs/scrub/xfile.c
@@ -94,6 +94,11 @@ xfile_create(
 
 	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
 
+	/*
+	 * We're not quite ready for large folios yet.
+	 */
+	mapping_clear_large_folios(inode->i_mapping);
+
 	trace_xfile_create(xf);
 
 	*xfilep = xf;
-- 
2.39.2

Re: disable large folios for shmem file used by xfs xfile

[Matthew Wilcox]

On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> Hi all,
> 
> Darrick reported that the fairly new XFS xfile code blows up when force
> enabling large folio for shmem.  This series fixes this quickly by disabling
> large folios for this particular shmem file for now until it can be fixed
> properly, which will be a lot more invasive.
> 
> I've added most of you to the CC list as I suspect most other users of
> shmem_file_setup and friends will have similar issues.

The graphics users _want_ to use large folios.  I'm pretty sure they've
been tested with this.  It's just XFS that didn't know about this
feature of shmem.

Re: disable large folios for shmem file used by xfs xfile

[Matthew Wilcox]

On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> Hi all,
> 
> Darrick reported that the fairly new XFS xfile code blows up when force
> enabling large folio for shmem.  This series fixes this quickly by disabling
> large folios for this particular shmem file for now until it can be fixed
> properly, which will be a lot more invasive.
> 
> I've added most of you to the CC list as I suspect most other users of
> shmem_file_setup and friends will have similar issues.

The graphics users _want_ to use large folios.  I'm pretty sure they've
been tested with this.  It's just XFS that didn't know about this
feature of shmem.

Re: disable large folios for shmem file used by xfs xfile

[Matthew Wilcox]

On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> Hi all,
> 
> Darrick reported that the fairly new XFS xfile code blows up when force
> enabling large folio for shmem.  This series fixes this quickly by disabling
> large folios for this particular shmem file for now until it can be fixed
> properly, which will be a lot more invasive.
> 
> I've added most of you to the CC list as I suspect most other users of
> shmem_file_setup and friends will have similar issues.

The graphics users _want_ to use large folios.  I'm pretty sure they've
been tested with this.  It's just XFS that didn't know about this
feature of shmem.

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] mm: add a mapping_clear_large_folios helper

[Patchwork]

== Series Details ==

Series: series starting with [1/2] mm: add a mapping_clear_large_folios helper
URL   : https://patchwork.freedesktop.org/series/128420/
State : warning

== Summary ==

Error: dim checkpatch failed
e1a8add4aba0 mm: add a mapping_clear_large_folios helper
0faf35dc1522 xfs: disable large folio support in xfile_create
-:14: WARNING:BAD_REPORTED_BY_LINK: Reported-by: should be immediately followed by Closes: with a URL to the report
#14: 
Reported-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>

total: 0 errors, 1 warnings, 0 checks, 11 lines checked

✗ Fi.CI.SPARSE: warning for series starting with [1/2] mm: add a mapping_clear_large_folios helper

[Patchwork]

== Series Details ==

Series: series starting with [1/2] mm: add a mapping_clear_large_folios helper
URL   : https://patchwork.freedesktop.org/series/128420/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for series starting with [1/2] mm: add a mapping_clear_large_folios helper

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 8319 bytes --]

== Series Details ==

Series: series starting with [1/2] mm: add a mapping_clear_large_folios helper
URL   : https://patchwork.freedesktop.org/series/128420/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14108 -> Patchwork_128420v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_128420v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_128420v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/index.html

Participating hosts (38 -> 38)
------------------------------

  Additional (1): bat-mtlp-8 
  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_128420v1:

### CI changes ###

#### Possible regressions ####

  * boot:
    - bat-rpls-2:         [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14108/bat-rpls-2/boot.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-rpls-2/boot.html

  
Known issues
------------

  Here are the changes found in Patchwork_128420v1 that come from known issues:

### CI changes ###

#### Issues hit ####

  * boot:
    - fi-bsw-n3050:       [PASS][3] -> [FAIL][4] ([i915#8293])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14108/fi-bsw-n3050/boot.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/fi-bsw-n3050/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-mtlp-8:         NOTRUN -> [SKIP][5] ([i915#9318])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@debugfs_test@basic-hwmon.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-mtlp-8:         NOTRUN -> [SKIP][6] ([i915#4613]) +3 other tests skip
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_mmap@basic:
    - bat-mtlp-8:         NOTRUN -> [SKIP][7] ([i915#4083])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@gem_mmap@basic.html

  * igt@gem_mmap_gtt@basic:
    - bat-mtlp-8:         NOTRUN -> [SKIP][8] ([i915#4077]) +2 other tests skip
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@gem_mmap_gtt@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-mtlp-8:         NOTRUN -> [SKIP][9] ([i915#4079]) +1 other test skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@gem_render_tiled_blits@basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-mtlp-8:         NOTRUN -> [SKIP][10] ([i915#6621])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@i915_pm_rps@basic-api.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-mtlp-8:         NOTRUN -> [SKIP][11] ([i915#6645])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-mtlp-8:         NOTRUN -> [SKIP][12] ([i915#5190])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-y-tiled-legacy:
    - bat-mtlp-8:         NOTRUN -> [SKIP][13] ([i915#4212]) +8 other tests skip
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_addfb_basic@basic-y-tiled-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-mtlp-8:         NOTRUN -> [SKIP][14] ([i915#4213]) +1 other test skip
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-mtlp-8:         NOTRUN -> [SKIP][15] ([i915#3555] / [i915#3840] / [i915#9159])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-mtlp-8:         NOTRUN -> [SKIP][16] ([fdo#109285])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-mtlp-8:         NOTRUN -> [SKIP][17] ([i915#5274])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-atsm-1:         NOTRUN -> [SKIP][18] ([i915#1836])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-atsm-1/igt@kms_pipe_crc_basic@suspend-read-crc.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-mtlp-8:         NOTRUN -> [SKIP][19] ([i915#3555] / [i915#8809])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-mmap:
    - bat-mtlp-8:         NOTRUN -> [SKIP][20] ([i915#3708] / [i915#4077]) +1 other test skip
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-fence-read:
    - bat-mtlp-8:         NOTRUN -> [SKIP][21] ([i915#3708]) +2 other tests skip
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-mtlp-8/igt@prime_vgem@basic-fence-read.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@lmem0:
    - bat-atsm-1:         [ABORT][22] -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14108/bat-atsm-1/igt@gem_exec_suspend@basic-s3@lmem0.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/bat-atsm-1/igt@gem_exec_suspend@basic-s3@lmem0.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293
  [i915#8809]: https://gitlab.freedesktop.org/drm/intel/issues/8809
  [i915#9159]: https://gitlab.freedesktop.org/drm/intel/issues/9159
  [i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
  [i915#9688]: https://gitlab.freedesktop.org/drm/intel/issues/9688


Build changes
-------------

  * Linux: CI_DRM_14108 -> Patchwork_128420v1

  CI-20190529: 20190529
  CI_DRM_14108: c58d1352dd193d8df380a14e95c05455acaf5b82 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7666: 5f97adfd0e1636788a6af5b592f0d15b4f1027c8 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_128420v1: c58d1352dd193d8df380a14e95c05455acaf5b82 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

7691534a4217 xfs: disable large folio support in xfile_create
e80c21ce38cb mm: add a mapping_clear_large_folios helper

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_128420v1/index.html

[-- Attachment #2: Type: text/html, Size: 9582 bytes --]

Re: disable large folios for shmem file used by xfs xfile

[Joonas Lahtinen]

Quoting Matthew Wilcox (2024-01-10 14:37:18)
> On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> > Hi all,
> > 
> > Darrick reported that the fairly new XFS xfile code blows up when force
> > enabling large folio for shmem.  This series fixes this quickly by disabling
> > large folios for this particular shmem file for now until it can be fixed
> > properly, which will be a lot more invasive.
> > 
> > I've added most of you to the CC list as I suspect most other users of
> > shmem_file_setup and friends will have similar issues.
> 
> The graphics users _want_ to use large folios.  I'm pretty sure they've
> been tested with this.

Correct. We've done quite a bit of optimization in userspace and
enabling in kernel to take advantage of page sizes of 2M and beyond.

However we specifically pass "huge=within_size" to vfs_kern_mount when
creating a private mount of tmpfs for the purpose of i915 created
allocations.

Older hardware also had some address hashing bugs where 2M aligned
memory caused a lot of collisions in TLB so we don't enable it always.

You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
i915_gemfs_init for details and references.

So in short, functionality wise we should be fine either default
for using 2M pages or not. If they become the default, we'd probably
want an option that would still be able to prevent them for performance
regression reasons on older hardware.

Regards, Joonas

> It's just XFS that didn't know about this
> feature of shmem.

Re: disable large folios for shmem file used by xfs xfile

[Joonas Lahtinen]

Quoting Matthew Wilcox (2024-01-10 14:37:18)
> On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> > Hi all,
> > 
> > Darrick reported that the fairly new XFS xfile code blows up when force
> > enabling large folio for shmem.  This series fixes this quickly by disabling
> > large folios for this particular shmem file for now until it can be fixed
> > properly, which will be a lot more invasive.
> > 
> > I've added most of you to the CC list as I suspect most other users of
> > shmem_file_setup and friends will have similar issues.
> 
> The graphics users _want_ to use large folios.  I'm pretty sure they've
> been tested with this.

Correct. We've done quite a bit of optimization in userspace and
enabling in kernel to take advantage of page sizes of 2M and beyond.

However we specifically pass "huge=within_size" to vfs_kern_mount when
creating a private mount of tmpfs for the purpose of i915 created
allocations.

Older hardware also had some address hashing bugs where 2M aligned
memory caused a lot of collisions in TLB so we don't enable it always.

You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
i915_gemfs_init for details and references.

So in short, functionality wise we should be fine either default
for using 2M pages or not. If they become the default, we'd probably
want an option that would still be able to prevent them for performance
regression reasons on older hardware.

Regards, Joonas

> It's just XFS that didn't know about this
> feature of shmem.

Re: disable large folios for shmem file used by xfs xfile

[Joonas Lahtinen]

Quoting Matthew Wilcox (2024-01-10 14:37:18)
> On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> > Hi all,
> > 
> > Darrick reported that the fairly new XFS xfile code blows up when force
> > enabling large folio for shmem.  This series fixes this quickly by disabling
> > large folios for this particular shmem file for now until it can be fixed
> > properly, which will be a lot more invasive.
> > 
> > I've added most of you to the CC list as I suspect most other users of
> > shmem_file_setup and friends will have similar issues.
> 
> The graphics users _want_ to use large folios.  I'm pretty sure they've
> been tested with this.

Correct. We've done quite a bit of optimization in userspace and
enabling in kernel to take advantage of page sizes of 2M and beyond.

However we specifically pass "huge=within_size" to vfs_kern_mount when
creating a private mount of tmpfs for the purpose of i915 created
allocations.

Older hardware also had some address hashing bugs where 2M aligned
memory caused a lot of collisions in TLB so we don't enable it always.

You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
i915_gemfs_init for details and references.

So in short, functionality wise we should be fine either default
for using 2M pages or not. If they become the default, we'd probably
want an option that would still be able to prevent them for performance
regression reasons on older hardware.

Regards, Joonas

> It's just XFS that didn't know about this
> feature of shmem.

Re: disable large folios for shmem file used by xfs xfile

[Joonas Lahtinen]

Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> Quoting Matthew Wilcox (2024-01-10 14:37:18)
> > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> > > Hi all,
> > > 
> > > Darrick reported that the fairly new XFS xfile code blows up when force
> > > enabling large folio for shmem.  This series fixes this quickly by disabling
> > > large folios for this particular shmem file for now until it can be fixed
> > > properly, which will be a lot more invasive.
> > > 
> > > I've added most of you to the CC list as I suspect most other users of
> > > shmem_file_setup and friends will have similar issues.
> > 
> > The graphics users _want_ to use large folios.  I'm pretty sure they've
> > been tested with this.
> 
> Correct. We've done quite a bit of optimization in userspace and
> enabling in kernel to take advantage of page sizes of 2M and beyond.
> 
> However we specifically pass "huge=within_size" to vfs_kern_mount when
> creating a private mount of tmpfs for the purpose of i915 created
> allocations.
> 
> Older hardware also had some address hashing bugs where 2M aligned
> memory caused a lot of collisions in TLB so we don't enable it always.
> 
> You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> i915_gemfs_init for details and references.
> 
> So in short, functionality wise we should be fine either default
> for using 2M pages or not. If they become the default, we'd probably
> want an option that would still be able to prevent them for performance
> regression reasons on older hardware.

To maybe write out my concern better:

Is there plan to enable huge pages by default in shmem?

If not I guess we should be pretty good with the way current code is, force
enabling them just might bring out some performance, so we might want to add
a warning for that.

If there is, then we'll probably want to in sync with those default changes
apply a similar call to block them on older HW.

Regards, Joonas

> 
> Regards, Joonas
> 
> > It's just XFS that didn't know about this
> > feature of shmem.

Re: disable large folios for shmem file used by xfs xfile

[Joonas Lahtinen]

Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> Quoting Matthew Wilcox (2024-01-10 14:37:18)
> > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> > > Hi all,
> > > 
> > > Darrick reported that the fairly new XFS xfile code blows up when force
> > > enabling large folio for shmem.  This series fixes this quickly by disabling
> > > large folios for this particular shmem file for now until it can be fixed
> > > properly, which will be a lot more invasive.
> > > 
> > > I've added most of you to the CC list as I suspect most other users of
> > > shmem_file_setup and friends will have similar issues.
> > 
> > The graphics users _want_ to use large folios.  I'm pretty sure they've
> > been tested with this.
> 
> Correct. We've done quite a bit of optimization in userspace and
> enabling in kernel to take advantage of page sizes of 2M and beyond.
> 
> However we specifically pass "huge=within_size" to vfs_kern_mount when
> creating a private mount of tmpfs for the purpose of i915 created
> allocations.
> 
> Older hardware also had some address hashing bugs where 2M aligned
> memory caused a lot of collisions in TLB so we don't enable it always.
> 
> You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> i915_gemfs_init for details and references.
> 
> So in short, functionality wise we should be fine either default
> for using 2M pages or not. If they become the default, we'd probably
> want an option that would still be able to prevent them for performance
> regression reasons on older hardware.

To maybe write out my concern better:

Is there plan to enable huge pages by default in shmem?

If not I guess we should be pretty good with the way current code is, force
enabling them just might bring out some performance, so we might want to add
a warning for that.

If there is, then we'll probably want to in sync with those default changes
apply a similar call to block them on older HW.

Regards, Joonas

> 
> Regards, Joonas
> 
> > It's just XFS that didn't know about this
> > feature of shmem.

Re: disable large folios for shmem file used by xfs xfile

[Joonas Lahtinen]

Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> Quoting Matthew Wilcox (2024-01-10 14:37:18)
> > On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> > > Hi all,
> > > 
> > > Darrick reported that the fairly new XFS xfile code blows up when force
> > > enabling large folio for shmem.  This series fixes this quickly by disabling
> > > large folios for this particular shmem file for now until it can be fixed
> > > properly, which will be a lot more invasive.
> > > 
> > > I've added most of you to the CC list as I suspect most other users of
> > > shmem_file_setup and friends will have similar issues.
> > 
> > The graphics users _want_ to use large folios.  I'm pretty sure they've
> > been tested with this.
> 
> Correct. We've done quite a bit of optimization in userspace and
> enabling in kernel to take advantage of page sizes of 2M and beyond.
> 
> However we specifically pass "huge=within_size" to vfs_kern_mount when
> creating a private mount of tmpfs for the purpose of i915 created
> allocations.
> 
> Older hardware also had some address hashing bugs where 2M aligned
> memory caused a lot of collisions in TLB so we don't enable it always.
> 
> You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> i915_gemfs_init for details and references.
> 
> So in short, functionality wise we should be fine either default
> for using 2M pages or not. If they become the default, we'd probably
> want an option that would still be able to prevent them for performance
> regression reasons on older hardware.

To maybe write out my concern better:

Is there plan to enable huge pages by default in shmem?

If not I guess we should be pretty good with the way current code is, force
enabling them just might bring out some performance, so we might want to add
a warning for that.

If there is, then we'll probably want to in sync with those default changes
apply a similar call to block them on older HW.

Regards, Joonas

> 
> Regards, Joonas
> 
> > It's just XFS that didn't know about this
> > feature of shmem.

Re: disable large folios for shmem file used by xfs xfile

[Matthew Wilcox]

On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote:
> Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> > However we specifically pass "huge=within_size" to vfs_kern_mount when
> > creating a private mount of tmpfs for the purpose of i915 created
> > allocations.
> > 
> > Older hardware also had some address hashing bugs where 2M aligned
> > memory caused a lot of collisions in TLB so we don't enable it always.
> > 
> > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> > i915_gemfs_init for details and references.
> > 
> > So in short, functionality wise we should be fine either default
> > for using 2M pages or not. If they become the default, we'd probably
> > want an option that would still be able to prevent them for performance
> > regression reasons on older hardware.
> 
> To maybe write out my concern better:
> 
> Is there plan to enable huge pages by default in shmem?

Not in the next kernel release, but eventually the plan is to allow
arbitrary order folios to be used in shmem.  So you could ask it to create
a 256kB folio for you, if that's the right size to manage memory in.

How shmem and its various users go about choosing the right size is not
quite clear to me yet.  Perhaps somebody else will do it before I get
to it; I have a lot of different sub-projects to work on at the moment,
and shmem isn't blocking any of them.  And I have a sneaking suspicion
that more work is needed in the swap code to deal with arbitrary order
folios, so that's another reason for me to delay looking at this ;-)

Re: disable large folios for shmem file used by xfs xfile

[Matthew Wilcox]

On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote:
> Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> > However we specifically pass "huge=within_size" to vfs_kern_mount when
> > creating a private mount of tmpfs for the purpose of i915 created
> > allocations.
> > 
> > Older hardware also had some address hashing bugs where 2M aligned
> > memory caused a lot of collisions in TLB so we don't enable it always.
> > 
> > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> > i915_gemfs_init for details and references.
> > 
> > So in short, functionality wise we should be fine either default
> > for using 2M pages or not. If they become the default, we'd probably
> > want an option that would still be able to prevent them for performance
> > regression reasons on older hardware.
> 
> To maybe write out my concern better:
> 
> Is there plan to enable huge pages by default in shmem?

Not in the next kernel release, but eventually the plan is to allow
arbitrary order folios to be used in shmem.  So you could ask it to create
a 256kB folio for you, if that's the right size to manage memory in.

How shmem and its various users go about choosing the right size is not
quite clear to me yet.  Perhaps somebody else will do it before I get
to it; I have a lot of different sub-projects to work on at the moment,
and shmem isn't blocking any of them.  And I have a sneaking suspicion
that more work is needed in the swap code to deal with arbitrary order
folios, so that's another reason for me to delay looking at this ;-)

Re: disable large folios for shmem file used by xfs xfile

[Matthew Wilcox]

On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote:
> Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> > However we specifically pass "huge=within_size" to vfs_kern_mount when
> > creating a private mount of tmpfs for the purpose of i915 created
> > allocations.
> > 
> > Older hardware also had some address hashing bugs where 2M aligned
> > memory caused a lot of collisions in TLB so we don't enable it always.
> > 
> > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> > i915_gemfs_init for details and references.
> > 
> > So in short, functionality wise we should be fine either default
> > for using 2M pages or not. If they become the default, we'd probably
> > want an option that would still be able to prevent them for performance
> > regression reasons on older hardware.
> 
> To maybe write out my concern better:
> 
> Is there plan to enable huge pages by default in shmem?

Not in the next kernel release, but eventually the plan is to allow
arbitrary order folios to be used in shmem.  So you could ask it to create
a 256kB folio for you, if that's the right size to manage memory in.

How shmem and its various users go about choosing the right size is not
quite clear to me yet.  Perhaps somebody else will do it before I get
to it; I have a lot of different sub-projects to work on at the moment,
and shmem isn't blocking any of them.  And I have a sneaking suspicion
that more work is needed in the swap code to deal with arbitrary order
folios, so that's another reason for me to delay looking at this ;-)

Re: disable large folios for shmem file used by xfs xfile

[Andrew Morton]

On Wed, 10 Jan 2024 10:21:07 +0100 Christoph Hellwig <hch@lst.de> wrote:

> Hi all,
> 
> Darrick reported that the fairly new XFS xfile code blows up when force
> enabling large folio for shmem.  This series fixes this quickly by disabling
> large folios for this particular shmem file for now until it can be fixed
> properly, which will be a lot more invasive.
> 

Patches seems reasonable as a short-term only-affects-xfs thing.

I assume that kernels which contain 137db333b29186 ("xfs: teach xfile
to pass back direct-map pages to caller") want this, so a Fixes: that
and a cc:stable are appropriate?

Re: disable large folios for shmem file used by xfs xfile

[Andrew Morton]

On Wed, 10 Jan 2024 10:21:07 +0100 Christoph Hellwig <hch@lst.de> wrote:

> Hi all,
> 
> Darrick reported that the fairly new XFS xfile code blows up when force
> enabling large folio for shmem.  This series fixes this quickly by disabling
> large folios for this particular shmem file for now until it can be fixed
> properly, which will be a lot more invasive.
> 

Patches seems reasonable as a short-term only-affects-xfs thing.

I assume that kernels which contain 137db333b29186 ("xfs: teach xfile
to pass back direct-map pages to caller") want this, so a Fixes: that
and a cc:stable are appropriate?

Re: disable large folios for shmem file used by xfs xfile

[Andrew Morton]

On Wed, 10 Jan 2024 10:21:07 +0100 Christoph Hellwig <hch@lst.de> wrote:

> Hi all,
> 
> Darrick reported that the fairly new XFS xfile code blows up when force
> enabling large folio for shmem.  This series fixes this quickly by disabling
> large folios for this particular shmem file for now until it can be fixed
> properly, which will be a lot more invasive.
> 

Patches seems reasonable as a short-term only-affects-xfs thing.

I assume that kernels which contain 137db333b29186 ("xfs: teach xfile
to pass back direct-map pages to caller") want this, so a Fixes: that
and a cc:stable are appropriate?

Re: disable large folios for shmem file used by xfs xfile

[Christoph Hellwig]

On Wed, Jan 10, 2024 at 12:37:18PM +0000, Matthew Wilcox wrote:
> On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> > Hi all,
> > 
> > Darrick reported that the fairly new XFS xfile code blows up when force
> > enabling large folio for shmem.  This series fixes this quickly by disabling
> > large folios for this particular shmem file for now until it can be fixed
> > properly, which will be a lot more invasive.
> > 
> > I've added most of you to the CC list as I suspect most other users of
> > shmem_file_setup and friends will have similar issues.
> 
> The graphics users _want_ to use large folios.  I'm pretty sure they've
> been tested with this.  It's just XFS that didn't know about this
> feature of shmem.

At least sgx has all kinds of PAGE_SIZE assumptions.  I haven't audited
(and am probably not qualified to) audit that code, so I wanted to send
a headsup to everyone.

Re: disable large folios for shmem file used by xfs xfile

[Christoph Hellwig]

On Wed, Jan 10, 2024 at 12:37:18PM +0000, Matthew Wilcox wrote:
> On Wed, Jan 10, 2024 at 10:21:07AM +0100, Christoph Hellwig wrote:
> > Hi all,
> > 
> > Darrick reported that the fairly new XFS xfile code blows up when force
> > enabling large folio for shmem.  This series fixes this quickly by disabling
> > large folios for this particular shmem file for now until it can be fixed
> > properly, which will be a lot more invasive.
> > 
> > I've added most of you to the CC list as I suspect most other users of
> > shmem_file_setup and friends will have similar issues.
> 
> The graphics users _want_ to use large folios.  I'm pretty sure they've
> been tested with this.  It's just XFS that didn't know about this
> feature of shmem.

At least sgx has all kinds of PAGE_SIZE assumptions.  I haven't audited
(and am probably not qualified to) audit that code, so I wanted to send
a headsup to everyone.

Re: disable large folios for shmem file used by xfs xfile

[Christoph Hellwig]

On Wed, Jan 10, 2024 at 07:38:43AM -0800, Andrew Morton wrote:
> I assume that kernels which contain 137db333b29186 ("xfs: teach xfile
> to pass back direct-map pages to caller") want this, so a Fixes: that
> and a cc:stable are appropriate?

I think it needs to back all the way back to 3934e8ebb7c
("xfs: create a big array data structure") as that only clears the page
sized chunk of a new allocation and could lead to corruption / or
information leaks.

Re: disable large folios for shmem file used by xfs xfile

[Christoph Hellwig]

On Wed, Jan 10, 2024 at 07:38:43AM -0800, Andrew Morton wrote:
> I assume that kernels which contain 137db333b29186 ("xfs: teach xfile
> to pass back direct-map pages to caller") want this, so a Fixes: that
> and a cc:stable are appropriate?

I think it needs to back all the way back to 3934e8ebb7c
("xfs: create a big array data structure") as that only clears the page
sized chunk of a new allocation and could lead to corruption / or
information leaks.

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Wed, Jan 10, 2024 at 10:21:09AM +0100, Christoph Hellwig wrote:
> The xfarray code will crash if large folios are force enabled using:
> 
>    echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled
> 
> Fixing this will require a bit of an API change, and prefeably sorting out
> the hwpoison story for pages vs folio and where it is placed in the shmem
> API.  For now use this one liner to disable large folios.
> 
> Reported-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Can someone who knows more about shmem.c than I do please review
https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
so that I can feel slightly more confident as hch and I sort through the
xfile.c issues?

For this patch,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/scrub/xfile.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> index 090c3ead43fdf1..1a8d1bedd0b0dc 100644
> --- a/fs/xfs/scrub/xfile.c
> +++ b/fs/xfs/scrub/xfile.c
> @@ -94,6 +94,11 @@ xfile_create(
>  
>  	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
>  
> +	/*
> +	 * We're not quite ready for large folios yet.
> +	 */
> +	mapping_clear_large_folios(inode->i_mapping);
> +
>  	trace_xfile_create(xf);
>  
>  	*xfilep = xf;
> -- 
> 2.39.2
> 
> 

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Wed, Jan 10, 2024 at 10:21:09AM +0100, Christoph Hellwig wrote:
> The xfarray code will crash if large folios are force enabled using:
> 
>    echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled
> 
> Fixing this will require a bit of an API change, and prefeably sorting out
> the hwpoison story for pages vs folio and where it is placed in the shmem
> API.  For now use this one liner to disable large folios.
> 
> Reported-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Can someone who knows more about shmem.c than I do please review
https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
so that I can feel slightly more confident as hch and I sort through the
xfile.c issues?

For this patch,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/scrub/xfile.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> index 090c3ead43fdf1..1a8d1bedd0b0dc 100644
> --- a/fs/xfs/scrub/xfile.c
> +++ b/fs/xfs/scrub/xfile.c
> @@ -94,6 +94,11 @@ xfile_create(
>  
>  	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
>  
> +	/*
> +	 * We're not quite ready for large folios yet.
> +	 */
> +	mapping_clear_large_folios(inode->i_mapping);
> +
>  	trace_xfile_create(xf);
>  
>  	*xfilep = xf;
> -- 
> 2.39.2
> 
> 

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Wed, Jan 10, 2024 at 10:21:09AM +0100, Christoph Hellwig wrote:
> The xfarray code will crash if large folios are force enabled using:
> 
>    echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled
> 
> Fixing this will require a bit of an API change, and prefeably sorting out
> the hwpoison story for pages vs folio and where it is placed in the shmem
> API.  For now use this one liner to disable large folios.
> 
> Reported-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Can someone who knows more about shmem.c than I do please review
https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
so that I can feel slightly more confident as hch and I sort through the
xfile.c issues?

For this patch,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/scrub/xfile.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> index 090c3ead43fdf1..1a8d1bedd0b0dc 100644
> --- a/fs/xfs/scrub/xfile.c
> +++ b/fs/xfs/scrub/xfile.c
> @@ -94,6 +94,11 @@ xfile_create(
>  
>  	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
>  
> +	/*
> +	 * We're not quite ready for large folios yet.
> +	 */
> +	mapping_clear_large_folios(inode->i_mapping);
> +
>  	trace_xfile_create(xf);
>  
>  	*xfilep = xf;
> -- 
> 2.39.2
> 
> 

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Wed, Jan 10, 2024 at 09:55:15AM -0800, Darrick J. Wong wrote:
> On Wed, Jan 10, 2024 at 10:21:09AM +0100, Christoph Hellwig wrote:
> > The xfarray code will crash if large folios are force enabled using:
> > 
> >    echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled
> > 
> > Fixing this will require a bit of an API change, and prefeably sorting out
> > the hwpoison story for pages vs folio and where it is placed in the shmem
> > API.  For now use this one liner to disable large folios.
> > 
> > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Can someone who knows more about shmem.c than I do please review
> https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> so that I can feel slightly more confident as hch and I sort through the
> xfile.c issues?
> 
> For this patch,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

...except that I'm still getting 2M THPs even with this enabled, so I
guess either we get to fix it now, or create our own private tmpfs mount
so that we can pass in huge=never, similar to what i915 does. :(

--D

> --D
> 
> > ---
> >  fs/xfs/scrub/xfile.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> > index 090c3ead43fdf1..1a8d1bedd0b0dc 100644
> > --- a/fs/xfs/scrub/xfile.c
> > +++ b/fs/xfs/scrub/xfile.c
> > @@ -94,6 +94,11 @@ xfile_create(
> >  
> >  	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
> >  
> > +	/*
> > +	 * We're not quite ready for large folios yet.
> > +	 */
> > +	mapping_clear_large_folios(inode->i_mapping);
> > +
> >  	trace_xfile_create(xf);
> >  
> >  	*xfilep = xf;
> > -- 
> > 2.39.2
> > 
> > 
> 

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Wed, Jan 10, 2024 at 09:55:15AM -0800, Darrick J. Wong wrote:
> On Wed, Jan 10, 2024 at 10:21:09AM +0100, Christoph Hellwig wrote:
> > The xfarray code will crash if large folios are force enabled using:
> > 
> >    echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled
> > 
> > Fixing this will require a bit of an API change, and prefeably sorting out
> > the hwpoison story for pages vs folio and where it is placed in the shmem
> > API.  For now use this one liner to disable large folios.
> > 
> > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Can someone who knows more about shmem.c than I do please review
> https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> so that I can feel slightly more confident as hch and I sort through the
> xfile.c issues?
> 
> For this patch,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

...except that I'm still getting 2M THPs even with this enabled, so I
guess either we get to fix it now, or create our own private tmpfs mount
so that we can pass in huge=never, similar to what i915 does. :(

--D

> --D
> 
> > ---
> >  fs/xfs/scrub/xfile.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> > index 090c3ead43fdf1..1a8d1bedd0b0dc 100644
> > --- a/fs/xfs/scrub/xfile.c
> > +++ b/fs/xfs/scrub/xfile.c
> > @@ -94,6 +94,11 @@ xfile_create(
> >  
> >  	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
> >  
> > +	/*
> > +	 * We're not quite ready for large folios yet.
> > +	 */
> > +	mapping_clear_large_folios(inode->i_mapping);
> > +
> >  	trace_xfile_create(xf);
> >  
> >  	*xfilep = xf;
> > -- 
> > 2.39.2
> > 
> > 
> 

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Wed, Jan 10, 2024 at 09:55:15AM -0800, Darrick J. Wong wrote:
> On Wed, Jan 10, 2024 at 10:21:09AM +0100, Christoph Hellwig wrote:
> > The xfarray code will crash if large folios are force enabled using:
> > 
> >    echo force > /sys/kernel/mm/transparent_hugepage/shmem_enabled
> > 
> > Fixing this will require a bit of an API change, and prefeably sorting out
> > the hwpoison story for pages vs folio and where it is placed in the shmem
> > API.  For now use this one liner to disable large folios.
> > 
> > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Can someone who knows more about shmem.c than I do please review
> https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> so that I can feel slightly more confident as hch and I sort through the
> xfile.c issues?
> 
> For this patch,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>

...except that I'm still getting 2M THPs even with this enabled, so I
guess either we get to fix it now, or create our own private tmpfs mount
so that we can pass in huge=never, similar to what i915 does. :(

--D

> --D
> 
> > ---
> >  fs/xfs/scrub/xfile.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> > index 090c3ead43fdf1..1a8d1bedd0b0dc 100644
> > --- a/fs/xfs/scrub/xfile.c
> > +++ b/fs/xfs/scrub/xfile.c
> > @@ -94,6 +94,11 @@ xfile_create(
> >  
> >  	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
> >  
> > +	/*
> > +	 * We're not quite ready for large folios yet.
> > +	 */
> > +	mapping_clear_large_folios(inode->i_mapping);
> > +
> >  	trace_xfile_create(xf);
> >  
> >  	*xfilep = xf;
> > -- 
> > 2.39.2
> > 
> > 
> 

Re: disable large folios for shmem file used by xfs xfile

[Daniel Gomez]

On Wed, Jan 10, 2024 at 4:35 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote:
> > Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> > > However we specifically pass "huge=within_size" to vfs_kern_mount when
> > > creating a private mount of tmpfs for the purpose of i915 created
> > > allocations.
> > >
> > > Older hardware also had some address hashing bugs where 2M aligned
> > > memory caused a lot of collisions in TLB so we don't enable it always.
> > >
> > > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> > > i915_gemfs_init for details and references.
> > >
> > > So in short, functionality wise we should be fine either default
> > > for using 2M pages or not. If they become the default, we'd probably
> > > want an option that would still be able to prevent them for performance
> > > regression reasons on older hardware.
> >
> > To maybe write out my concern better:
> >
> > Is there plan to enable huge pages by default in shmem?
>
> Not in the next kernel release, but eventually the plan is to allow
> arbitrary order folios to be used in shmem.  So you could ask it to create
> a 256kB folio for you, if that's the right size to manage memory in.
>
> How shmem and its various users go about choosing the right size is not
> quite clear to me yet.  Perhaps somebody else will do it before I get
> to it; I have a lot of different sub-projects to work on at the moment,
> and shmem isn't blocking any of them.  And I have a sneaking suspicion
> that more work is needed in the swap code to deal with arbitrary order
> folios, so that's another reason for me to delay looking at this ;-)

I have sent large folios support for shmem for the write and fallocate
path some releases ago. The main problem I was facing was a current
upstream problem with huge pages when seeking holes/data (fstests
generic/285 and generic/436). The strategy suggested was to use large
folios in an opportunistic way based on the file size. This hit the
same problem we currently have with huge pages and I considered that a
regression. We have made some progress to fix seeking in huge pages
upstream but is not yet finished. I can send the patches tomorrow for
further discussion.

>

Re: disable large folios for shmem file used by xfs xfile

[Daniel Gomez]

On Wed, Jan 10, 2024 at 4:35 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote:
> > Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> > > However we specifically pass "huge=within_size" to vfs_kern_mount when
> > > creating a private mount of tmpfs for the purpose of i915 created
> > > allocations.
> > >
> > > Older hardware also had some address hashing bugs where 2M aligned
> > > memory caused a lot of collisions in TLB so we don't enable it always.
> > >
> > > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> > > i915_gemfs_init for details and references.
> > >
> > > So in short, functionality wise we should be fine either default
> > > for using 2M pages or not. If they become the default, we'd probably
> > > want an option that would still be able to prevent them for performance
> > > regression reasons on older hardware.
> >
> > To maybe write out my concern better:
> >
> > Is there plan to enable huge pages by default in shmem?
>
> Not in the next kernel release, but eventually the plan is to allow
> arbitrary order folios to be used in shmem.  So you could ask it to create
> a 256kB folio for you, if that's the right size to manage memory in.
>
> How shmem and its various users go about choosing the right size is not
> quite clear to me yet.  Perhaps somebody else will do it before I get
> to it; I have a lot of different sub-projects to work on at the moment,
> and shmem isn't blocking any of them.  And I have a sneaking suspicion
> that more work is needed in the swap code to deal with arbitrary order
> folios, so that's another reason for me to delay looking at this ;-)

I have sent large folios support for shmem for the write and fallocate
path some releases ago. The main problem I was facing was a current
upstream problem with huge pages when seeking holes/data (fstests
generic/285 and generic/436). The strategy suggested was to use large
folios in an opportunistic way based on the file size. This hit the
same problem we currently have with huge pages and I considered that a
regression. We have made some progress to fix seeking in huge pages
upstream but is not yet finished. I can send the patches tomorrow for
further discussion.

>

Re: disable large folios for shmem file used by xfs xfile

[Daniel Gomez]

On Wed, Jan 10, 2024 at 4:35 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Wed, Jan 10, 2024 at 05:28:22PM +0200, Joonas Lahtinen wrote:
> > Quoting Joonas Lahtinen (2024-01-10 17:20:24)
> > > However we specifically pass "huge=within_size" to vfs_kern_mount when
> > > creating a private mount of tmpfs for the purpose of i915 created
> > > allocations.
> > >
> > > Older hardware also had some address hashing bugs where 2M aligned
> > > memory caused a lot of collisions in TLB so we don't enable it always.
> > >
> > > You can see drivers/gpu/drm/i915/gem/i915_gemfs.c function
> > > i915_gemfs_init for details and references.
> > >
> > > So in short, functionality wise we should be fine either default
> > > for using 2M pages or not. If they become the default, we'd probably
> > > want an option that would still be able to prevent them for performance
> > > regression reasons on older hardware.
> >
> > To maybe write out my concern better:
> >
> > Is there plan to enable huge pages by default in shmem?
>
> Not in the next kernel release, but eventually the plan is to allow
> arbitrary order folios to be used in shmem.  So you could ask it to create
> a 256kB folio for you, if that's the right size to manage memory in.
>
> How shmem and its various users go about choosing the right size is not
> quite clear to me yet.  Perhaps somebody else will do it before I get
> to it; I have a lot of different sub-projects to work on at the moment,
> and shmem isn't blocking any of them.  And I have a sneaking suspicion
> that more work is needed in the swap code to deal with arbitrary order
> folios, so that's another reason for me to delay looking at this ;-)

I have sent large folios support for shmem for the write and fallocate
path some releases ago. The main problem I was facing was a current
upstream problem with huge pages when seeking holes/data (fstests
generic/285 and generic/436). The strategy suggested was to use large
folios in an opportunistic way based on the file size. This hit the
same problem we currently have with huge pages and I considered that a
regression. We have made some progress to fix seeking in huge pages
upstream but is not yet finished. I can send the patches tomorrow for
further discussion.

>

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Andrew Morton]

On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:

> > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > API.  For now use this one liner to disable large folios.
> > > 
> > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Can someone who knows more about shmem.c than I do please review
> > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > so that I can feel slightly more confident as hch and I sort through the
> > xfile.c issues?
> > 
> > For this patch,
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> ...except that I'm still getting 2M THPs even with this enabled, so I
> guess either we get to fix it now, or create our own private tmpfs mount
> so that we can pass in huge=never, similar to what i915 does. :(

What is "this"?  Are you saying that $Subject doesn't work, or that the
above-linked please-review patch doesn't work?

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Andrew Morton]

On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:

> > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > API.  For now use this one liner to disable large folios.
> > > 
> > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Can someone who knows more about shmem.c than I do please review
> > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > so that I can feel slightly more confident as hch and I sort through the
> > xfile.c issues?
> > 
> > For this patch,
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> ...except that I'm still getting 2M THPs even with this enabled, so I
> guess either we get to fix it now, or create our own private tmpfs mount
> so that we can pass in huge=never, similar to what i915 does. :(

What is "this"?  Are you saying that $Subject doesn't work, or that the
above-linked please-review patch doesn't work?

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Andrew Morton]

On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:

> > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > API.  For now use this one liner to disable large folios.
> > > 
> > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Can someone who knows more about shmem.c than I do please review
> > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > so that I can feel slightly more confident as hch and I sort through the
> > xfile.c issues?
> > 
> > For this patch,
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> ...except that I'm still getting 2M THPs even with this enabled, so I
> guess either we get to fix it now, or create our own private tmpfs mount
> so that we can pass in huge=never, similar to what i915 does. :(

What is "this"?  Are you saying that $Subject doesn't work, or that the
above-linked please-review patch doesn't work?

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Matthew Wilcox]

On Thu, Jan 11, 2024 at 02:00:53PM -0800, Andrew Morton wrote:
> On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> 
> > > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > > API.  For now use this one liner to disable large folios.
> > > > 
> > > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > Can someone who knows more about shmem.c than I do please review
> > > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > > so that I can feel slightly more confident as hch and I sort through the
> > > xfile.c issues?
> > > 
> > > For this patch,
> > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > 
> > ...except that I'm still getting 2M THPs even with this enabled, so I
> > guess either we get to fix it now, or create our own private tmpfs mount
> > so that we can pass in huge=never, similar to what i915 does. :(
> 
> What is "this"?  Are you saying that $Subject doesn't work, or that the
> above-linked please-review patch doesn't work?

shmem pays no attention to the mapping_large_folio_support() flag,
so the proposed fix doesn't work.  It ought to, but it has its own way
of doing it that predates mapping_large_folio_support existing.

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Matthew Wilcox]

On Thu, Jan 11, 2024 at 02:00:53PM -0800, Andrew Morton wrote:
> On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> 
> > > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > > API.  For now use this one liner to disable large folios.
> > > > 
> > > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > Can someone who knows more about shmem.c than I do please review
> > > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > > so that I can feel slightly more confident as hch and I sort through the
> > > xfile.c issues?
> > > 
> > > For this patch,
> > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > 
> > ...except that I'm still getting 2M THPs even with this enabled, so I
> > guess either we get to fix it now, or create our own private tmpfs mount
> > so that we can pass in huge=never, similar to what i915 does. :(
> 
> What is "this"?  Are you saying that $Subject doesn't work, or that the
> above-linked please-review patch doesn't work?

shmem pays no attention to the mapping_large_folio_support() flag,
so the proposed fix doesn't work.  It ought to, but it has its own way
of doing it that predates mapping_large_folio_support existing.

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Matthew Wilcox]

On Thu, Jan 11, 2024 at 02:00:53PM -0800, Andrew Morton wrote:
> On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> 
> > > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > > API.  For now use this one liner to disable large folios.
> > > > 
> > > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > Can someone who knows more about shmem.c than I do please review
> > > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > > so that I can feel slightly more confident as hch and I sort through the
> > > xfile.c issues?
> > > 
> > > For this patch,
> > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > 
> > ...except that I'm still getting 2M THPs even with this enabled, so I
> > guess either we get to fix it now, or create our own private tmpfs mount
> > so that we can pass in huge=never, similar to what i915 does. :(
> 
> What is "this"?  Are you saying that $Subject doesn't work, or that the
> above-linked please-review patch doesn't work?

shmem pays no attention to the mapping_large_folio_support() flag,
so the proposed fix doesn't work.  It ought to, but it has its own way
of doing it that predates mapping_large_folio_support existing.

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Thu, Jan 11, 2024 at 10:45:53PM +0000, Matthew Wilcox wrote:
> On Thu, Jan 11, 2024 at 02:00:53PM -0800, Andrew Morton wrote:
> > On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> > 
> > > > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > > > API.  For now use this one liner to disable large folios.
> > > > > 
> > > > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > 
> > > > Can someone who knows more about shmem.c than I do please review
> > > > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > > > so that I can feel slightly more confident as hch and I sort through the
> > > > xfile.c issues?
> > > > 
> > > > For this patch,
> > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > ...except that I'm still getting 2M THPs even with this enabled, so I
> > > guess either we get to fix it now, or create our own private tmpfs mount
> > > so that we can pass in huge=never, similar to what i915 does. :(
> > 
> > What is "this"?  Are you saying that $Subject doesn't work, or that the
> > above-linked please-review patch doesn't work?
> 
> shmem pays no attention to the mapping_large_folio_support() flag,
> so the proposed fix doesn't work.  It ought to, but it has its own way
> of doing it that predates mapping_large_folio_support existing.

Yep.  It turned out to be easier to fix xfile.c to deal with large
folios than I thought it would be.  Or so I think.  We'll see what
happens on fstestscloud overnight.

--D

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Thu, Jan 11, 2024 at 10:45:53PM +0000, Matthew Wilcox wrote:
> On Thu, Jan 11, 2024 at 02:00:53PM -0800, Andrew Morton wrote:
> > On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> > 
> > > > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > > > API.  For now use this one liner to disable large folios.
> > > > > 
> > > > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > 
> > > > Can someone who knows more about shmem.c than I do please review
> > > > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > > > so that I can feel slightly more confident as hch and I sort through the
> > > > xfile.c issues?
> > > > 
> > > > For this patch,
> > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > ...except that I'm still getting 2M THPs even with this enabled, so I
> > > guess either we get to fix it now, or create our own private tmpfs mount
> > > so that we can pass in huge=never, similar to what i915 does. :(
> > 
> > What is "this"?  Are you saying that $Subject doesn't work, or that the
> > above-linked please-review patch doesn't work?
> 
> shmem pays no attention to the mapping_large_folio_support() flag,
> so the proposed fix doesn't work.  It ought to, but it has its own way
> of doing it that predates mapping_large_folio_support existing.

Yep.  It turned out to be easier to fix xfile.c to deal with large
folios than I thought it would be.  Or so I think.  We'll see what
happens on fstestscloud overnight.

--D

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Thu, Jan 11, 2024 at 10:45:53PM +0000, Matthew Wilcox wrote:
> On Thu, Jan 11, 2024 at 02:00:53PM -0800, Andrew Morton wrote:
> > On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> > 
> > > > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > > > API.  For now use this one liner to disable large folios.
> > > > > 
> > > > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > 
> > > > Can someone who knows more about shmem.c than I do please review
> > > > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > > > so that I can feel slightly more confident as hch and I sort through the
> > > > xfile.c issues?
> > > > 
> > > > For this patch,
> > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > ...except that I'm still getting 2M THPs even with this enabled, so I
> > > guess either we get to fix it now, or create our own private tmpfs mount
> > > so that we can pass in huge=never, similar to what i915 does. :(
> > 
> > What is "this"?  Are you saying that $Subject doesn't work, or that the
> > above-linked please-review patch doesn't work?
> 
> shmem pays no attention to the mapping_large_folio_support() flag,
> so the proposed fix doesn't work.  It ought to, but it has its own way
> of doing it that predates mapping_large_folio_support existing.

Yep.  It turned out to be easier to fix xfile.c to deal with large
folios than I thought it would be.  Or so I think.  We'll see what
happens on fstestscloud overnight.

--D

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Andrew Morton]

On Thu, 11 Jan 2024 18:22:50 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:

> On Thu, Jan 11, 2024 at 10:45:53PM +0000, Matthew Wilcox wrote:
> > On Thu, Jan 11, 2024 at 02:00:53PM -0800, Andrew Morton wrote:
> > > On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> > > 
> > > > > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > > > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > > > > API.  For now use this one liner to disable large folios.
> > > > > > 
> > > > > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > > 
> > > > > Can someone who knows more about shmem.c than I do please review
> > > > > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > > > > so that I can feel slightly more confident as hch and I sort through the
> > > > > xfile.c issues?
> > > > > 
> > > > > For this patch,
> > > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > ...except that I'm still getting 2M THPs even with this enabled, so I
> > > > guess either we get to fix it now, or create our own private tmpfs mount
> > > > so that we can pass in huge=never, similar to what i915 does. :(
> > > 
> > > What is "this"?  Are you saying that $Subject doesn't work, or that the
> > > above-linked please-review patch doesn't work?
> > 
> > shmem pays no attention to the mapping_large_folio_support() flag,
> > so the proposed fix doesn't work.  It ought to, but it has its own way
> > of doing it that predates mapping_large_folio_support existing.
> 
> Yep.  It turned out to be easier to fix xfile.c to deal with large
> folios than I thought it would be.  Or so I think.  We'll see what
> happens on fstestscloud overnight.

Where do we stand with this?  Should I merge these two patches into
6.8-rcX, cc:stable?

Re: [PATCH 2/2] xfs: disable large folio support in xfile_create

[Darrick J. Wong]

On Wed, Feb 07, 2024 at 05:56:21PM -0800, Andrew Morton wrote:
> On Thu, 11 Jan 2024 18:22:50 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> 
> > On Thu, Jan 11, 2024 at 10:45:53PM +0000, Matthew Wilcox wrote:
> > > On Thu, Jan 11, 2024 at 02:00:53PM -0800, Andrew Morton wrote:
> > > > On Wed, 10 Jan 2024 12:04:51 -0800 "Darrick J. Wong" <djwong@kernel.org> wrote:
> > > > 
> > > > > > > Fixing this will require a bit of an API change, and prefeably sorting out
> > > > > > > the hwpoison story for pages vs folio and where it is placed in the shmem
> > > > > > > API.  For now use this one liner to disable large folios.
> > > > > > > 
> > > > > > > Reported-by: Darrick J. Wong <djwong@kernel.org>
> > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > > > > 
> > > > > > Can someone who knows more about shmem.c than I do please review
> > > > > > https://lore.kernel.org/linux-xfs/20240103084126.513354-4-hch@lst.de/
> > > > > > so that I can feel slightly more confident as hch and I sort through the
> > > > > > xfile.c issues?
> > > > > > 
> > > > > > For this patch,
> > > > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > > > 
> > > > > ...except that I'm still getting 2M THPs even with this enabled, so I
> > > > > guess either we get to fix it now, or create our own private tmpfs mount
> > > > > so that we can pass in huge=never, similar to what i915 does. :(
> > > > 
> > > > What is "this"?  Are you saying that $Subject doesn't work, or that the
> > > > above-linked please-review patch doesn't work?
> > > 
> > > shmem pays no attention to the mapping_large_folio_support() flag,
> > > so the proposed fix doesn't work.  It ought to, but it has its own way
> > > of doing it that predates mapping_large_folio_support existing.
> > 
> > Yep.  It turned out to be easier to fix xfile.c to deal with large
> > folios than I thought it would be.  Or so I think.  We'll see what
> > happens on fstestscloud overnight.
> 
> Where do we stand with this?  Should I merge these two patches into
> 6.8-rcX, cc:stable?

This patchset doesn't actually fix the problem, so no, let's not merge
it.

For 6.9 we'll make xfile.c clean w.r.t. large folios:
https://lore.kernel.org/linux-xfs/20240129143502.189370-1-hch@lst.de/

I don't think we need a 6.8 backport since xfile.c is only used by an
experimental feature that is default n in kconfig.

--D