On 09.08.24 12:31, Usama Arif wrote:
On 08/08/2024 16:55, David Hildenbrand wrote:
On 07.08.24 15:46, Usama Arif wrote:
This is an attempt to mitigate the issue of running out of memory when THP
is always enabled. During runtime whenever a THP is being faulted in
(__do_huge_pmd_anonymous_page) or collapsed by khugepaged
(collapse_huge_page), the THP is added to _deferred_list. Whenever memory
reclaim happens in linux, the kernel runs the deferred_split
shrinker which goes through the _deferred_list.
If the folio was partially mapped, the shrinker attempts to split it.
A new boolean is added to be able to distinguish between partially
mapped folios and others in the deferred_list at split time in
deferred_split_scan. Its needed as __folio_remove_rmap decrements
the folio mapcount elements, hence it won't be possible to distinguish
between partially mapped folios and others in deferred_split_scan
without the boolean.
Just so I get this right: Are you saying that we might now add fully mapped folios to the deferred split queue and that's what you want to distinguish?
Yes
If that's the case, then could we use a bit in folio->_flags_1 instead?
Yes, thats a good idea. Will create the below flag for the next revision
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 5769fe6e4950..5825bd1cf6db 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -189,6 +189,11 @@ enum pageflags {
#define PAGEFLAGS_MASK ((1UL << NR_PAGEFLAGS) - 1)
+enum folioflags_1 {
+ /* The first 8 bits of folio->_flags_1 are used to keep track of folio order */
+ FOLIO_PARTIALLY_MAPPED = 8, /* folio is partially mapped */
+}
This might be what you want to achieve:
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index a0a29bd092f8..d4722ed60ef8 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -182,6 +182,7 @@ enum pageflags {
/* At least one page in this folio has the hwpoison flag set */
PG_has_hwpoisoned = PG_active,
PG_large_rmappable = PG_workingset, /* anon or file-backed */
+ PG_partially_mapped, /* was identified to be partially mapped */
};
#define PAGEFLAGS_MASK ((1UL << NR_PAGEFLAGS) - 1)
@@ -861,8 +862,9 @@ static inline void ClearPageCompound(struct page *page)
ClearPageHead(page);
}
FOLIO_FLAG(large_rmappable, FOLIO_SECOND_PAGE)
+FOLIO_FLAG(partially_mapped, FOLIO_SECOND_PAGE)
#else
-FOLIO_FLAG_FALSE(large_rmappable)
+FOLIO_FLAG_FALSE(partially_mapped)
#endif
#define PG_head_mask ((1UL << PG_head))
The downside is an atomic op to set/clear, but it should likely not really matter
(initially, the flag will be clear, and we should only ever set it once when
partially unmapping). If it hurts, we can reconsider.
[...]
I would actually suggest to split decoupling of "_deferred_list" and "partially mapped" into a separate preparation patch.
Yes, will do. I will split it into 3 patches, 1st one that introduces FOLIO_PARTIALLY_MAPPED and sets/clear it in the right place without introducing any functional change, 2nd to split underutilized THPs and 3rd to add sysfs entry to enable/disable the shrinker. Should make the patches quite small and easy to review.
Great! As always, please shout if you disagree with something I propose :)
--
Cheers,
David / dhildenb
