On 06.03.24 10:52, Barry Song wrote:
From: Barry Song <v-songbaohua@xxxxxxxx>
Within try_to_unmap_one(), page_vma_mapped_walk() races with other
PTE modifications preceded by pte clear. While iterating over PTEs
of a large folio, it only starts acquiring PTL from the first valid
(present) PTE. PTE modifications can temporarily set PTEs to
pte_none. Consequently, the initial PTEs of a large folio might
be skipped in try_to_unmap_one().
For example, for an anon folio, if we skip PTE0, we may have PTE0
which is still present, while PTE1 ~ PTE(nr_pages - 1) are swap
entries after try_to_unmap_one().
So folio will be still mapped, the folio fails to be reclaimed
and is put back to LRU in this round.
This also breaks up PTEs optimization such as CONT-PTE on this
large folio and may lead to accident folio_split() afterwards.
And since a part of PTEs are now swap entries, accessing those
parts will introduce overhead - do_swap_page.
Although the kernel can withstand all of the above issues, the
situation still seems quite awkward and warrants making it more
ideal.
The same race also occurs with small folios, but they have only
one PTE, thus, it won't be possible for them to be partially
unmapped.
This patch holds PTL from PTE0, allowing us to avoid reading PTE
values that are in the process of being transformed. With stable
PTE values, we can ensure that this large folio is either
completely reclaimed or that all PTEs remain untouched in this
round.
A corner case is that if we hold PTL from PTE0 and most initial
PTEs have been really unmapped before that, we may increase the
duration of holding PTL. Thus we only apply this optimization to
folios which are still entirely mapped (not in deferred_split
list).
Cc: Hugh Dickins <hughd@xxxxxxxxxx>
Signed-off-by: Barry Song <v-songbaohua@xxxxxxxx>
---
v2:
* Refine commit message and code comment after reading all comments
from Ryan and David, thanks!
* Avoid increasing the duration of PTL by applying optimization
on folios not in deferred_split_list with respect to Ying's
comment, thanks!
mm/vmscan.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 0b888a2afa58..7106741387e8 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1270,6 +1270,18 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
if (folio_test_pmd_mappable(folio))
flags |= TTU_SPLIT_HUGE_PMD;
+ /*
+ * Without TTU_SYNC, try_to_unmap will only begin to hold PTL
+ * from the first present PTE within a large folio. Some initial
+ * PTEs might be skipped due to races with parallel PTE writes
+ * in which PTEs can be cleared temporarily before being written
+ * new present values. This will lead to a large folio is still
+ * mapped while some subpages have been partially unmapped after
+ * try_to_unmap; TTU_SYNC helps try_to_unmap acquire PTL from the
+ * first PTE, eliminating the influence of temporary PTE values.
+ */
+ if (folio_test_large(folio) && list_empty(&folio->_deferred_list))
+ flags |= TTU_SYNC;
try_to_unmap(folio, flags);
if (folio_mapped(folio)) {
Hopefully this won't have unexpected performance "surprises".
I do wonder if we should really care about the "_deferred_list"
optimization here, though, I'd just drop it.
In any case
Acked-by: David Hildenbrand <david@xxxxxxxxxx>
--
Cheers,
David / dhildenb
