On 2024/8/26 07:14, Hugh Dickins wrote:
On Mon, 12 Aug 2024, Baolin Wang wrote:
Shmem will support large folio allocation [1] [2] to get a better performance,
however, the memory reclaim still splits the precious large folios when trying
to swap out shmem, which may lead to the memory fragmentation issue and can not
take advantage of the large folio for shmeme.
Moreover, the swap code already supports for swapping out large folio without
split, hence this patch set supports the large folio swap out for shmem.
Note the i915_gem_shmem driver still need to be split when swapping, thus
add a new flag 'split_large_folio' for writeback_control to indicate spliting
the large folio.
Is that last paragraph a misunderstanding? The i915 THP splitting in
shmem_writepage() was to avoid mm VM_BUG_ONs and crashes when shmem.c
did not support huge page swapout: but now you are enabling that support,
and such VM_BUG_ONs and crashes are gone (so far as I can see: and this
is written on a laptop using the i915 driver).
Thanks for the history, and I understand.
I cannot think of why i915 itself would care how mm implements swapout
(beyond enjoying faster): I think all the wbc->split_large_folio you
introduce here should be reverted. But you may know better!
I do need a further change to shmem_writepage() here: see fixup patch
below: that's written to apply on top of this 9/9, but I'd be glad to
see a replacement with wbc->split_large_folio gone, and just one
!IS_ENABLED(CONFIG_THP_SWAP) instead.
Sure. After Andrew queuing your fixes, I can send a proper fix patch to
remove the 'wbc->split_large_folio'.
[1] https://lore.kernel.org/all/cover.1717495894.git.baolin.wang@xxxxxxxxxxxxxxxxx/
[2] https://lore.kernel.org/all/20240515055719.32577-1-da.gomez@xxxxxxxxxxx/
I get "Not found" for that [2] link.
Weird, I can access the link, not sure why.
Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 1 +
include/linux/writeback.h | 4 +++
mm/shmem.c | 12 ++++++---
mm/vmscan.c | 32 ++++++++++++++++++-----
4 files changed, 38 insertions(+), 11 deletions(-)
[PATCH] mm: shmem: shmem_writepage() split folio at EOF before swapout
Working in a constrained (size= or nr_blocks=) huge=always tmpfs relies
on swapout to split a large folio at EOF, to trim off its excess before
hitting premature ENOSPC: shmem_unused_huge_shrink() contains no code to
handle splitting huge swap blocks, and nobody would want that to be added.
Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
---
mm/shmem.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/mm/shmem.c b/mm/shmem.c
index 37c300f69baf..4dd0570962fa 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1459,6 +1459,7 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc)
swp_entry_t swap;
pgoff_t index;
int nr_pages;
+ bool split = false;
/*
* Our capabilities prevent regular writeback or sync from ever calling
@@ -1480,8 +1481,20 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc)
* If /sys/kernel/mm/transparent_hugepage/shmem_enabled is "always" or
* "force", drivers/gpu/drm/i915/gem/i915_gem_shmem.c gets huge pages,
* and its shmem_writeback() needs them to be split when swapping.
+ *
+ * And shrinkage of pages beyond i_size does not split swap, so
+ * swapout of a large folio crossing i_size needs to split too
+ * (unless fallocate has been used to preallocate beyond EOF).
*/
- if (wbc->split_large_folio && folio_test_large(folio)) {
+ if (folio_test_large(folio)) {
+ split = wbc->split_large_folio;
+ index = shmem_fallocend(inode,
+ DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE));
+ if (index > folio->index && index < folio_next_index(folio))
+ split = true;
+ }
+
+ if (split) {
try_split:
/* Ensure the subpages are still dirty */
folio_test_set_dirty(folio);
Thanks for the fix, Hugh. Very appreciated for your reviewing.
