* [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-21 16:08 [PATCH mm-unstable v1 0/4] mm/swap: stop using page->private on tail pages for THP_SWAP + cleanups David Hildenbrand
@ 2023-08-21 16:08 ` David Hildenbrand
2023-08-22 16:24 ` Catalin Marinas
` (3 more replies)
2023-08-21 16:08 ` [PATCH mm-unstable v1 2/4] mm/swap: use dedicated entry for swap in folio David Hildenbrand
` (2 subsequent siblings)
3 siblings, 4 replies; 17+ messages in thread
From: David Hildenbrand @ 2023-08-21 16:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linux-arm-kernel, David Hildenbrand, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
Let's stop using page->private on tail pages, making it possible to
just unconditionally reuse that field in the tail pages of large folios.
The remaining usage of the private field for THP_SWAP is in the THP
splitting code (mm/huge_memory.c), that we'll handle separately later.
Update the THP_SWAP documentation and sanity checks in mm_types.h and
__split_huge_page_tail().
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/arm64/mm/mteswap.c | 5 +++--
include/linux/mm_types.h | 12 +-----------
include/linux/swap.h | 9 +++++++++
mm/huge_memory.c | 15 ++++++---------
mm/memory.c | 2 +-
mm/rmap.c | 2 +-
mm/swap_state.c | 5 +++--
mm/swapfile.c | 4 ++--
8 files changed, 26 insertions(+), 28 deletions(-)
diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
index cd508ba80ab1..a31833e3ddc5 100644
--- a/arch/arm64/mm/mteswap.c
+++ b/arch/arm64/mm/mteswap.c
@@ -33,8 +33,9 @@ int mte_save_tags(struct page *page)
mte_save_page_tags(page_address(page), tag_storage);
- /* page_private contains the swap entry.val set in do_swap_page */
- ret = xa_store(&mte_pages, page_private(page), tag_storage, GFP_KERNEL);
+ /* lookup the swap entry.val from the page */
+ ret = xa_store(&mte_pages, page_swap_entry(page).val, tag_storage,
+ GFP_KERNEL);
if (WARN(xa_is_err(ret), "Failed to store MTE tags")) {
mte_free_tag_storage(tag_storage);
return xa_err(ret);
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index b9b6c88875b9..61361f1750c3 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -333,11 +333,8 @@ struct folio {
atomic_t _pincount;
#ifdef CONFIG_64BIT
unsigned int _folio_nr_pages;
- /* 4 byte gap here */
- /* private: the union with struct page is transitional */
- /* Fix THP_SWAP to not use tail->private */
- unsigned long _private_1;
#endif
+ /* private: the union with struct page is transitional */
};
struct page __page_1;
};
@@ -358,9 +355,6 @@ struct folio {
/* public: */
struct list_head _deferred_list;
/* private: the union with struct page is transitional */
- unsigned long _avail_2a;
- /* Fix THP_SWAP to not use tail->private */
- unsigned long _private_2a;
};
struct page __page_2;
};
@@ -385,9 +379,6 @@ FOLIO_MATCH(memcg_data, memcg_data);
offsetof(struct page, pg) + sizeof(struct page))
FOLIO_MATCH(flags, _flags_1);
FOLIO_MATCH(compound_head, _head_1);
-#ifdef CONFIG_64BIT
-FOLIO_MATCH(private, _private_1);
-#endif
#undef FOLIO_MATCH
#define FOLIO_MATCH(pg, fl) \
static_assert(offsetof(struct folio, fl) == \
@@ -396,7 +387,6 @@ FOLIO_MATCH(flags, _flags_2);
FOLIO_MATCH(compound_head, _head_2);
FOLIO_MATCH(flags, _flags_2a);
FOLIO_MATCH(compound_head, _head_2a);
-FOLIO_MATCH(private, _private_2a);
#undef FOLIO_MATCH
/**
diff --git a/include/linux/swap.h b/include/linux/swap.h
index bb5adc604144..84fe0e94f5cd 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio)
return entry;
}
+static inline swp_entry_t page_swap_entry(struct page *page)
+{
+ struct folio *folio = page_folio(page);
+ swp_entry_t entry = folio_swap_entry(folio);
+
+ entry.val += page - &folio->page;
+ return entry;
+}
+
static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
{
folio->private = (void *)entry.val;
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index cc2f65f8cc62..c04702ae71d2 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct page *head, int tail,
page_tail->index = head->index + tail;
/*
- * page->private should not be set in tail pages with the exception
- * of swap cache pages that store the swp_entry_t in tail pages.
- * Fix up and warn once if private is unexpectedly set.
- *
- * What of 32-bit systems, on which folio->_pincount overlays
- * head[1].private? No problem: THP_SWAP is not enabled on 32-bit, and
- * pincount must be 0 for folio_ref_freeze() to have succeeded.
+ * page->private should not be set in tail pages. Fix up and warn once
+ * if private is unexpectedly set.
*/
- if (!folio_test_swapcache(page_folio(head))) {
- VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail);
+ if (unlikely(page_tail->private)) {
+ VM_WARN_ON_ONCE_PAGE(true, page_tail);
page_tail->private = 0;
}
+ if (PageSwapCache(head))
+ set_page_private(page_tail, (unsigned long)head->private + tail);
/* Page flags must be visible before we make the page non-compound. */
smp_wmb();
diff --git a/mm/memory.c b/mm/memory.c
index d003076b218d..ff13242c1589 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3882,7 +3882,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
* changed.
*/
if (unlikely(!folio_test_swapcache(folio) ||
- page_private(page) != entry.val))
+ page_swap_entry(page).val != entry.val))
goto out_page;
/*
diff --git a/mm/rmap.c b/mm/rmap.c
index 1f04debdc87a..ec7f8e6c9e48 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1647,7 +1647,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
*/
dec_mm_counter(mm, mm_counter(&folio->page));
} else if (folio_test_anon(folio)) {
- swp_entry_t entry = { .val = page_private(subpage) };
+ swp_entry_t entry = page_swap_entry(subpage);
pte_t swp_pte;
/*
* Store the swap location in the pte.
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 01f15139b7d9..2f2417810052 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -100,6 +100,7 @@ int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
folio_ref_add(folio, nr);
folio_set_swapcache(folio);
+ folio_set_swap_entry(folio, entry);
do {
xas_lock_irq(&xas);
@@ -113,7 +114,6 @@ int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
if (shadowp)
*shadowp = old;
}
- set_page_private(folio_page(folio, i), entry.val + i);
xas_store(&xas, folio);
xas_next(&xas);
}
@@ -154,9 +154,10 @@ void __delete_from_swap_cache(struct folio *folio,
for (i = 0; i < nr; i++) {
void *entry = xas_store(&xas, shadow);
VM_BUG_ON_PAGE(entry != folio, entry);
- set_page_private(folio_page(folio, i), 0);
xas_next(&xas);
}
+ entry.val = 0;
+ folio_set_swap_entry(folio, entry);
folio_clear_swapcache(folio);
address_space->nrpages -= nr;
__node_stat_mod_folio(folio, NR_FILE_PAGES, -nr);
diff --git a/mm/swapfile.c b/mm/swapfile.c
index d46933adf789..bd9d904671b9 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -3369,7 +3369,7 @@ struct swap_info_struct *swp_swap_info(swp_entry_t entry)
struct swap_info_struct *page_swap_info(struct page *page)
{
- swp_entry_t entry = { .val = page_private(page) };
+ swp_entry_t entry = page_swap_entry(page);
return swp_swap_info(entry);
}
@@ -3384,7 +3384,7 @@ EXPORT_SYMBOL_GPL(swapcache_mapping);
pgoff_t __page_file_index(struct page *page)
{
- swp_entry_t swap = { .val = page_private(page) };
+ swp_entry_t swap = page_swap_entry(page);
return swp_offset(swap);
}
EXPORT_SYMBOL_GPL(__page_file_index);
--
2.41.0
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-21 16:08 ` [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP David Hildenbrand
@ 2023-08-22 16:24 ` Catalin Marinas
2023-08-22 17:00 ` Yosry Ahmed
` (2 subsequent siblings)
3 siblings, 0 replies; 17+ messages in thread
From: Catalin Marinas @ 2023-08-22 16:24 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Will Deacon, Hugh Dickins,
Seth Jennings, Dan Streetman, Vitaly Wool
On Mon, Aug 21, 2023 at 06:08:46PM +0200, David Hildenbrand wrote:
> diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
> index cd508ba80ab1..a31833e3ddc5 100644
> --- a/arch/arm64/mm/mteswap.c
> +++ b/arch/arm64/mm/mteswap.c
> @@ -33,8 +33,9 @@ int mte_save_tags(struct page *page)
>
> mte_save_page_tags(page_address(page), tag_storage);
>
> - /* page_private contains the swap entry.val set in do_swap_page */
> - ret = xa_store(&mte_pages, page_private(page), tag_storage, GFP_KERNEL);
> + /* lookup the swap entry.val from the page */
> + ret = xa_store(&mte_pages, page_swap_entry(page).val, tag_storage,
> + GFP_KERNEL);
> if (WARN(xa_is_err(ret), "Failed to store MTE tags")) {
> mte_free_tag_storage(tag_storage);
> return xa_err(ret);
For arm64:
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-21 16:08 ` [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP David Hildenbrand
2023-08-22 16:24 ` Catalin Marinas
@ 2023-08-22 17:00 ` Yosry Ahmed
2023-08-22 17:14 ` David Hildenbrand
2023-08-23 12:15 ` David Hildenbrand
2023-08-23 15:12 ` Yosry Ahmed
3 siblings, 1 reply; 17+ messages in thread
From: Yosry Ahmed @ 2023-08-22 17:00 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
>
> Let's stop using page->private on tail pages, making it possible to
> just unconditionally reuse that field in the tail pages of large folios.
>
> The remaining usage of the private field for THP_SWAP is in the THP
> splitting code (mm/huge_memory.c), that we'll handle separately later.
>
> Update the THP_SWAP documentation and sanity checks in mm_types.h and
> __split_huge_page_tail().
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> arch/arm64/mm/mteswap.c | 5 +++--
> include/linux/mm_types.h | 12 +-----------
> include/linux/swap.h | 9 +++++++++
> mm/huge_memory.c | 15 ++++++---------
> mm/memory.c | 2 +-
> mm/rmap.c | 2 +-
> mm/swap_state.c | 5 +++--
> mm/swapfile.c | 4 ++--
> 8 files changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
> index cd508ba80ab1..a31833e3ddc5 100644
> --- a/arch/arm64/mm/mteswap.c
> +++ b/arch/arm64/mm/mteswap.c
> @@ -33,8 +33,9 @@ int mte_save_tags(struct page *page)
>
> mte_save_page_tags(page_address(page), tag_storage);
>
> - /* page_private contains the swap entry.val set in do_swap_page */
> - ret = xa_store(&mte_pages, page_private(page), tag_storage, GFP_KERNEL);
> + /* lookup the swap entry.val from the page */
> + ret = xa_store(&mte_pages, page_swap_entry(page).val, tag_storage,
> + GFP_KERNEL);
> if (WARN(xa_is_err(ret), "Failed to store MTE tags")) {
> mte_free_tag_storage(tag_storage);
> return xa_err(ret);
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index b9b6c88875b9..61361f1750c3 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -333,11 +333,8 @@ struct folio {
> atomic_t _pincount;
> #ifdef CONFIG_64BIT
> unsigned int _folio_nr_pages;
> - /* 4 byte gap here */
> - /* private: the union with struct page is transitional */
> - /* Fix THP_SWAP to not use tail->private */
> - unsigned long _private_1;
> #endif
> + /* private: the union with struct page is transitional */
> };
> struct page __page_1;
> };
> @@ -358,9 +355,6 @@ struct folio {
> /* public: */
> struct list_head _deferred_list;
> /* private: the union with struct page is transitional */
> - unsigned long _avail_2a;
> - /* Fix THP_SWAP to not use tail->private */
> - unsigned long _private_2a;
> };
> struct page __page_2;
> };
> @@ -385,9 +379,6 @@ FOLIO_MATCH(memcg_data, memcg_data);
> offsetof(struct page, pg) + sizeof(struct page))
> FOLIO_MATCH(flags, _flags_1);
> FOLIO_MATCH(compound_head, _head_1);
> -#ifdef CONFIG_64BIT
> -FOLIO_MATCH(private, _private_1);
> -#endif
> #undef FOLIO_MATCH
> #define FOLIO_MATCH(pg, fl) \
> static_assert(offsetof(struct folio, fl) == \
> @@ -396,7 +387,6 @@ FOLIO_MATCH(flags, _flags_2);
> FOLIO_MATCH(compound_head, _head_2);
> FOLIO_MATCH(flags, _flags_2a);
> FOLIO_MATCH(compound_head, _head_2a);
> -FOLIO_MATCH(private, _private_2a);
> #undef FOLIO_MATCH
>
> /**
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index bb5adc604144..84fe0e94f5cd 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio)
> return entry;
> }
>
> +static inline swp_entry_t page_swap_entry(struct page *page)
> +{
> + struct folio *folio = page_folio(page);
> + swp_entry_t entry = folio_swap_entry(folio);
> +
> + entry.val += page - &folio->page;
Would it be better to use folio_page_idx() here?
> + return entry;
> +}
> +
> static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
> {
> folio->private = (void *)entry.val;
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index cc2f65f8cc62..c04702ae71d2 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct page *head, int tail,
> page_tail->index = head->index + tail;
>
> /*
> - * page->private should not be set in tail pages with the exception
> - * of swap cache pages that store the swp_entry_t in tail pages.
> - * Fix up and warn once if private is unexpectedly set.
> - *
> - * What of 32-bit systems, on which folio->_pincount overlays
> - * head[1].private? No problem: THP_SWAP is not enabled on 32-bit, and
> - * pincount must be 0 for folio_ref_freeze() to have succeeded.
> + * page->private should not be set in tail pages. Fix up and warn once
> + * if private is unexpectedly set.
> */
> - if (!folio_test_swapcache(page_folio(head))) {
> - VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail);
> + if (unlikely(page_tail->private)) {
> + VM_WARN_ON_ONCE_PAGE(true, page_tail);
> page_tail->private = 0;
> }
> + if (PageSwapCache(head))
> + set_page_private(page_tail, (unsigned long)head->private + tail);
>
> /* Page flags must be visible before we make the page non-compound. */
> smp_wmb();
> diff --git a/mm/memory.c b/mm/memory.c
> index d003076b218d..ff13242c1589 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3882,7 +3882,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> * changed.
> */
> if (unlikely(!folio_test_swapcache(folio) ||
> - page_private(page) != entry.val))
> + page_swap_entry(page).val != entry.val))
> goto out_page;
>
> /*
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 1f04debdc87a..ec7f8e6c9e48 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1647,7 +1647,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> */
> dec_mm_counter(mm, mm_counter(&folio->page));
> } else if (folio_test_anon(folio)) {
> - swp_entry_t entry = { .val = page_private(subpage) };
> + swp_entry_t entry = page_swap_entry(subpage);
> pte_t swp_pte;
> /*
> * Store the swap location in the pte.
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 01f15139b7d9..2f2417810052 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -100,6 +100,7 @@ int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
>
> folio_ref_add(folio, nr);
> folio_set_swapcache(folio);
> + folio_set_swap_entry(folio, entry);
>
> do {
> xas_lock_irq(&xas);
> @@ -113,7 +114,6 @@ int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
> if (shadowp)
> *shadowp = old;
> }
> - set_page_private(folio_page(folio, i), entry.val + i);
> xas_store(&xas, folio);
> xas_next(&xas);
> }
> @@ -154,9 +154,10 @@ void __delete_from_swap_cache(struct folio *folio,
> for (i = 0; i < nr; i++) {
> void *entry = xas_store(&xas, shadow);
> VM_BUG_ON_PAGE(entry != folio, entry);
> - set_page_private(folio_page(folio, i), 0);
> xas_next(&xas);
> }
> + entry.val = 0;
> + folio_set_swap_entry(folio, entry);
> folio_clear_swapcache(folio);
> address_space->nrpages -= nr;
> __node_stat_mod_folio(folio, NR_FILE_PAGES, -nr);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index d46933adf789..bd9d904671b9 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3369,7 +3369,7 @@ struct swap_info_struct *swp_swap_info(swp_entry_t entry)
>
> struct swap_info_struct *page_swap_info(struct page *page)
> {
> - swp_entry_t entry = { .val = page_private(page) };
> + swp_entry_t entry = page_swap_entry(page);
> return swp_swap_info(entry);
> }
>
> @@ -3384,7 +3384,7 @@ EXPORT_SYMBOL_GPL(swapcache_mapping);
>
> pgoff_t __page_file_index(struct page *page)
> {
> - swp_entry_t swap = { .val = page_private(page) };
> + swp_entry_t swap = page_swap_entry(page);
> return swp_offset(swap);
> }
> EXPORT_SYMBOL_GPL(__page_file_index);
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-22 17:00 ` Yosry Ahmed
@ 2023-08-22 17:14 ` David Hildenbrand
0 siblings, 0 replies; 17+ messages in thread
From: David Hildenbrand @ 2023-08-22 17:14 UTC (permalink / raw)
To: Yosry Ahmed
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
>>
>> +static inline swp_entry_t page_swap_entry(struct page *page)
>> +{
>> + struct folio *folio = page_folio(page);
>> + swp_entry_t entry = folio_swap_entry(folio);
>> +
>> + entry.val += page - &folio->page;
>
> Would it be better to use folio_page_idx() here?
Sounds reasonable!
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-21 16:08 ` [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP David Hildenbrand
2023-08-22 16:24 ` Catalin Marinas
2023-08-22 17:00 ` Yosry Ahmed
@ 2023-08-23 12:15 ` David Hildenbrand
2023-08-23 15:12 ` Yosry Ahmed
3 siblings, 0 replies; 17+ messages in thread
From: David Hildenbrand @ 2023-08-23 12:15 UTC (permalink / raw)
To: linux-kernel, Andrew Morton
Cc: linux-mm, linux-arm-kernel, Matthew Wilcox, Peter Xu,
Catalin Marinas, Will Deacon, Hugh Dickins, Seth Jennings,
Dan Streetman, Vitaly Wool
On 21.08.23 18:08, David Hildenbrand wrote:
> Let's stop using page->private on tail pages, making it possible to
> just unconditionally reuse that field in the tail pages of large folios.
>
> The remaining usage of the private field for THP_SWAP is in the THP
> splitting code (mm/huge_memory.c), that we'll handle separately later.
>
> Update the THP_SWAP documentation and sanity checks in mm_types.h and
> __split_huge_page_tail().
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
@Andrew, the following change on top
From 2b1fd10ef07c6d47aa9cd0ce10445ab1e1b97361 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <david@redhat.com>
Date: Tue, 22 Aug 2023 19:16:55 +0200
Subject: [PATCH] fixup: mm/swap: stop using page->private on tail pages for
THP_SWAP
Per Yoshry, use folio_page_idx(). It shouldn't make a difference for
our (THP) use case, but it's certainly cleaner.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
include/linux/swap.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 84fe0e94f5cd..e5cf58a1cf9e 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -344,7 +344,7 @@ static inline swp_entry_t page_swap_entry(struct page *page)
struct folio *folio = page_folio(page);
swp_entry_t entry = folio_swap_entry(folio);
- entry.val += page - &folio->page;
+ entry.val += folio_page_idx(folio, page);
return entry;
}
--
2.41.0
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-21 16:08 ` [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP David Hildenbrand
` (2 preceding siblings ...)
2023-08-23 12:15 ` David Hildenbrand
@ 2023-08-23 15:12 ` Yosry Ahmed
2023-08-23 15:17 ` David Hildenbrand
3 siblings, 1 reply; 17+ messages in thread
From: Yosry Ahmed @ 2023-08-23 15:12 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
>
> Let's stop using page->private on tail pages, making it possible to
> just unconditionally reuse that field in the tail pages of large folios.
>
> The remaining usage of the private field for THP_SWAP is in the THP
> splitting code (mm/huge_memory.c), that we'll handle separately later.
>
> Update the THP_SWAP documentation and sanity checks in mm_types.h and
> __split_huge_page_tail().
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
The mm part looks good to me (with the added fixup):
Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
Minor nit below, not worth a respin, but perhaps if you respin anyway
for something else.
> ---
> arch/arm64/mm/mteswap.c | 5 +++--
> include/linux/mm_types.h | 12 +-----------
> include/linux/swap.h | 9 +++++++++
> mm/huge_memory.c | 15 ++++++---------
> mm/memory.c | 2 +-
> mm/rmap.c | 2 +-
> mm/swap_state.c | 5 +++--
> mm/swapfile.c | 4 ++--
> 8 files changed, 26 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/mm/mteswap.c b/arch/arm64/mm/mteswap.c
> index cd508ba80ab1..a31833e3ddc5 100644
> --- a/arch/arm64/mm/mteswap.c
> +++ b/arch/arm64/mm/mteswap.c
> @@ -33,8 +33,9 @@ int mte_save_tags(struct page *page)
>
> mte_save_page_tags(page_address(page), tag_storage);
>
> - /* page_private contains the swap entry.val set in do_swap_page */
> - ret = xa_store(&mte_pages, page_private(page), tag_storage, GFP_KERNEL);
> + /* lookup the swap entry.val from the page */
> + ret = xa_store(&mte_pages, page_swap_entry(page).val, tag_storage,
> + GFP_KERNEL);
> if (WARN(xa_is_err(ret), "Failed to store MTE tags")) {
> mte_free_tag_storage(tag_storage);
> return xa_err(ret);
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index b9b6c88875b9..61361f1750c3 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -333,11 +333,8 @@ struct folio {
> atomic_t _pincount;
> #ifdef CONFIG_64BIT
> unsigned int _folio_nr_pages;
> - /* 4 byte gap here */
> - /* private: the union with struct page is transitional */
> - /* Fix THP_SWAP to not use tail->private */
> - unsigned long _private_1;
> #endif
> + /* private: the union with struct page is transitional */
> };
> struct page __page_1;
> };
> @@ -358,9 +355,6 @@ struct folio {
> /* public: */
> struct list_head _deferred_list;
> /* private: the union with struct page is transitional */
> - unsigned long _avail_2a;
> - /* Fix THP_SWAP to not use tail->private */
> - unsigned long _private_2a;
> };
> struct page __page_2;
> };
> @@ -385,9 +379,6 @@ FOLIO_MATCH(memcg_data, memcg_data);
> offsetof(struct page, pg) + sizeof(struct page))
> FOLIO_MATCH(flags, _flags_1);
> FOLIO_MATCH(compound_head, _head_1);
> -#ifdef CONFIG_64BIT
> -FOLIO_MATCH(private, _private_1);
> -#endif
> #undef FOLIO_MATCH
> #define FOLIO_MATCH(pg, fl) \
> static_assert(offsetof(struct folio, fl) == \
> @@ -396,7 +387,6 @@ FOLIO_MATCH(flags, _flags_2);
> FOLIO_MATCH(compound_head, _head_2);
> FOLIO_MATCH(flags, _flags_2a);
> FOLIO_MATCH(compound_head, _head_2a);
> -FOLIO_MATCH(private, _private_2a);
> #undef FOLIO_MATCH
>
> /**
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index bb5adc604144..84fe0e94f5cd 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio)
> return entry;
> }
>
> +static inline swp_entry_t page_swap_entry(struct page *page)
> +{
> + struct folio *folio = page_folio(page);
> + swp_entry_t entry = folio_swap_entry(folio);
> +
> + entry.val += page - &folio->page;
> + return entry;
> +}
> +
> static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
> {
> folio->private = (void *)entry.val;
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index cc2f65f8cc62..c04702ae71d2 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct page *head, int tail,
> page_tail->index = head->index + tail;
>
> /*
> - * page->private should not be set in tail pages with the exception
> - * of swap cache pages that store the swp_entry_t in tail pages.
> - * Fix up and warn once if private is unexpectedly set.
> - *
> - * What of 32-bit systems, on which folio->_pincount overlays
> - * head[1].private? No problem: THP_SWAP is not enabled on 32-bit, and
> - * pincount must be 0 for folio_ref_freeze() to have succeeded.
> + * page->private should not be set in tail pages. Fix up and warn once
> + * if private is unexpectedly set.
> */
> - if (!folio_test_swapcache(page_folio(head))) {
> - VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail);
> + if (unlikely(page_tail->private)) {
> + VM_WARN_ON_ONCE_PAGE(true, page_tail);
> page_tail->private = 0;
> }
Could probably save a couple of lines here:
if (VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail))
page_tail->private = 0;
> + if (PageSwapCache(head))
> + set_page_private(page_tail, (unsigned long)head->private + tail);
>
> /* Page flags must be visible before we make the page non-compound. */
> smp_wmb();
> diff --git a/mm/memory.c b/mm/memory.c
> index d003076b218d..ff13242c1589 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3882,7 +3882,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> * changed.
> */
> if (unlikely(!folio_test_swapcache(folio) ||
> - page_private(page) != entry.val))
> + page_swap_entry(page).val != entry.val))
> goto out_page;
>
> /*
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 1f04debdc87a..ec7f8e6c9e48 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1647,7 +1647,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> */
> dec_mm_counter(mm, mm_counter(&folio->page));
> } else if (folio_test_anon(folio)) {
> - swp_entry_t entry = { .val = page_private(subpage) };
> + swp_entry_t entry = page_swap_entry(subpage);
> pte_t swp_pte;
> /*
> * Store the swap location in the pte.
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 01f15139b7d9..2f2417810052 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -100,6 +100,7 @@ int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
>
> folio_ref_add(folio, nr);
> folio_set_swapcache(folio);
> + folio_set_swap_entry(folio, entry);
>
> do {
> xas_lock_irq(&xas);
> @@ -113,7 +114,6 @@ int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
> if (shadowp)
> *shadowp = old;
> }
> - set_page_private(folio_page(folio, i), entry.val + i);
> xas_store(&xas, folio);
> xas_next(&xas);
> }
> @@ -154,9 +154,10 @@ void __delete_from_swap_cache(struct folio *folio,
> for (i = 0; i < nr; i++) {
> void *entry = xas_store(&xas, shadow);
> VM_BUG_ON_PAGE(entry != folio, entry);
> - set_page_private(folio_page(folio, i), 0);
> xas_next(&xas);
> }
> + entry.val = 0;
> + folio_set_swap_entry(folio, entry);
> folio_clear_swapcache(folio);
> address_space->nrpages -= nr;
> __node_stat_mod_folio(folio, NR_FILE_PAGES, -nr);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index d46933adf789..bd9d904671b9 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -3369,7 +3369,7 @@ struct swap_info_struct *swp_swap_info(swp_entry_t entry)
>
> struct swap_info_struct *page_swap_info(struct page *page)
> {
> - swp_entry_t entry = { .val = page_private(page) };
> + swp_entry_t entry = page_swap_entry(page);
> return swp_swap_info(entry);
> }
>
> @@ -3384,7 +3384,7 @@ EXPORT_SYMBOL_GPL(swapcache_mapping);
>
> pgoff_t __page_file_index(struct page *page)
> {
> - swp_entry_t swap = { .val = page_private(page) };
> + swp_entry_t swap = page_swap_entry(page);
> return swp_offset(swap);
> }
> EXPORT_SYMBOL_GPL(__page_file_index);
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-23 15:12 ` Yosry Ahmed
@ 2023-08-23 15:17 ` David Hildenbrand
2023-08-23 15:21 ` Yosry Ahmed
0 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand @ 2023-08-23 15:17 UTC (permalink / raw)
To: Yosry Ahmed
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
On 23.08.23 17:12, Yosry Ahmed wrote:
> On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> Let's stop using page->private on tail pages, making it possible to
>> just unconditionally reuse that field in the tail pages of large folios.
>>
>> The remaining usage of the private field for THP_SWAP is in the THP
>> splitting code (mm/huge_memory.c), that we'll handle separately later.
>>
>> Update the THP_SWAP documentation and sanity checks in mm_types.h and
>> __split_huge_page_tail().
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>
> The mm part looks good to me (with the added fixup):
>
> Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
Thanks!
>> /**
>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> index bb5adc604144..84fe0e94f5cd 100644
>> --- a/include/linux/swap.h
>> +++ b/include/linux/swap.h
>> @@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio)
>> return entry;
>> }
>>
>> +static inline swp_entry_t page_swap_entry(struct page *page)
>> +{
>> + struct folio *folio = page_folio(page);
>> + swp_entry_t entry = folio_swap_entry(folio);
>> +
>> + entry.val += page - &folio->page;
>> + return entry;
>> +}
>> +
>> static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
>> {
>> folio->private = (void *)entry.val;
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index cc2f65f8cc62..c04702ae71d2 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct page *head, int tail,
>> page_tail->index = head->index + tail;
>>
>> /*
>> - * page->private should not be set in tail pages with the exception
>> - * of swap cache pages that store the swp_entry_t in tail pages.
>> - * Fix up and warn once if private is unexpectedly set.
>> - *
>> - * What of 32-bit systems, on which folio->_pincount overlays
>> - * head[1].private? No problem: THP_SWAP is not enabled on 32-bit, and
>> - * pincount must be 0 for folio_ref_freeze() to have succeeded.
>> + * page->private should not be set in tail pages. Fix up and warn once
>> + * if private is unexpectedly set.
>> */
>> - if (!folio_test_swapcache(page_folio(head))) {
>> - VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail);
>> + if (unlikely(page_tail->private)) {
>> + VM_WARN_ON_ONCE_PAGE(true, page_tail);
>> page_tail->private = 0;
>> }
>
> Could probably save a couple of lines here:
>
> if (VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail))
>
> page_tail->private = 0;
>
That would mean that we eventually compile out the runtime check
#define VM_WARN_ON_ONCE_PAGE(cond, page) BUILD_BUG_ON_INVALID(cond)
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-23 15:17 ` David Hildenbrand
@ 2023-08-23 15:21 ` Yosry Ahmed
2023-08-23 15:26 ` David Hildenbrand
0 siblings, 1 reply; 17+ messages in thread
From: Yosry Ahmed @ 2023-08-23 15:21 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
On Wed, Aug 23, 2023 at 8:17 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 23.08.23 17:12, Yosry Ahmed wrote:
> > On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> Let's stop using page->private on tail pages, making it possible to
> >> just unconditionally reuse that field in the tail pages of large folios.
> >>
> >> The remaining usage of the private field for THP_SWAP is in the THP
> >> splitting code (mm/huge_memory.c), that we'll handle separately later.
> >>
> >> Update the THP_SWAP documentation and sanity checks in mm_types.h and
> >> __split_huge_page_tail().
> >>
> >> Signed-off-by: David Hildenbrand <david@redhat.com>
> >
> > The mm part looks good to me (with the added fixup):
> >
> > Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
>
> Thanks!
>
> >> /**
> >> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >> index bb5adc604144..84fe0e94f5cd 100644
> >> --- a/include/linux/swap.h
> >> +++ b/include/linux/swap.h
> >> @@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio)
> >> return entry;
> >> }
> >>
> >> +static inline swp_entry_t page_swap_entry(struct page *page)
> >> +{
> >> + struct folio *folio = page_folio(page);
> >> + swp_entry_t entry = folio_swap_entry(folio);
> >> +
> >> + entry.val += page - &folio->page;
> >> + return entry;
> >> +}
> >> +
> >> static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
> >> {
> >> folio->private = (void *)entry.val;
> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> index cc2f65f8cc62..c04702ae71d2 100644
> >> --- a/mm/huge_memory.c
> >> +++ b/mm/huge_memory.c
> >> @@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct page *head, int tail,
> >> page_tail->index = head->index + tail;
> >>
> >> /*
> >> - * page->private should not be set in tail pages with the exception
> >> - * of swap cache pages that store the swp_entry_t in tail pages.
> >> - * Fix up and warn once if private is unexpectedly set.
> >> - *
> >> - * What of 32-bit systems, on which folio->_pincount overlays
> >> - * head[1].private? No problem: THP_SWAP is not enabled on 32-bit, and
> >> - * pincount must be 0 for folio_ref_freeze() to have succeeded.
> >> + * page->private should not be set in tail pages. Fix up and warn once
> >> + * if private is unexpectedly set.
> >> */
> >> - if (!folio_test_swapcache(page_folio(head))) {
> >> - VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail);
> >> + if (unlikely(page_tail->private)) {
> >> + VM_WARN_ON_ONCE_PAGE(true, page_tail);
> >> page_tail->private = 0;
> >> }
> >
> > Could probably save a couple of lines here:
> >
> > if (VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail))
> >
> > page_tail->private = 0;
> >
>
> That would mean that we eventually compile out the runtime check
>
> #define VM_WARN_ON_ONCE_PAGE(cond, page) BUILD_BUG_ON_INVALID(cond)
I thought the warning would be compiled out but not the check, my bad.
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-23 15:21 ` Yosry Ahmed
@ 2023-08-23 15:26 ` David Hildenbrand
2023-08-23 15:31 ` Yosry Ahmed
0 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand @ 2023-08-23 15:26 UTC (permalink / raw)
To: Yosry Ahmed
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
On 23.08.23 17:21, Yosry Ahmed wrote:
> On Wed, Aug 23, 2023 at 8:17 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 23.08.23 17:12, Yosry Ahmed wrote:
>>> On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
>>>>
>>>> Let's stop using page->private on tail pages, making it possible to
>>>> just unconditionally reuse that field in the tail pages of large folios.
>>>>
>>>> The remaining usage of the private field for THP_SWAP is in the THP
>>>> splitting code (mm/huge_memory.c), that we'll handle separately later.
>>>>
>>>> Update the THP_SWAP documentation and sanity checks in mm_types.h and
>>>> __split_huge_page_tail().
>>>>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>
>>> The mm part looks good to me (with the added fixup):
>>>
>>> Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
>>
>> Thanks!
>>
>>>> /**
>>>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>>>> index bb5adc604144..84fe0e94f5cd 100644
>>>> --- a/include/linux/swap.h
>>>> +++ b/include/linux/swap.h
>>>> @@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio)
>>>> return entry;
>>>> }
>>>>
>>>> +static inline swp_entry_t page_swap_entry(struct page *page)
>>>> +{
>>>> + struct folio *folio = page_folio(page);
>>>> + swp_entry_t entry = folio_swap_entry(folio);
>>>> +
>>>> + entry.val += page - &folio->page;
>>>> + return entry;
>>>> +}
>>>> +
>>>> static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
>>>> {
>>>> folio->private = (void *)entry.val;
>>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>>> index cc2f65f8cc62..c04702ae71d2 100644
>>>> --- a/mm/huge_memory.c
>>>> +++ b/mm/huge_memory.c
>>>> @@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct page *head, int tail,
>>>> page_tail->index = head->index + tail;
>>>>
>>>> /*
>>>> - * page->private should not be set in tail pages with the exception
>>>> - * of swap cache pages that store the swp_entry_t in tail pages.
>>>> - * Fix up and warn once if private is unexpectedly set.
>>>> - *
>>>> - * What of 32-bit systems, on which folio->_pincount overlays
>>>> - * head[1].private? No problem: THP_SWAP is not enabled on 32-bit, and
>>>> - * pincount must be 0 for folio_ref_freeze() to have succeeded.
>>>> + * page->private should not be set in tail pages. Fix up and warn once
>>>> + * if private is unexpectedly set.
>>>> */
>>>> - if (!folio_test_swapcache(page_folio(head))) {
>>>> - VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail);
>>>> + if (unlikely(page_tail->private)) {
>>>> + VM_WARN_ON_ONCE_PAGE(true, page_tail);
>>>> page_tail->private = 0;
>>>> }
>>>
>>> Could probably save a couple of lines here:
>>>
>>> if (VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail))
>>>
>>> page_tail->private = 0;
>>>
>>
>> That would mean that we eventually compile out the runtime check
>>
>> #define VM_WARN_ON_ONCE_PAGE(cond, page) BUILD_BUG_ON_INVALID(cond)
>
> I thought the warning would be compiled out but not the check, my bad.
I even remembered that VM_WARN_ON_ONCE and friends could/should not be
used in conditionals.
But we do seem to have two users now:
$ git grep "if (VM_WARN_ON"
mm/mmap.c: if (VM_WARN_ON_ONCE_MM(vma->vm_end != vmi_end, mm))
mm/mmap.c: if (VM_WARN_ON_ONCE_MM(vma->vm_start != vmi_start, mm))
But they only do warning-related action, to dump the stack, the vma, ...
So if the warnings get compiled out, also all the other stuff gets compiled out as well,
which makes sense here.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP
2023-08-23 15:26 ` David Hildenbrand
@ 2023-08-23 15:31 ` Yosry Ahmed
0 siblings, 0 replies; 17+ messages in thread
From: Yosry Ahmed @ 2023-08-23 15:31 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
On Wed, Aug 23, 2023 at 8:26 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 23.08.23 17:21, Yosry Ahmed wrote:
> > On Wed, Aug 23, 2023 at 8:17 AM David Hildenbrand <david@redhat.com> wrote:
> >>
> >> On 23.08.23 17:12, Yosry Ahmed wrote:
> >>> On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
> >>>>
> >>>> Let's stop using page->private on tail pages, making it possible to
> >>>> just unconditionally reuse that field in the tail pages of large folios.
> >>>>
> >>>> The remaining usage of the private field for THP_SWAP is in the THP
> >>>> splitting code (mm/huge_memory.c), that we'll handle separately later.
> >>>>
> >>>> Update the THP_SWAP documentation and sanity checks in mm_types.h and
> >>>> __split_huge_page_tail().
> >>>>
> >>>> Signed-off-by: David Hildenbrand <david@redhat.com>
> >>>
> >>> The mm part looks good to me (with the added fixup):
> >>>
> >>> Reviewed-by: Yosry Ahmed <yosryahmed@google.com>
> >>
> >> Thanks!
> >>
> >>>> /**
> >>>> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >>>> index bb5adc604144..84fe0e94f5cd 100644
> >>>> --- a/include/linux/swap.h
> >>>> +++ b/include/linux/swap.h
> >>>> @@ -339,6 +339,15 @@ static inline swp_entry_t folio_swap_entry(struct folio *folio)
> >>>> return entry;
> >>>> }
> >>>>
> >>>> +static inline swp_entry_t page_swap_entry(struct page *page)
> >>>> +{
> >>>> + struct folio *folio = page_folio(page);
> >>>> + swp_entry_t entry = folio_swap_entry(folio);
> >>>> +
> >>>> + entry.val += page - &folio->page;
> >>>> + return entry;
> >>>> +}
> >>>> +
> >>>> static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
> >>>> {
> >>>> folio->private = (void *)entry.val;
> >>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >>>> index cc2f65f8cc62..c04702ae71d2 100644
> >>>> --- a/mm/huge_memory.c
> >>>> +++ b/mm/huge_memory.c
> >>>> @@ -2446,18 +2446,15 @@ static void __split_huge_page_tail(struct page *head, int tail,
> >>>> page_tail->index = head->index + tail;
> >>>>
> >>>> /*
> >>>> - * page->private should not be set in tail pages with the exception
> >>>> - * of swap cache pages that store the swp_entry_t in tail pages.
> >>>> - * Fix up and warn once if private is unexpectedly set.
> >>>> - *
> >>>> - * What of 32-bit systems, on which folio->_pincount overlays
> >>>> - * head[1].private? No problem: THP_SWAP is not enabled on 32-bit, and
> >>>> - * pincount must be 0 for folio_ref_freeze() to have succeeded.
> >>>> + * page->private should not be set in tail pages. Fix up and warn once
> >>>> + * if private is unexpectedly set.
> >>>> */
> >>>> - if (!folio_test_swapcache(page_folio(head))) {
> >>>> - VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail);
> >>>> + if (unlikely(page_tail->private)) {
> >>>> + VM_WARN_ON_ONCE_PAGE(true, page_tail);
> >>>> page_tail->private = 0;
> >>>> }
> >>>
> >>> Could probably save a couple of lines here:
> >>>
> >>> if (VM_WARN_ON_ONCE_PAGE(page_tail->private != 0, page_tail))
> >>>
> >>> page_tail->private = 0;
> >>>
> >>
> >> That would mean that we eventually compile out the runtime check
> >>
> >> #define VM_WARN_ON_ONCE_PAGE(cond, page) BUILD_BUG_ON_INVALID(cond)
> >
> > I thought the warning would be compiled out but not the check, my bad.
>
> I even remembered that VM_WARN_ON_ONCE and friends could/should not be
> used in conditionals.
>
> But we do seem to have two users now:
>
> $ git grep "if (VM_WARN_ON"
> mm/mmap.c: if (VM_WARN_ON_ONCE_MM(vma->vm_end != vmi_end, mm))
> mm/mmap.c: if (VM_WARN_ON_ONCE_MM(vma->vm_start != vmi_start, mm))
>
> But they only do warning-related action, to dump the stack, the vma, ...
>
> So if the warnings get compiled out, also all the other stuff gets compiled out as well,
> which makes sense here.
Funny enough, I did the same grep and immediately thought that since
we have users of that, then it's okay (i.e the check wouldn't be
compiled out). I wasn't thorough enough to actually check what they
are doing :)
>
> --
> Cheers,
>
> David / dhildenb
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH mm-unstable v1 2/4] mm/swap: use dedicated entry for swap in folio
2023-08-21 16:08 [PATCH mm-unstable v1 0/4] mm/swap: stop using page->private on tail pages for THP_SWAP + cleanups David Hildenbrand
2023-08-21 16:08 ` [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP David Hildenbrand
@ 2023-08-21 16:08 ` David Hildenbrand
2023-08-23 13:15 ` Chris Li
2023-08-21 16:08 ` [PATCH mm-unstable v1 3/4] mm/swap: inline folio_set_swap_entry() and folio_swap_entry() David Hildenbrand
2023-08-21 16:08 ` [PATCH mm-unstable v1 4/4] mm/huge_memory: work on folio->swap instead of page->private when splitting folio David Hildenbrand
3 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand @ 2023-08-21 16:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linux-arm-kernel, David Hildenbrand, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
From: Matthew Wilcox <willy@infradead.org>
Let's stop working on the private field and use an explicit swap field.
We have to move the swp_entry_t typedef.
Signed-off-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
include/linux/mm_types.h | 23 +++++++++++++----------
include/linux/swap.h | 5 ++---
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 61361f1750c3..438a07854f8c 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -259,6 +259,14 @@ static inline struct page *encoded_page_ptr(struct encoded_page *page)
*/
#define TAIL_MAPPING_REUSED_MAX (2)
+/*
+ * A swap entry has to fit into a "unsigned long", as the entry is hidden
+ * in the "index" field of the swapper address space.
+ */
+typedef struct {
+ unsigned long val;
+} swp_entry_t;
+
/**
* struct folio - Represents a contiguous set of bytes.
* @flags: Identical to the page flags.
@@ -269,7 +277,7 @@ static inline struct page *encoded_page_ptr(struct encoded_page *page)
* @index: Offset within the file, in units of pages. For anonymous memory,
* this is the index from the beginning of the mmap.
* @private: Filesystem per-folio data (see folio_attach_private()).
- * Used for swp_entry_t if folio_test_swapcache().
+ * @swap: Used for swp_entry_t if folio_test_swapcache().
* @_mapcount: Do not access this member directly. Use folio_mapcount() to
* find out how many times this folio is mapped by userspace.
* @_refcount: Do not access this member directly. Use folio_ref_count()
@@ -312,7 +320,10 @@ struct folio {
};
struct address_space *mapping;
pgoff_t index;
- void *private;
+ union {
+ void *private;
+ swp_entry_t swap;
+ };
atomic_t _mapcount;
atomic_t _refcount;
#ifdef CONFIG_MEMCG
@@ -1220,14 +1231,6 @@ enum tlb_flush_reason {
NR_TLB_FLUSH_REASONS,
};
- /*
- * A swap entry has to fit into a "unsigned long", as the entry is hidden
- * in the "index" field of the swapper address space.
- */
-typedef struct {
- unsigned long val;
-} swp_entry_t;
-
/**
* enum fault_flag - Fault flag definitions.
* @FAULT_FLAG_WRITE: Fault was a write fault.
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 84fe0e94f5cd..82859a1944f5 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -335,8 +335,7 @@ struct swap_info_struct {
static inline swp_entry_t folio_swap_entry(struct folio *folio)
{
- swp_entry_t entry = { .val = page_private(&folio->page) };
- return entry;
+ return folio->swap;
}
static inline swp_entry_t page_swap_entry(struct page *page)
@@ -350,7 +349,7 @@ static inline swp_entry_t page_swap_entry(struct page *page)
static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
{
- folio->private = (void *)entry.val;
+ folio->swap = entry;
}
/* linux/mm/workingset.c */
--
2.41.0
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 2/4] mm/swap: use dedicated entry for swap in folio
2023-08-21 16:08 ` [PATCH mm-unstable v1 2/4] mm/swap: use dedicated entry for swap in folio David Hildenbrand
@ 2023-08-23 13:15 ` Chris Li
0 siblings, 0 replies; 17+ messages in thread
From: Chris Li @ 2023-08-23 13:15 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
Reviewed-by: Chris Li <chrisl@kernel.org>
Chris
On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
>
> From: Matthew Wilcox <willy@infradead.org>
>
> Let's stop working on the private field and use an explicit swap field.
> We have to move the swp_entry_t typedef.
>
> Signed-off-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> include/linux/mm_types.h | 23 +++++++++++++----------
> include/linux/swap.h | 5 ++---
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 61361f1750c3..438a07854f8c 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -259,6 +259,14 @@ static inline struct page *encoded_page_ptr(struct encoded_page *page)
> */
> #define TAIL_MAPPING_REUSED_MAX (2)
>
> +/*
> + * A swap entry has to fit into a "unsigned long", as the entry is hidden
> + * in the "index" field of the swapper address space.
> + */
> +typedef struct {
> + unsigned long val;
> +} swp_entry_t;
> +
> /**
> * struct folio - Represents a contiguous set of bytes.
> * @flags: Identical to the page flags.
> @@ -269,7 +277,7 @@ static inline struct page *encoded_page_ptr(struct encoded_page *page)
> * @index: Offset within the file, in units of pages. For anonymous memory,
> * this is the index from the beginning of the mmap.
> * @private: Filesystem per-folio data (see folio_attach_private()).
> - * Used for swp_entry_t if folio_test_swapcache().
> + * @swap: Used for swp_entry_t if folio_test_swapcache().
> * @_mapcount: Do not access this member directly. Use folio_mapcount() to
> * find out how many times this folio is mapped by userspace.
> * @_refcount: Do not access this member directly. Use folio_ref_count()
> @@ -312,7 +320,10 @@ struct folio {
> };
> struct address_space *mapping;
> pgoff_t index;
> - void *private;
> + union {
> + void *private;
> + swp_entry_t swap;
> + };
> atomic_t _mapcount;
> atomic_t _refcount;
> #ifdef CONFIG_MEMCG
> @@ -1220,14 +1231,6 @@ enum tlb_flush_reason {
> NR_TLB_FLUSH_REASONS,
> };
>
> - /*
> - * A swap entry has to fit into a "unsigned long", as the entry is hidden
> - * in the "index" field of the swapper address space.
> - */
> -typedef struct {
> - unsigned long val;
> -} swp_entry_t;
> -
> /**
> * enum fault_flag - Fault flag definitions.
> * @FAULT_FLAG_WRITE: Fault was a write fault.
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 84fe0e94f5cd..82859a1944f5 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -335,8 +335,7 @@ struct swap_info_struct {
>
> static inline swp_entry_t folio_swap_entry(struct folio *folio)
> {
> - swp_entry_t entry = { .val = page_private(&folio->page) };
> - return entry;
> + return folio->swap;
> }
>
> static inline swp_entry_t page_swap_entry(struct page *page)
> @@ -350,7 +349,7 @@ static inline swp_entry_t page_swap_entry(struct page *page)
>
> static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
> {
> - folio->private = (void *)entry.val;
> + folio->swap = entry;
> }
>
> /* linux/mm/workingset.c */
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH mm-unstable v1 3/4] mm/swap: inline folio_set_swap_entry() and folio_swap_entry()
2023-08-21 16:08 [PATCH mm-unstable v1 0/4] mm/swap: stop using page->private on tail pages for THP_SWAP + cleanups David Hildenbrand
2023-08-21 16:08 ` [PATCH mm-unstable v1 1/4] mm/swap: stop using page->private on tail pages for THP_SWAP David Hildenbrand
2023-08-21 16:08 ` [PATCH mm-unstable v1 2/4] mm/swap: use dedicated entry for swap in folio David Hildenbrand
@ 2023-08-21 16:08 ` David Hildenbrand
2023-08-23 13:15 ` Chris Li
2023-08-21 16:08 ` [PATCH mm-unstable v1 4/4] mm/huge_memory: work on folio->swap instead of page->private when splitting folio David Hildenbrand
3 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand @ 2023-08-21 16:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linux-arm-kernel, David Hildenbrand, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
Let's simply work on the folio directly and remove the helpers.
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
include/linux/swap.h | 12 +-----------
mm/memory.c | 2 +-
mm/shmem.c | 6 +++---
mm/swap_state.c | 7 +++----
mm/swapfile.c | 2 +-
mm/util.c | 2 +-
mm/vmscan.c | 2 +-
mm/zswap.c | 4 ++--
8 files changed, 13 insertions(+), 24 deletions(-)
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 82859a1944f5..603acf813873 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -333,25 +333,15 @@ struct swap_info_struct {
*/
};
-static inline swp_entry_t folio_swap_entry(struct folio *folio)
-{
- return folio->swap;
-}
-
static inline swp_entry_t page_swap_entry(struct page *page)
{
struct folio *folio = page_folio(page);
- swp_entry_t entry = folio_swap_entry(folio);
+ swp_entry_t entry = folio->swap;
entry.val += page - &folio->page;
return entry;
}
-static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
-{
- folio->swap = entry;
-}
-
/* linux/mm/workingset.c */
bool workingset_test_recent(void *shadow, bool file, bool *workingset);
void workingset_age_nonresident(struct lruvec *lruvec, unsigned long nr_pages);
diff --git a/mm/memory.c b/mm/memory.c
index ff13242c1589..c51800dbfa9b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3831,7 +3831,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
folio_add_lru(folio);
/* To provide entry to swap_readpage() */
- folio_set_swap_entry(folio, entry);
+ folio->swap = entry;
swap_readpage(page, true, NULL);
folio->private = NULL;
}
diff --git a/mm/shmem.c b/mm/shmem.c
index 7a0c1e19d9f8..fc1afe9dfcfe 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1657,7 +1657,7 @@ static int shmem_replace_folio(struct folio **foliop, gfp_t gfp,
int error;
old = *foliop;
- entry = folio_swap_entry(old);
+ entry = old->swap;
swap_index = swp_offset(entry);
swap_mapping = swap_address_space(entry);
@@ -1678,7 +1678,7 @@ static int shmem_replace_folio(struct folio **foliop, gfp_t gfp,
__folio_set_locked(new);
__folio_set_swapbacked(new);
folio_mark_uptodate(new);
- folio_set_swap_entry(new, entry);
+ new->swap = entry;
folio_set_swapcache(new);
/*
@@ -1800,7 +1800,7 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
/* We have to do this with folio locked to prevent races */
folio_lock(folio);
if (!folio_test_swapcache(folio) ||
- folio_swap_entry(folio).val != swap.val ||
+ folio->swap.val != swap.val ||
!shmem_confirm_swap(mapping, index, swap)) {
error = -EEXIST;
goto unlock;
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 2f2417810052..b3b14bd0dd64 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -100,7 +100,7 @@ int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
folio_ref_add(folio, nr);
folio_set_swapcache(folio);
- folio_set_swap_entry(folio, entry);
+ folio->swap = entry;
do {
xas_lock_irq(&xas);
@@ -156,8 +156,7 @@ void __delete_from_swap_cache(struct folio *folio,
VM_BUG_ON_PAGE(entry != folio, entry);
xas_next(&xas);
}
- entry.val = 0;
- folio_set_swap_entry(folio, entry);
+ folio->swap.val = 0;
folio_clear_swapcache(folio);
address_space->nrpages -= nr;
__node_stat_mod_folio(folio, NR_FILE_PAGES, -nr);
@@ -233,7 +232,7 @@ bool add_to_swap(struct folio *folio)
*/
void delete_from_swap_cache(struct folio *folio)
{
- swp_entry_t entry = folio_swap_entry(folio);
+ swp_entry_t entry = folio->swap;
struct address_space *address_space = swap_address_space(entry);
xa_lock_irq(&address_space->i_pages);
diff --git a/mm/swapfile.c b/mm/swapfile.c
index bd9d904671b9..e52f486834eb 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -1536,7 +1536,7 @@ static bool swap_page_trans_huge_swapped(struct swap_info_struct *si,
static bool folio_swapped(struct folio *folio)
{
- swp_entry_t entry = folio_swap_entry(folio);
+ swp_entry_t entry = folio->swap;
struct swap_info_struct *si = _swap_info_get(entry);
if (!si)
diff --git a/mm/util.c b/mm/util.c
index cde229b05eb3..f31e2ca62cfa 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -764,7 +764,7 @@ struct address_space *folio_mapping(struct folio *folio)
return NULL;
if (unlikely(folio_test_swapcache(folio)))
- return swap_address_space(folio_swap_entry(folio));
+ return swap_address_space(folio->swap);
mapping = folio->mapping;
if ((unsigned long)mapping & PAGE_MAPPING_FLAGS)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index c7c149cb8d66..6f13394b112e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1423,7 +1423,7 @@ static int __remove_mapping(struct address_space *mapping, struct folio *folio,
}
if (folio_test_swapcache(folio)) {
- swp_entry_t swap = folio_swap_entry(folio);
+ swp_entry_t swap = folio->swap;
if (reclaimed && !mapping_exiting(mapping))
shadow = workingset_eviction(folio, target_memcg);
diff --git a/mm/zswap.c b/mm/zswap.c
index 7300b98d4a03..412b1409a0d7 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -1190,7 +1190,7 @@ static void zswap_fill_page(void *ptr, unsigned long value)
bool zswap_store(struct folio *folio)
{
- swp_entry_t swp = folio_swap_entry(folio);
+ swp_entry_t swp = folio->swap;
int type = swp_type(swp);
pgoff_t offset = swp_offset(swp);
struct page *page = &folio->page;
@@ -1370,7 +1370,7 @@ bool zswap_store(struct folio *folio)
bool zswap_load(struct folio *folio)
{
- swp_entry_t swp = folio_swap_entry(folio);
+ swp_entry_t swp = folio->swap;
int type = swp_type(swp);
pgoff_t offset = swp_offset(swp);
struct page *page = &folio->page;
--
2.41.0
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 3/4] mm/swap: inline folio_set_swap_entry() and folio_swap_entry()
2023-08-21 16:08 ` [PATCH mm-unstable v1 3/4] mm/swap: inline folio_set_swap_entry() and folio_swap_entry() David Hildenbrand
@ 2023-08-23 13:15 ` Chris Li
0 siblings, 0 replies; 17+ messages in thread
From: Chris Li @ 2023-08-23 13:15 UTC (permalink / raw)
To: David Hildenbrand
Cc: LKML, linux-mm, linux-arm-kernel, Andrew Morton, Matthew Wilcox,
Peter Xu, Catalin Marinas, Will Deacon, Hugh Dickins,
Seth Jennings, Dan Streetman, Vitaly Wool
Reviewed-by: Chris Li <chrisl@kernel.org>
Chris
On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
>
> Let's simply work on the folio directly and remove the helpers.
>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> include/linux/swap.h | 12 +-----------
> mm/memory.c | 2 +-
> mm/shmem.c | 6 +++---
> mm/swap_state.c | 7 +++----
> mm/swapfile.c | 2 +-
> mm/util.c | 2 +-
> mm/vmscan.c | 2 +-
> mm/zswap.c | 4 ++--
> 8 files changed, 13 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 82859a1944f5..603acf813873 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -333,25 +333,15 @@ struct swap_info_struct {
> */
> };
>
> -static inline swp_entry_t folio_swap_entry(struct folio *folio)
> -{
> - return folio->swap;
> -}
> -
> static inline swp_entry_t page_swap_entry(struct page *page)
> {
> struct folio *folio = page_folio(page);
> - swp_entry_t entry = folio_swap_entry(folio);
> + swp_entry_t entry = folio->swap;
>
> entry.val += page - &folio->page;
> return entry;
> }
>
> -static inline void folio_set_swap_entry(struct folio *folio, swp_entry_t entry)
> -{
> - folio->swap = entry;
> -}
> -
> /* linux/mm/workingset.c */
> bool workingset_test_recent(void *shadow, bool file, bool *workingset);
> void workingset_age_nonresident(struct lruvec *lruvec, unsigned long nr_pages);
> diff --git a/mm/memory.c b/mm/memory.c
> index ff13242c1589..c51800dbfa9b 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3831,7 +3831,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
> folio_add_lru(folio);
>
> /* To provide entry to swap_readpage() */
> - folio_set_swap_entry(folio, entry);
> + folio->swap = entry;
> swap_readpage(page, true, NULL);
> folio->private = NULL;
> }
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 7a0c1e19d9f8..fc1afe9dfcfe 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1657,7 +1657,7 @@ static int shmem_replace_folio(struct folio **foliop, gfp_t gfp,
> int error;
>
> old = *foliop;
> - entry = folio_swap_entry(old);
> + entry = old->swap;
> swap_index = swp_offset(entry);
> swap_mapping = swap_address_space(entry);
>
> @@ -1678,7 +1678,7 @@ static int shmem_replace_folio(struct folio **foliop, gfp_t gfp,
> __folio_set_locked(new);
> __folio_set_swapbacked(new);
> folio_mark_uptodate(new);
> - folio_set_swap_entry(new, entry);
> + new->swap = entry;
> folio_set_swapcache(new);
>
> /*
> @@ -1800,7 +1800,7 @@ static int shmem_swapin_folio(struct inode *inode, pgoff_t index,
> /* We have to do this with folio locked to prevent races */
> folio_lock(folio);
> if (!folio_test_swapcache(folio) ||
> - folio_swap_entry(folio).val != swap.val ||
> + folio->swap.val != swap.val ||
> !shmem_confirm_swap(mapping, index, swap)) {
> error = -EEXIST;
> goto unlock;
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 2f2417810052..b3b14bd0dd64 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -100,7 +100,7 @@ int add_to_swap_cache(struct folio *folio, swp_entry_t entry,
>
> folio_ref_add(folio, nr);
> folio_set_swapcache(folio);
> - folio_set_swap_entry(folio, entry);
> + folio->swap = entry;
>
> do {
> xas_lock_irq(&xas);
> @@ -156,8 +156,7 @@ void __delete_from_swap_cache(struct folio *folio,
> VM_BUG_ON_PAGE(entry != folio, entry);
> xas_next(&xas);
> }
> - entry.val = 0;
> - folio_set_swap_entry(folio, entry);
> + folio->swap.val = 0;
> folio_clear_swapcache(folio);
> address_space->nrpages -= nr;
> __node_stat_mod_folio(folio, NR_FILE_PAGES, -nr);
> @@ -233,7 +232,7 @@ bool add_to_swap(struct folio *folio)
> */
> void delete_from_swap_cache(struct folio *folio)
> {
> - swp_entry_t entry = folio_swap_entry(folio);
> + swp_entry_t entry = folio->swap;
> struct address_space *address_space = swap_address_space(entry);
>
> xa_lock_irq(&address_space->i_pages);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index bd9d904671b9..e52f486834eb 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -1536,7 +1536,7 @@ static bool swap_page_trans_huge_swapped(struct swap_info_struct *si,
>
> static bool folio_swapped(struct folio *folio)
> {
> - swp_entry_t entry = folio_swap_entry(folio);
> + swp_entry_t entry = folio->swap;
> struct swap_info_struct *si = _swap_info_get(entry);
>
> if (!si)
> diff --git a/mm/util.c b/mm/util.c
> index cde229b05eb3..f31e2ca62cfa 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -764,7 +764,7 @@ struct address_space *folio_mapping(struct folio *folio)
> return NULL;
>
> if (unlikely(folio_test_swapcache(folio)))
> - return swap_address_space(folio_swap_entry(folio));
> + return swap_address_space(folio->swap);
>
> mapping = folio->mapping;
> if ((unsigned long)mapping & PAGE_MAPPING_FLAGS)
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index c7c149cb8d66..6f13394b112e 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1423,7 +1423,7 @@ static int __remove_mapping(struct address_space *mapping, struct folio *folio,
> }
>
> if (folio_test_swapcache(folio)) {
> - swp_entry_t swap = folio_swap_entry(folio);
> + swp_entry_t swap = folio->swap;
>
> if (reclaimed && !mapping_exiting(mapping))
> shadow = workingset_eviction(folio, target_memcg);
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 7300b98d4a03..412b1409a0d7 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -1190,7 +1190,7 @@ static void zswap_fill_page(void *ptr, unsigned long value)
>
> bool zswap_store(struct folio *folio)
> {
> - swp_entry_t swp = folio_swap_entry(folio);
> + swp_entry_t swp = folio->swap;
> int type = swp_type(swp);
> pgoff_t offset = swp_offset(swp);
> struct page *page = &folio->page;
> @@ -1370,7 +1370,7 @@ bool zswap_store(struct folio *folio)
>
> bool zswap_load(struct folio *folio)
> {
> - swp_entry_t swp = folio_swap_entry(folio);
> + swp_entry_t swp = folio->swap;
> int type = swp_type(swp);
> pgoff_t offset = swp_offset(swp);
> struct page *page = &folio->page;
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH mm-unstable v1 4/4] mm/huge_memory: work on folio->swap instead of page->private when splitting folio
2023-08-21 16:08 [PATCH mm-unstable v1 0/4] mm/swap: stop using page->private on tail pages for THP_SWAP + cleanups David Hildenbrand
` (2 preceding siblings ...)
2023-08-21 16:08 ` [PATCH mm-unstable v1 3/4] mm/swap: inline folio_set_swap_entry() and folio_swap_entry() David Hildenbrand
@ 2023-08-21 16:08 ` David Hildenbrand
2023-08-23 13:16 ` Chris Li
3 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand @ 2023-08-21 16:08 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, linux-arm-kernel, David Hildenbrand, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
Let's work on folio->swap instead. While at it, use folio_test_anon() and
folio_test_swapcache() -- the original folio remains valid even after
splitting (but is then an order-0 folio).
We can probably convert a lot more to folios in that code, let's focus
on folio->swap handling only for now.
Signed-off-by: David Hildenbrand <david@redhat.com>
---
mm/huge_memory.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index c04702ae71d2..4465915711c3 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2401,10 +2401,16 @@ static void lru_add_page_tail(struct page *head, struct page *tail,
}
}
-static void __split_huge_page_tail(struct page *head, int tail,
+static void __split_huge_page_tail(struct folio *folio, int tail,
struct lruvec *lruvec, struct list_head *list)
{
+ struct page *head = &folio->page;
struct page *page_tail = head + tail;
+ /*
+ * Careful: new_folio is not a "real" folio before we cleared PageTail.
+ * Don't pass it around before clear_compound_head().
+ */
+ struct folio *new_folio = (struct folio *)page_tail;
VM_BUG_ON_PAGE(atomic_read(&page_tail->_mapcount) != -1, page_tail);
@@ -2453,8 +2459,8 @@ static void __split_huge_page_tail(struct page *head, int tail,
VM_WARN_ON_ONCE_PAGE(true, page_tail);
page_tail->private = 0;
}
- if (PageSwapCache(head))
- set_page_private(page_tail, (unsigned long)head->private + tail);
+ if (folio_test_swapcache(folio))
+ new_folio->swap.val = folio->swap.val + tail;
/* Page flags must be visible before we make the page non-compound. */
smp_wmb();
@@ -2500,11 +2506,9 @@ static void __split_huge_page(struct page *page, struct list_head *list,
/* complete memcg works before add pages to LRU */
split_page_memcg(head, nr);
- if (PageAnon(head) && PageSwapCache(head)) {
- swp_entry_t entry = { .val = page_private(head) };
-
- offset = swp_offset(entry);
- swap_cache = swap_address_space(entry);
+ if (folio_test_anon(folio) && folio_test_swapcache(folio)) {
+ offset = swp_offset(folio->swap);
+ swap_cache = swap_address_space(folio->swap);
xa_lock(&swap_cache->i_pages);
}
@@ -2514,7 +2518,7 @@ static void __split_huge_page(struct page *page, struct list_head *list,
ClearPageHasHWPoisoned(head);
for (i = nr - 1; i >= 1; i--) {
- __split_huge_page_tail(head, i, lruvec, list);
+ __split_huge_page_tail(folio, i, lruvec, list);
/* Some pages can be beyond EOF: drop them from page cache */
if (head[i].index >= end) {
struct folio *tail = page_folio(head + i);
@@ -2559,11 +2563,8 @@ static void __split_huge_page(struct page *page, struct list_head *list,
remap_page(folio, nr);
- if (PageSwapCache(head)) {
- swp_entry_t entry = { .val = page_private(head) };
-
- split_swap_cluster(entry);
- }
+ if (folio_test_swapcache(folio))
+ split_swap_cluster(folio->swap);
for (i = 0; i < nr; i++) {
struct page *subpage = head + i;
--
2.41.0
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH mm-unstable v1 4/4] mm/huge_memory: work on folio->swap instead of page->private when splitting folio
2023-08-21 16:08 ` [PATCH mm-unstable v1 4/4] mm/huge_memory: work on folio->swap instead of page->private when splitting folio David Hildenbrand
@ 2023-08-23 13:16 ` Chris Li
0 siblings, 0 replies; 17+ messages in thread
From: Chris Li @ 2023-08-23 13:16 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, linux-mm, linux-arm-kernel, Andrew Morton,
Matthew Wilcox, Peter Xu, Catalin Marinas, Will Deacon,
Hugh Dickins, Seth Jennings, Dan Streetman, Vitaly Wool
Hi David,
Reviewed-by: Chris Li <chrisl@kernel.org>
Chris
Chris
On Mon, Aug 21, 2023 at 9:09 AM David Hildenbrand <david@redhat.com> wrote:
>
> Let's work on folio->swap instead. While at it, use folio_test_anon() and
> folio_test_swapcache() -- the original folio remains valid even after
> splitting (but is then an order-0 folio).
>
> We can probably convert a lot more to folios in that code, let's focus
> on folio->swap handling only for now.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> mm/huge_memory.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index c04702ae71d2..4465915711c3 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2401,10 +2401,16 @@ static void lru_add_page_tail(struct page *head, struct page *tail,
> }
> }
>
> -static void __split_huge_page_tail(struct page *head, int tail,
> +static void __split_huge_page_tail(struct folio *folio, int tail,
> struct lruvec *lruvec, struct list_head *list)
> {
> + struct page *head = &folio->page;
> struct page *page_tail = head + tail;
> + /*
> + * Careful: new_folio is not a "real" folio before we cleared PageTail.
> + * Don't pass it around before clear_compound_head().
> + */
> + struct folio *new_folio = (struct folio *)page_tail;
>
> VM_BUG_ON_PAGE(atomic_read(&page_tail->_mapcount) != -1, page_tail);
>
> @@ -2453,8 +2459,8 @@ static void __split_huge_page_tail(struct page *head, int tail,
> VM_WARN_ON_ONCE_PAGE(true, page_tail);
> page_tail->private = 0;
> }
> - if (PageSwapCache(head))
> - set_page_private(page_tail, (unsigned long)head->private + tail);
> + if (folio_test_swapcache(folio))
> + new_folio->swap.val = folio->swap.val + tail;
>
> /* Page flags must be visible before we make the page non-compound. */
> smp_wmb();
> @@ -2500,11 +2506,9 @@ static void __split_huge_page(struct page *page, struct list_head *list,
> /* complete memcg works before add pages to LRU */
> split_page_memcg(head, nr);
>
> - if (PageAnon(head) && PageSwapCache(head)) {
> - swp_entry_t entry = { .val = page_private(head) };
> -
> - offset = swp_offset(entry);
> - swap_cache = swap_address_space(entry);
> + if (folio_test_anon(folio) && folio_test_swapcache(folio)) {
> + offset = swp_offset(folio->swap);
> + swap_cache = swap_address_space(folio->swap);
> xa_lock(&swap_cache->i_pages);
> }
>
> @@ -2514,7 +2518,7 @@ static void __split_huge_page(struct page *page, struct list_head *list,
> ClearPageHasHWPoisoned(head);
>
> for (i = nr - 1; i >= 1; i--) {
> - __split_huge_page_tail(head, i, lruvec, list);
> + __split_huge_page_tail(folio, i, lruvec, list);
> /* Some pages can be beyond EOF: drop them from page cache */
> if (head[i].index >= end) {
> struct folio *tail = page_folio(head + i);
> @@ -2559,11 +2563,8 @@ static void __split_huge_page(struct page *page, struct list_head *list,
>
> remap_page(folio, nr);
>
> - if (PageSwapCache(head)) {
> - swp_entry_t entry = { .val = page_private(head) };
> -
> - split_swap_cluster(entry);
> - }
> + if (folio_test_swapcache(folio))
> + split_swap_cluster(folio->swap);
>
> for (i = 0; i < nr; i++) {
> struct page *subpage = head + i;
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread