All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: akpm@linux-foundation.org, hughd@google.com, willy@infradead.org,
	 david@redhat.com, wangkefeng.wang@huawei.com, chrisl@kernel.org,
	 ying.huang@intel.com, 21cnbao@gmail.com, ryan.roberts@arm.com,
	 shy828301@gmail.com, ziy@nvidia.com, ioworker0@gmail.com,
	 da.gomez@samsung.com, p.raghav@samsung.com, linux-mm@kvack.org,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/7] support large folio swap-out and swap-in for shmem
Date: Tue, 11 Jun 2024 22:46:34 -0700 (PDT)	[thread overview]
Message-ID: <7477de0e-e5bb-529d-e2c0-cabd0d39fb5c@google.com> (raw)
In-Reply-To: <cover.1717673614.git.baolin.wang@linux.alibaba.com>

On Thu, 6 Jun 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, and large folio swap-in[3] series is queued into mm-unstable branch.
> Hence this patch set also supports the large folio swap-out and swap-in for
> shmem.
> 
> [1] https://lore.kernel.org/all/cover.1717495894.git.baolin.wang@linux.alibaba.com/
> [2] https://lore.kernel.org/all/20240515055719.32577-1-da.gomez@samsung.com/
> [3] https://lore.kernel.org/all/20240508224040.190469-6-21cnbao@gmail.com/T/
> 
> Changes from RFC:
>  - Rebased to the latest mm-unstable.
>  - Drop the counter name fixing patch, which was queued into mm-hotfixes-stable
>  branch.
> 
> Baolin Wang (7):
>   mm: vmscan: add validation before spliting shmem large folio
>   mm: swap: extend swap_shmem_alloc() to support batch SWAP_MAP_SHMEM
>     flag setting
>   mm: shmem: support large folio allocation for shmem_replace_folio()
>   mm: shmem: extend shmem_partial_swap_usage() to support large folio
>     swap
>   mm: add new 'orders' parameter for find_get_entries() and
>     find_lock_entries()
>   mm: shmem: use swap_free_nr() to free shmem swap entries
>   mm: shmem: support large folio swap out
> 
>  drivers/gpu/drm/i915/gem/i915_gem_shmem.c |  1 +
>  include/linux/swap.h                      |  4 +-
>  include/linux/writeback.h                 |  1 +
>  mm/filemap.c                              | 27 ++++++-
>  mm/internal.h                             |  4 +-
>  mm/shmem.c                                | 58 ++++++++------
>  mm/swapfile.c                             | 98 ++++++++++++-----------
>  mm/truncate.c                             |  8 +-
>  mm/vmscan.c                               | 22 ++++-
>  9 files changed, 140 insertions(+), 83 deletions(-)

I wanted to have some tests running, while looking through these
and your shmem mTHP patches; but I wasted too much time on that by
applying these on top and hitting crash, OOMs and dreadful thrashing -
testing did not get very far at all.

Perhaps all easily fixed, but I don't have more time to spend on it,
and think this series cannot expect to go into 6.11: I'll have another
try with it next cycle.

I really must turn my attention to your shmem mTHP series: no doubt
I'll have minor adjustments to ask there - but several other people
are also waiting for me to respond (or given up on me completely).

The little crash fix needed in this series appears to be:

--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2053,7 +2053,8 @@ static int shmem_swapin_folio(struct ino
 			goto failed;
 	}
 
-	error = shmem_add_to_page_cache(folio, mapping, index,
+	error = shmem_add_to_page_cache(folio, mapping,
+					round_down(index, nr_pages),
 					swp_to_radix_entry(swap), gfp);
 	if (error)
 		goto failed;

Then the OOMs and dreadful thrashing are due to refcount confusion:
I did not even glance at these patches to work out what's wanted,
but a printk in __remove_mapping() showed that folio->_refcount was
1024 where 513 was expected, so reclaim was freeing none of them.

Hugh


  parent reply	other threads:[~2024-06-12  5:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-06 11:58 [PATCH 0/7] support large folio swap-out and swap-in for shmem Baolin Wang
2024-06-06 11:58 ` [PATCH 1/7] mm: vmscan: add validation before spliting shmem large folio Baolin Wang
2024-06-06 11:58 ` [PATCH 2/7] mm: swap: extend swap_shmem_alloc() to support batch SWAP_MAP_SHMEM flag setting Baolin Wang
2024-06-06 11:58 ` [PATCH 3/7] mm: shmem: support large folio allocation for shmem_replace_folio() Baolin Wang
2024-06-06 11:58 ` [PATCH 4/7] mm: shmem: extend shmem_partial_swap_usage() to support large folio swap Baolin Wang
2024-06-10 14:53   ` Daniel Gomez
2024-06-11  2:59     ` Baolin Wang
2024-06-06 11:58 ` [PATCH 5/7] mm: add new 'orders' parameter for find_get_entries() and find_lock_entries() Baolin Wang
2024-06-10 15:23   ` Daniel Gomez
2024-06-11  3:31     ` Baolin Wang
2024-06-10 16:59   ` Matthew Wilcox
2024-06-11  3:38     ` Baolin Wang
2024-06-06 11:58 ` [PATCH 6/7] mm: shmem: use swap_free_nr() to free shmem swap entries Baolin Wang
2024-06-06 11:58 ` [PATCH 7/7] mm: shmem: support large folio swap out Baolin Wang
2024-06-12  5:46 ` Hugh Dickins [this message]
2024-06-12  6:23   ` [PATCH 0/7] support large folio swap-out and swap-in for shmem Baolin Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7477de0e-e5bb-529d-e2c0-cabd0d39fb5c@google.com \
    --to=hughd@google.com \
    --cc=21cnbao@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=chrisl@kernel.org \
    --cc=da.gomez@samsung.com \
    --cc=david@redhat.com \
    --cc=ioworker0@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=p.raghav@samsung.com \
    --cc=ryan.roberts@arm.com \
    --cc=shy828301@gmail.com \
    --cc=wangkefeng.wang@huawei.com \
    --cc=willy@infradead.org \
    --cc=ying.huang@intel.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.