On 04.08.24 21:02, Usama Arif wrote:
On 30/07/2024 16:14, David Hildenbrand wrote:
On 30.07.24 14:46, Usama Arif wrote:
From: Yu Zhao <yuzhao@xxxxxxxxxx>
If a tail page has only two references left, one inherited from the
isolation of its head and the other from lru_add_page_tail() which we
are about to drop, it means this tail page was concurrently zapped.
Then we can safely free it and save page reclaim or migration the
trouble of trying it.
Signed-off-by: Yu Zhao <yuzhao@xxxxxxxxxx>
Tested-by: Shuang Zhai <zhais@xxxxxxxxxx>
Signed-off-by: Usama Arif <usamaarif642@xxxxxxxxx>
---
mm/huge_memory.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 0167dc27e365..76a3b6a2b796 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2923,6 +2923,8 @@ static void __split_huge_page(struct page *page, struct list_head *list,
unsigned int new_nr = 1 << new_order;
int order = folio_order(folio);
unsigned int nr = 1 << order;
+ LIST_HEAD(pages_to_free);
+ int nr_pages_to_free = 0;
/* complete memcg works before add pages to LRU */
split_page_memcg(head, order, new_order);
@@ -3007,6 +3009,24 @@ static void __split_huge_page(struct page *page, struct list_head *list,
if (subpage == page)
continue;
folio_unlock(new_folio);
+ /*
+ * If a tail page has only two references left, one inherited
+ * from the isolation of its head and the other from
+ * lru_add_page_tail() which we are about to drop, it means this
+ * tail page was concurrently zapped. Then we can safely free it
+ * and save page reclaim or migration the trouble of trying it.
+ */
+ if (list && page_ref_freeze(subpage, 2)) {
+ VM_BUG_ON_PAGE(PageLRU(subpage), subpage);
+ VM_BUG_ON_PAGE(PageCompound(subpage), subpage);
+ VM_BUG_ON_PAGE(page_mapped(subpage), subpage);
+
No VM_BUG_*, VM_WARN is good enough.
+ ClearPageActive(subpage);
+ ClearPageUnevictable(subpage);
+ list_move(&subpage->lru, &pages_to_free);
Most checks here should operate on new_folio instead of subpage.
Do you mean instead of doing the PageLRU, PageCompound and page_mapped check on the subpage, there should be checks on new_folio?
If new_folio is a large folio, then it could be that only some of the subpages were zapped?
We do a:
struct folio *new_folio = page_folio(subpage);
Then:
PageLRU() will end up getting translated to
folio_test_lru(page_folio(subpage))
page_mapped() will end up getting translated to
folio_mapped(page_folio(subpage))
PageCompound() is essentially a folio_test_large() check.
So what stops us from doing
VM_WARN_ON_ONCE_FOLIO(folio_test_lru(new_folio), new_folio);
VM_WARN_ON_ONCE_FOLIO(folio_test_large(new_folio), new_folio);
VM_WARN_ON_ONCE_FOLIO(folio_mapped(new_folio), new_folio);
folio_clear_active(new_folio);
folio_clear_unevictable(new_folio);
...
?
The page_ref_freeze() should make sure that we don't have a tail page of
a large folio. Tail pages would have a refcount of 0.
Or what am I missing?
Could do below if subpage makes sense
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 3305e6d0b90e..abfcd4b7cbba 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3041,9 +3041,9 @@ static void __split_huge_page(struct page *page, struct list_head *list,
* and save page reclaim or migration the trouble of trying it.
*/
if (list && page_ref_freeze(subpage, 2)) {
- VM_BUG_ON_PAGE(PageLRU(subpage), subpage);
- VM_BUG_ON_PAGE(PageCompound(subpage), subpage);
- VM_BUG_ON_PAGE(page_mapped(subpage), subpage);
+ VM_WARN_ON_ONCE_PAGE(PageLRU(subpage), subpage);
+ VM_WARN_ON_ONCE_PAGE(PageCompound(subpage), subpage);
+ VM_WARN_ON_ONCE_PAGE(page_mapped(subpage), subpage);
ClearPageActive(subpage);
ClearPageUnevictable(subpage);
--
Cheers,
David / dhildenb
