* [PATCH v2 0/2] Honor XA_FLAGS_ACCOUNT in xas_split_alloc() and charge to folio's memcg
@ 2026-07-31 2:17 Zi Yan
2026-07-31 2:18 ` [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split() Zi Yan
2026-07-31 2:18 ` [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc() Zi Yan
0 siblings, 2 replies; 19+ messages in thread
From: Zi Yan @ 2026-07-31 2:17 UTC (permalink / raw)
To: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Lorenzo Stoakes, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif
Cc: linux-kernel, linux-fsdevel, linux-mm, Zi Yan
__GFP_ACCOUNT is needed for xarray node allocation accounting when
XA_FLAGS_ACCOUNT is set. Commit 7b785645e8f13 ("mm: fix page cache
convergence regression") fixed a workingset regression with it.
xas_split_alloc() does not have it and needs to be fixed.
In addition, based on Sashiko's review[1] and Johannes' confirmation[2], to
charge the right memcg, folio's memcg needs to be active during folio
split. Add that before adding __GFP_ACCOUNT.
There is no workingset convergence regression related to missing
__GFP_ACCOUNT in xas_split_alloc() and the impact to userspace should be
minor.
Both patches need be applied on top of the mapping_set_update() fix[3] to
avoid causing the same issue for uniform split path.
Link: https://sashiko.dev/#/patchset/20260727-add-gfp_account-to-xas_split_alloc-v1-1-9fae6bf64838%40nvidia.com?part=1 [1]
Link: https://lore.kernel.org/all/amtcBZ-_QVRgCd6b@cmpxchg.org/ [2]
Link: https://lore.kernel.org/all/20260725101419.3938406-1-matt@readmodwrite.com/ [3]
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
Changes in v2:
1. added memcg switch code to __folio_split() to make __GFP_ACCOUNT charge
to the right memcg.
2. removed RFC.
- Link to v1: https://lore.kernel.org/r/20260727-add-gfp_account-to-xas_split_alloc-v1-1-9fae6bf64838@nvidia.com
---
Zi Yan (2):
mm/huge_memory: use folio's memcg inside __folio_split()
xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc()
lib/xarray.c | 3 +++
mm/huge_memory.c | 20 ++++++++++++++++----
2 files changed, 19 insertions(+), 4 deletions(-)
---
base-commit: 34cacd0ac7c10dc9e34e1b9f28a2b78d335cb487
change-id: 20260727-add-gfp_account-to-xas_split_alloc-83a1bc848b77
Best regards,
--
Yan, Zi
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-07-31 2:17 [PATCH v2 0/2] Honor XA_FLAGS_ACCOUNT in xas_split_alloc() and charge to folio's memcg Zi Yan
@ 2026-07-31 2:18 ` Zi Yan
2026-08-01 6:57 ` Johannes Weiner
` (2 more replies)
2026-07-31 2:18 ` [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc() Zi Yan
1 sibling, 3 replies; 19+ messages in thread
From: Zi Yan @ 2026-07-31 2:18 UTC (permalink / raw)
To: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Lorenzo Stoakes, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif
Cc: linux-kernel, linux-fsdevel, linux-mm, Zi Yan
During a pagecache folio split, an xarray node allocation can happen and
needs to charge at folio's memcg instead of folio split invoker's memcg,
because for example folio split can happen during reclaim and reclaim's
active memcg might not be folio's memcg. Switch to folio's memcg at the
beginning and switch back afterwards.
Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
mm/huge_memory.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 04e8a6b553435..b9c2d8908e564 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -4063,34 +4063,42 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
XA_STATE(xas, &folio->mapping->i_pages, folio->index);
struct folio *end_folio = folio_next(folio);
bool is_anon = folio_test_anon(folio);
+ struct mem_cgroup *memcg, *old_memcg;
struct address_space *mapping = NULL;
struct anon_vma *anon_vma = NULL;
int old_order = folio_order(folio);
struct folio *new_folio, *next;
int nr_shmem_dropped = 0;
enum ttu_flags ttu_flags = 0;
- int ret;
pgoff_t end = 0;
+ int ret;
VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
ret = -EINVAL;
- goto out;
+ goto out_no_memcg;
}
if (new_order >= old_order) {
ret = -EINVAL;
- goto out;
+ goto out_no_memcg;
}
ret = folio_check_splittable(folio, new_order, split_type);
if (ret) {
VM_WARN_ONCE(ret == -EINVAL, "Tried to split an unsplittable folio");
- goto out;
+ goto out_no_memcg;
}
+ /*
+ * switch to folio's memcg as xarray node allocation can happen and
+ * needs to charge to it.
+ */
+ memcg = get_mem_cgroup_from_folio(folio);
+ old_memcg = set_active_memcg(memcg);
+
if (is_anon) {
/*
* The caller does not necessarily hold an mmap_lock that would
@@ -4231,6 +4239,10 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
if (mapping)
i_mmap_unlock_read(mapping);
out:
+ /* restore to caller's old_memcg */
+ set_active_memcg(old_memcg);
+ mem_cgroup_put(memcg);
+out_no_memcg:
xas_destroy(&xas);
if (is_pmd_order(old_order))
count_vm_event(!ret ? THP_SPLIT_PAGE : THP_SPLIT_PAGE_FAILED);
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc()
2026-07-31 2:17 [PATCH v2 0/2] Honor XA_FLAGS_ACCOUNT in xas_split_alloc() and charge to folio's memcg Zi Yan
2026-07-31 2:18 ` [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split() Zi Yan
@ 2026-07-31 2:18 ` Zi Yan
2026-08-01 6:58 ` Johannes Weiner
2026-08-01 9:38 ` Lorenzo Stoakes (ARM)
1 sibling, 2 replies; 19+ messages in thread
From: Zi Yan @ 2026-07-31 2:18 UTC (permalink / raw)
To: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Lorenzo Stoakes, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif
Cc: linux-kernel, linux-fsdevel, linux-mm, Zi Yan
XArray operations that allocate xa_nodes, such as xas_nomem() and
xas_alloc(), add __GFP_ACCOUNT when the array has XA_FLAGS_ACCOUNT set.
This charges the allocated memory and avoids the workingset convergence
issue described by commit 7b785645e8f13 ("mm: fix page cache convergence
regression").
xas_split_alloc() does not have that flag. Add it when necessary.
Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
lib/xarray.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/xarray.c b/lib/xarray.c
index 9a8b4916540cf..bfe7bef80f34e 100644
--- a/lib/xarray.c
+++ b/lib/xarray.c
@@ -1053,6 +1053,9 @@ void xas_split_alloc(struct xa_state *xas, void *entry, unsigned int order,
if (xas->xa_shift + XA_CHUNK_SHIFT > order)
return;
+ if (xas->xa->xa_flags & XA_FLAGS_ACCOUNT)
+ gfp |= __GFP_ACCOUNT;
+
do {
struct xa_node *node;
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-07-31 2:18 ` [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split() Zi Yan
@ 2026-08-01 6:57 ` Johannes Weiner
2026-08-01 9:36 ` Lorenzo Stoakes (ARM)
2026-08-03 2:39 ` Baolin Wang
2 siblings, 0 replies; 19+ messages in thread
From: Johannes Weiner @ 2026-08-01 6:57 UTC (permalink / raw)
To: Zi Yan
Cc: Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Lorenzo Stoakes, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, linux-kernel, linux-fsdevel, linux-mm
On Thu, Jul 30, 2026 at 10:18:00PM -0400, Zi Yan wrote:
> During a pagecache folio split, an xarray node allocation can happen and
> needs to charge at folio's memcg instead of folio split invoker's memcg,
> because for example folio split can happen during reclaim and reclaim's
> active memcg might not be folio's memcg. Switch to folio's memcg at the
> beginning and switch back afterwards.
>
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
> Signed-off-by: Zi Yan <ziy@nvidia.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc()
2026-07-31 2:18 ` [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc() Zi Yan
@ 2026-08-01 6:58 ` Johannes Weiner
2026-08-01 9:38 ` Lorenzo Stoakes (ARM)
1 sibling, 0 replies; 19+ messages in thread
From: Johannes Weiner @ 2026-08-01 6:58 UTC (permalink / raw)
To: Zi Yan
Cc: Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Lorenzo Stoakes, Baolin Wang, Liam R. Howlett,
Nico Pache, Ryan Roberts, Dev Jain, Barry Song, Lance Yang,
Usama Arif, linux-kernel, linux-fsdevel, linux-mm
On Thu, Jul 30, 2026 at 10:18:01PM -0400, Zi Yan wrote:
> XArray operations that allocate xa_nodes, such as xas_nomem() and
> xas_alloc(), add __GFP_ACCOUNT when the array has XA_FLAGS_ACCOUNT set.
> This charges the allocated memory and avoids the workingset convergence
> issue described by commit 7b785645e8f13 ("mm: fix page cache convergence
> regression").
>
> xas_split_alloc() does not have that flag. Add it when necessary.
>
> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
> Signed-off-by: Zi Yan <ziy@nvidia.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-07-31 2:18 ` [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split() Zi Yan
2026-08-01 6:57 ` Johannes Weiner
@ 2026-08-01 9:36 ` Lorenzo Stoakes (ARM)
2026-08-03 15:02 ` Zi Yan
2026-08-03 2:39 ` Baolin Wang
2 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-01 9:36 UTC (permalink / raw)
To: Zi Yan
Cc: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Baolin Wang, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
linux-kernel, linux-fsdevel, linux-mm
On Thu, Jul 30, 2026 at 10:18:00PM -0400, Zi Yan wrote:
> During a pagecache folio split, an xarray node allocation can happen and
> needs to charge at folio's memcg instead of folio split invoker's memcg,
> because for example folio split can happen during reclaim and reclaim's
> active memcg might not be folio's memcg. Switch to folio's memcg at the
> beginning and switch back afterwards.
I assume this is the only allocation? I guess in general it makes sense to have
the folio's memcg be active here regardless.
>
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
Cc: stable?
> Signed-off-by: Zi Yan <ziy@nvidia.com>
Change seems reasonable overall.
Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> mm/huge_memory.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 04e8a6b553435..b9c2d8908e564 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4063,34 +4063,42 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> XA_STATE(xas, &folio->mapping->i_pages, folio->index);
> struct folio *end_folio = folio_next(folio);
> bool is_anon = folio_test_anon(folio);
> + struct mem_cgroup *memcg, *old_memcg;
> struct address_space *mapping = NULL;
> struct anon_vma *anon_vma = NULL;
> int old_order = folio_order(folio);
> struct folio *new_folio, *next;
> int nr_shmem_dropped = 0;
> enum ttu_flags ttu_flags = 0;
> - int ret;
> pgoff_t end = 0;
> + int ret;
>
> VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
>
> if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
> ret = -EINVAL;
> - goto out;
> + goto out_no_memcg;
> }
>
> if (new_order >= old_order) {
> ret = -EINVAL;
> - goto out;
> + goto out_no_memcg;
> }
>
> ret = folio_check_splittable(folio, new_order, split_type);
> if (ret) {
> VM_WARN_ONCE(ret == -EINVAL, "Tried to split an unsplittable folio");
> - goto out;
> + goto out_no_memcg;
This function really badly needs splitting up and probably some cleanup.h work :)
> }
>
> + /*
> + * switch to folio's memcg as xarray node allocation can happen and
> + * needs to charge to it.
> + */
> + memcg = get_mem_cgroup_from_folio(folio);
> + old_memcg = set_active_memcg(memcg);
> +
> if (is_anon) {
> /*
> * The caller does not necessarily hold an mmap_lock that would
> @@ -4231,6 +4239,10 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> if (mapping)
> i_mmap_unlock_read(mapping);
> out:
> + /* restore to caller's old_memcg */
> + set_active_memcg(old_memcg);
> + mem_cgroup_put(memcg);
> +out_no_memcg:
> xas_destroy(&xas);
> if (is_pmd_order(old_order))
> count_vm_event(!ret ? THP_SPLIT_PAGE : THP_SPLIT_PAGE_FAILED);
>
> --
> 2.53.0
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc()
2026-07-31 2:18 ` [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc() Zi Yan
2026-08-01 6:58 ` Johannes Weiner
@ 2026-08-01 9:38 ` Lorenzo Stoakes (ARM)
2026-08-03 15:12 ` Zi Yan
1 sibling, 1 reply; 19+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-01 9:38 UTC (permalink / raw)
To: Zi Yan
Cc: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Baolin Wang, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
linux-kernel, linux-fsdevel, linux-mm
On Thu, Jul 30, 2026 at 10:18:01PM -0400, Zi Yan wrote:
> XArray operations that allocate xa_nodes, such as xas_nomem() and
> xas_alloc(), add __GFP_ACCOUNT when the array has XA_FLAGS_ACCOUNT set.
> This charges the allocated memory and avoids the workingset convergence
> issue described by commit 7b785645e8f13 ("mm: fix page cache convergence
> regression").
>
> xas_split_alloc() does not have that flag. Add it when necessary.
Nit but maybe 'split' rather than 'have'?
>
> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
Cc: stable?
> Signed-off-by: Zi Yan <ziy@nvidia.com>
Makes sense to me so:
Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> lib/xarray.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/lib/xarray.c b/lib/xarray.c
> index 9a8b4916540cf..bfe7bef80f34e 100644
> --- a/lib/xarray.c
> +++ b/lib/xarray.c
> @@ -1053,6 +1053,9 @@ void xas_split_alloc(struct xa_state *xas, void *entry, unsigned int order,
> if (xas->xa_shift + XA_CHUNK_SHIFT > order)
> return;
>
> + if (xas->xa->xa_flags & XA_FLAGS_ACCOUNT)
> + gfp |= __GFP_ACCOUNT;
> +
This is some confluence of flags :) I wonder if there are other places we've
missed setting this for?
> do {
> struct xa_node *node;
>
>
> --
> 2.53.0
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-07-31 2:18 ` [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split() Zi Yan
2026-08-01 6:57 ` Johannes Weiner
2026-08-01 9:36 ` Lorenzo Stoakes (ARM)
@ 2026-08-03 2:39 ` Baolin Wang
2 siblings, 0 replies; 19+ messages in thread
From: Baolin Wang @ 2026-08-03 2:39 UTC (permalink / raw)
To: Zi Yan, Johannes Weiner, Andrew Morton, Matthew Wilcox,
William Kucharski, David Hildenbrand, Lorenzo Stoakes,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Usama Arif
Cc: linux-kernel, linux-fsdevel, linux-mm
On 7/31/26 10:18 AM, Zi Yan wrote:
> During a pagecache folio split, an xarray node allocation can happen and
> needs to charge at folio's memcg instead of folio split invoker's memcg,
> because for example folio split can happen during reclaim and reclaim's
> active memcg might not be folio's memcg. Switch to folio's memcg at the
> beginning and switch back afterwards.
>
> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
LGTM.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-01 9:36 ` Lorenzo Stoakes (ARM)
@ 2026-08-03 15:02 ` Zi Yan
2026-08-03 15:07 ` Lorenzo Stoakes (ARM)
2026-08-04 20:47 ` Johannes Weiner
0 siblings, 2 replies; 19+ messages in thread
From: Zi Yan @ 2026-08-03 15:02 UTC (permalink / raw)
To: Johannes Weiner, Lorenzo Stoakes (ARM)
Cc: Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Baolin Wang, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
linux-kernel, linux-fsdevel, linux-mm
On Sat Aug 1, 2026 at 5:36 AM EDT, Lorenzo Stoakes (ARM) wrote:
> On Thu, Jul 30, 2026 at 10:18:00PM -0400, Zi Yan wrote:
>> During a pagecache folio split, an xarray node allocation can happen and
>> needs to charge at folio's memcg instead of folio split invoker's memcg,
>> because for example folio split can happen during reclaim and reclaim's
>> active memcg might not be folio's memcg. Switch to folio's memcg at the
>> beginning and switch back afterwards.
>
> I assume this is the only allocation? I guess in general it makes sense to have
> the folio's memcg be active here regardless.
>
>>
>> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
>> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
>
> Cc: stable?
Like you said above, only xas_split_alloc() is affected. And we have not
seen related workingset regression report (like what Johannes reported
in commit 7b785645e8f13 ("mm: fix page cache convergence regression")).
It might be OK to not backport.
Johannes, what is your take on this?
>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> Change seems reasonable overall.
>
> Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>
>> ---
>> mm/huge_memory.c | 20 ++++++++++++++++----
>> 1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 04e8a6b553435..b9c2d8908e564 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -4063,34 +4063,42 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>> XA_STATE(xas, &folio->mapping->i_pages, folio->index);
>> struct folio *end_folio = folio_next(folio);
>> bool is_anon = folio_test_anon(folio);
>> + struct mem_cgroup *memcg, *old_memcg;
>> struct address_space *mapping = NULL;
>> struct anon_vma *anon_vma = NULL;
>> int old_order = folio_order(folio);
>> struct folio *new_folio, *next;
>> int nr_shmem_dropped = 0;
>> enum ttu_flags ttu_flags = 0;
>> - int ret;
>> pgoff_t end = 0;
>> + int ret;
>>
>> VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
>> VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
>>
>> if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
>> ret = -EINVAL;
>> - goto out;
>> + goto out_no_memcg;
>> }
>>
>> if (new_order >= old_order) {
>> ret = -EINVAL;
>> - goto out;
>> + goto out_no_memcg;
>> }
>>
>> ret = folio_check_splittable(folio, new_order, split_type);
>> if (ret) {
>> VM_WARN_ONCE(ret == -EINVAL, "Tried to split an unsplittable folio");
>> - goto out;
>> + goto out_no_memcg;
>
> This function really badly needs splitting up and probably some cleanup.h work :)
You mean folio_check_splittable()? You want to move -EINVAL checks a
separate one?
--
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-03 15:02 ` Zi Yan
@ 2026-08-03 15:07 ` Lorenzo Stoakes (ARM)
2026-08-03 15:23 ` Zi Yan
2026-08-04 20:47 ` Johannes Weiner
1 sibling, 1 reply; 19+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-03 15:07 UTC (permalink / raw)
To: Zi Yan
Cc: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Baolin Wang, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
linux-kernel, linux-fsdevel, linux-mm
On Mon, Aug 03, 2026 at 11:02:30AM -0400, Zi Yan wrote:
> On Sat Aug 1, 2026 at 5:36 AM EDT, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Jul 30, 2026 at 10:18:00PM -0400, Zi Yan wrote:
> >> During a pagecache folio split, an xarray node allocation can happen and
> >> needs to charge at folio's memcg instead of folio split invoker's memcg,
> >> because for example folio split can happen during reclaim and reclaim's
> >> active memcg might not be folio's memcg. Switch to folio's memcg at the
> >> beginning and switch back afterwards.
> >
> > I assume this is the only allocation? I guess in general it makes sense to have
> > the folio's memcg be active here regardless.
> >
> >>
> >> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> >> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
> >
> > Cc: stable?
>
> Like you said above, only xas_split_alloc() is affected. And we have not
> seen related workingset regression report (like what Johannes reported
> in commit 7b785645e8f13 ("mm: fix page cache convergence regression")).
> It might be OK to not backport.
>
> Johannes, what is your take on this?
>
> >
> >> Signed-off-by: Zi Yan <ziy@nvidia.com>
> >
> > Change seems reasonable overall.
> >
> > Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> >
> >> ---
> >> mm/huge_memory.c | 20 ++++++++++++++++----
> >> 1 file changed, 16 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> index 04e8a6b553435..b9c2d8908e564 100644
> >> --- a/mm/huge_memory.c
> >> +++ b/mm/huge_memory.c
> >> @@ -4063,34 +4063,42 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> >> XA_STATE(xas, &folio->mapping->i_pages, folio->index);
> >> struct folio *end_folio = folio_next(folio);
> >> bool is_anon = folio_test_anon(folio);
> >> + struct mem_cgroup *memcg, *old_memcg;
> >> struct address_space *mapping = NULL;
> >> struct anon_vma *anon_vma = NULL;
> >> int old_order = folio_order(folio);
> >> struct folio *new_folio, *next;
> >> int nr_shmem_dropped = 0;
> >> enum ttu_flags ttu_flags = 0;
> >> - int ret;
> >> pgoff_t end = 0;
> >> + int ret;
> >>
> >> VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> >> VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
> >>
> >> if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
> >> ret = -EINVAL;
> >> - goto out;
> >> + goto out_no_memcg;
> >> }
> >>
> >> if (new_order >= old_order) {
> >> ret = -EINVAL;
> >> - goto out;
> >> + goto out_no_memcg;
> >> }
> >>
> >> ret = folio_check_splittable(folio, new_order, split_type);
> >> if (ret) {
> >> VM_WARN_ONCE(ret == -EINVAL, "Tried to split an unsplittable folio");
> >> - goto out;
> >> + goto out_no_memcg;
> >
> > This function really badly needs splitting up and probably some cleanup.h work :)
>
> You mean folio_check_splittable()? You want to move -EINVAL checks a
> separate one?
No __folio_split().
Comment about cleanup.h really was the whole pattern of goto xxx for various
levels of unwinding things.
But really I mean the folio splitting code in general, there's a lot of
massive-complicated-functions with a million things going on at once,
__folio_freeze_and_split_unmapped() is another.
Feels like we should really have this stuff in something like mm/folio.c anyway
too now that's renamed :)
>
> --
> Best Regards,
> Yan, Zi
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc()
2026-08-01 9:38 ` Lorenzo Stoakes (ARM)
@ 2026-08-03 15:12 ` Zi Yan
0 siblings, 0 replies; 19+ messages in thread
From: Zi Yan @ 2026-08-03 15:12 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Baolin Wang, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
linux-kernel, linux-fsdevel, linux-mm
On Sat Aug 1, 2026 at 5:38 AM EDT, Lorenzo Stoakes (ARM) wrote:
> On Thu, Jul 30, 2026 at 10:18:01PM -0400, Zi Yan wrote:
>> XArray operations that allocate xa_nodes, such as xas_nomem() and
>> xas_alloc(), add __GFP_ACCOUNT when the array has XA_FLAGS_ACCOUNT set.
>> This charges the allocated memory and avoids the workingset convergence
>> issue described by commit 7b785645e8f13 ("mm: fix page cache convergence
>> regression").
>>
>> xas_split_alloc() does not have that flag. Add it when necessary.
>
> Nit but maybe 'split' rather than 'have'?
Yeah, the sentence is pretty vague. How about?
xas_split_alloc() does not add _GFP_ACCOUNT when XA_FLAGS_ACCOUNT is
present. Add code to do it.
>
>>
>> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
>
> Cc: stable?
This should go along with Patch 1. So if we decided to backport Patch 1,
I will Cc: stable for this as well.
>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> Makes sense to me so:
>
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>
>> ---
>> lib/xarray.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/lib/xarray.c b/lib/xarray.c
>> index 9a8b4916540cf..bfe7bef80f34e 100644
>> --- a/lib/xarray.c
>> +++ b/lib/xarray.c
>> @@ -1053,6 +1053,9 @@ void xas_split_alloc(struct xa_state *xas, void *entry, unsigned int order,
>> if (xas->xa_shift + XA_CHUNK_SHIFT > order)
>> return;
>>
>> + if (xas->xa->xa_flags & XA_FLAGS_ACCOUNT)
>> + gfp |= __GFP_ACCOUNT;
>> +
>
> This is some confluence of flags :) I wonder if there are other places we've
> missed setting this for?
I did check the whole lib/xarray.c. xas_nomem(), __xas_nomem(),
xas_alloc(), xas_try_split(), and xas_split_alloc() are the ones using
gfp to allocate memory. Only xas_split_alloc() does not add
__GFP_ACCOUNT for XA_FLAGS_ACCOUNT.
--
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-03 15:07 ` Lorenzo Stoakes (ARM)
@ 2026-08-03 15:23 ` Zi Yan
2026-08-03 17:25 ` Kairui Song
2026-08-05 10:52 ` Lorenzo Stoakes (ARM)
0 siblings, 2 replies; 19+ messages in thread
From: Zi Yan @ 2026-08-03 15:23 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Baolin Wang, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
linux-kernel, linux-fsdevel, linux-mm
On Mon Aug 3, 2026 at 11:07 AM EDT, Lorenzo Stoakes (ARM) wrote:
> On Mon, Aug 03, 2026 at 11:02:30AM -0400, Zi Yan wrote:
>> On Sat Aug 1, 2026 at 5:36 AM EDT, Lorenzo Stoakes (ARM) wrote:
>> > On Thu, Jul 30, 2026 at 10:18:00PM -0400, Zi Yan wrote:
>> >> During a pagecache folio split, an xarray node allocation can happen and
>> >> needs to charge at folio's memcg instead of folio split invoker's memcg,
>> >> because for example folio split can happen during reclaim and reclaim's
>> >> active memcg might not be folio's memcg. Switch to folio's memcg at the
>> >> beginning and switch back afterwards.
>> >
>> > I assume this is the only allocation? I guess in general it makes sense to have
>> > the folio's memcg be active here regardless.
>> >
>> >>
>> >> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
>> >> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
>> >
>> > Cc: stable?
>>
>> Like you said above, only xas_split_alloc() is affected. And we have not
>> seen related workingset regression report (like what Johannes reported
>> in commit 7b785645e8f13 ("mm: fix page cache convergence regression")).
>> It might be OK to not backport.
>>
>> Johannes, what is your take on this?
>>
>> >
>> >> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> >
>> > Change seems reasonable overall.
>> >
>> > Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>> >
>> >> ---
>> >> mm/huge_memory.c | 20 ++++++++++++++++----
>> >> 1 file changed, 16 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> >> index 04e8a6b553435..b9c2d8908e564 100644
>> >> --- a/mm/huge_memory.c
>> >> +++ b/mm/huge_memory.c
>> >> @@ -4063,34 +4063,42 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>> >> XA_STATE(xas, &folio->mapping->i_pages, folio->index);
>> >> struct folio *end_folio = folio_next(folio);
>> >> bool is_anon = folio_test_anon(folio);
>> >> + struct mem_cgroup *memcg, *old_memcg;
>> >> struct address_space *mapping = NULL;
>> >> struct anon_vma *anon_vma = NULL;
>> >> int old_order = folio_order(folio);
>> >> struct folio *new_folio, *next;
>> >> int nr_shmem_dropped = 0;
>> >> enum ttu_flags ttu_flags = 0;
>> >> - int ret;
>> >> pgoff_t end = 0;
>> >> + int ret;
>> >>
>> >> VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
>> >> VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
>> >>
>> >> if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
>> >> ret = -EINVAL;
>> >> - goto out;
>> >> + goto out_no_memcg;
>> >> }
>> >>
>> >> if (new_order >= old_order) {
>> >> ret = -EINVAL;
>> >> - goto out;
>> >> + goto out_no_memcg;
>> >> }
>> >>
>> >> ret = folio_check_splittable(folio, new_order, split_type);
>> >> if (ret) {
>> >> VM_WARN_ONCE(ret == -EINVAL, "Tried to split an unsplittable folio");
>> >> - goto out;
>> >> + goto out_no_memcg;
>> >
>> > This function really badly needs splitting up and probably some cleanup.h work :)
>>
>> You mean folio_check_splittable()? You want to move -EINVAL checks a
>> separate one?
>
> No __folio_split().
>
> Comment about cleanup.h really was the whole pattern of goto xxx for various
> levels of unwinding things.
>
> But really I mean the folio splitting code in general, there's a lot of
> massive-complicated-functions with a million things going on at once,
> __folio_freeze_and_split_unmapped() is another.
>
> Feels like we should really have this stuff in something like mm/folio.c anyway
> too now that's renamed :)
>
I agree that __folio_split() is handling multiple cases, anon, shmem,
pagecache, all together. Do you prefer:
1. split __folio_split() to handle each case in a separate function with
some code duplication, like xarray for pagecache and shmem,
freeze/unfreeze folio for all;
or
2. encapulate per-case code in small functions, like
if (is_anon)
split_prepare_anon();
else
split_prepare_file_backed();
__folio_freeze_and_split_unmapped();
if (is_anon)
post_split_anon();
else
post_split_file_backed();
--
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-03 15:23 ` Zi Yan
@ 2026-08-03 17:25 ` Kairui Song
2026-08-03 17:55 ` Zi Yan
2026-08-05 10:52 ` Lorenzo Stoakes (ARM)
1 sibling, 1 reply; 19+ messages in thread
From: Kairui Song @ 2026-08-03 17:25 UTC (permalink / raw)
To: Zi Yan
Cc: Lorenzo Stoakes (ARM), Johannes Weiner, Andrew Morton,
Matthew Wilcox, William Kucharski, David Hildenbrand, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Usama Arif, linux-kernel, linux-fsdevel, linux-mm
On Tue, Aug 4, 2026 at 12:53 AM Zi Yan <ziy@nvidia.com> wrote:
>
> On Mon Aug 3, 2026 at 11:07 AM EDT, Lorenzo Stoakes (ARM) wrote:
> > No __folio_split().
> >
> > Comment about cleanup.h really was the whole pattern of goto xxx for various
> > levels of unwinding things.
> >
> > But really I mean the folio splitting code in general, there's a lot of
> > massive-complicated-functions with a million things going on at once,
> > __folio_freeze_and_split_unmapped() is another.
> >
> > Feels like we should really have this stuff in something like mm/folio.c anyway
> > too now that's renamed :)
> >
>
> I agree that __folio_split() is handling multiple cases, anon, shmem,
> pagecache, all together. Do you prefer:
>
> 1. split __folio_split() to handle each case in a separate function with
> some code duplication, like xarray for pagecache and shmem,
> freeze/unfreeze folio for all;
Hi all,
Do you like a __folio_freeze_split_unmap /
__folio_freeze_split_unmap_file split? :), I'm asking this as I'm
currently trying to sort up the mess about swap cache in huge_memory.c
and found it will be much cleaner if we move file related code into
__folio_freeze_split_unmap_file, and let __folio_freeze_split_unmap
handle anon and swap cache, and then saw the discussion here. (A bit
more detail on this, I think we ca just assume we just don't need or
want shmem swapcache split, because shmem swap cache is meant to be an
intermediate state during IO, and shmem drops swap cache once IO
compete, and hybrid half-tmpfs-half-swap state is really ugly and
should be avoided competely, swap cache lookup is still fine for shmem
just make sure the folio is not in shmem's mapping).
LOC seems lower with the split and swap part cleaned, they really
don't share much logic anyway, except for a for loop for putting the
splitted folio back to filemapping / swap cache, and a folio freeze
check.
And after doing that, for swap, splitting clean swap cache and
splitting swap cache to higher order are easily supported, a few ugly
checks are gone, we can't do that now partly because the code there is
really complex.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-03 17:25 ` Kairui Song
@ 2026-08-03 17:55 ` Zi Yan
2026-08-04 3:09 ` Kairui Song
0 siblings, 1 reply; 19+ messages in thread
From: Zi Yan @ 2026-08-03 17:55 UTC (permalink / raw)
To: Kairui Song
Cc: Lorenzo Stoakes (ARM), Johannes Weiner, Andrew Morton,
Matthew Wilcox, William Kucharski, David Hildenbrand, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Usama Arif, linux-kernel, linux-fsdevel, linux-mm
On 3 Aug 2026, at 13:25, Kairui Song wrote:
> On Tue, Aug 4, 2026 at 12:53 AM Zi Yan <ziy@nvidia.com> wrote:
>>
>> On Mon Aug 3, 2026 at 11:07 AM EDT, Lorenzo Stoakes (ARM) wrote:
>>> No __folio_split().
>>>
>>> Comment about cleanup.h really was the whole pattern of goto xxx for various
>>> levels of unwinding things.
>>>
>>> But really I mean the folio splitting code in general, there's a lot of
>>> massive-complicated-functions with a million things going on at once,
>>> __folio_freeze_and_split_unmapped() is another.
>>>
>>> Feels like we should really have this stuff in something like mm/folio.c anyway
>>> too now that's renamed :)
>>>
>>
>> I agree that __folio_split() is handling multiple cases, anon, shmem,
>> pagecache, all together. Do you prefer:
>>
>> 1. split __folio_split() to handle each case in a separate function with
>> some code duplication, like xarray for pagecache and shmem,
>> freeze/unfreeze folio for all;
>
> Hi all,
>
> Do you like a __folio_freeze_split_unmap /
> __folio_freeze_split_unmap_file split? :), I'm asking this as I'm
> currently trying to sort up the mess about swap cache in huge_memory.c
> and found it will be much cleaner if we move file related code into
> __folio_freeze_split_unmap_file, and let __folio_freeze_split_unmap
> handle anon and swap cache, and then saw the discussion here. (A bit
Sounds good to me.
Maybe s/__folio_freeze_split_unmap/__folio_freeze_split_unmap_anon/
to be specific? I assume shmem is handled in file part, since you said
below shmem in swapcache is not worth the support.
> more detail on this, I think we ca just assume we just don't need or
> want shmem swapcache split, because shmem swap cache is meant to be an
> intermediate state during IO, and shmem drops swap cache once IO
> compete, and hybrid half-tmpfs-half-swap state is really ugly and
> should be avoided competely, swap cache lookup is still fine for shmem
> just make sure the folio is not in shmem's mapping).
The reason looks good to me. Can you remove the shmem in swapcache TODO
and firmly say shmem in swapcache is not supported due to the above reason
when you split the code?
>
> LOC seems lower with the split and swap part cleaned, they really
> don't share much logic anyway, except for a for loop for putting the
> splitted folio back to filemapping / swap cache, and a folio freeze
> check.
>
> And after doing that, for swap, splitting clean swap cache and
> splitting swap cache to higher order are easily supported, a few ugly
> checks are gone, we can't do that now partly because the code there is
> really complex.
Looking forward to your patches. :)
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-03 17:55 ` Zi Yan
@ 2026-08-04 3:09 ` Kairui Song
2026-08-05 14:36 ` Zi Yan
0 siblings, 1 reply; 19+ messages in thread
From: Kairui Song @ 2026-08-04 3:09 UTC (permalink / raw)
To: Zi Yan
Cc: Lorenzo Stoakes (ARM), Johannes Weiner, Andrew Morton,
Matthew Wilcox, William Kucharski, David Hildenbrand, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Usama Arif, linux-kernel, linux-fsdevel, linux-mm
On Tue, Aug 4, 2026 at 1:55 AM Zi Yan <ziy@nvidia.com> wrote:
>
> On 3 Aug 2026, at 13:25, Kairui Song wrote:
>
> > On Tue, Aug 4, 2026 at 12:53 AM Zi Yan <ziy@nvidia.com> wrote:
> >>
> >> On Mon Aug 3, 2026 at 11:07 AM EDT, Lorenzo Stoakes (ARM) wrote:
> >>> No __folio_split().
> >>>
> >>> Comment about cleanup.h really was the whole pattern of goto xxx for various
> >>> levels of unwinding things.
> >>>
> >>> But really I mean the folio splitting code in general, there's a lot of
> >>> massive-complicated-functions with a million things going on at once,
> >>> __folio_freeze_and_split_unmapped() is another.
> >>>
> >>> Feels like we should really have this stuff in something like mm/folio.c anyway
> >>> too now that's renamed :)
> >>>
> >>
> >> I agree that __folio_split() is handling multiple cases, anon, shmem,
> >> pagecache, all together. Do you prefer:
> >>
> >> 1. split __folio_split() to handle each case in a separate function with
> >> some code duplication, like xarray for pagecache and shmem,
> >> freeze/unfreeze folio for all;
> >
> > Hi all,
> >
> > Do you like a __folio_freeze_split_unmap /
> > __folio_freeze_split_unmap_file split? :), I'm asking this as I'm
> > currently trying to sort up the mess about swap cache in huge_memory.c
> > and found it will be much cleaner if we move file related code into
> > __folio_freeze_split_unmap_file, and let __folio_freeze_split_unmap
> > handle anon and swap cache, and then saw the discussion here. (A bit
>
> Sounds good to me.
> Maybe s/__folio_freeze_split_unmap/__folio_freeze_split_unmap_anon/
> to be specific?
We will have to deal with clean (not yet added to anon) swap cache;
I'm not sure if that counts as anon? I'm fine either way about naming
though.
> > more detail on this, I think we ca just assume we just don't need or
> > want shmem swapcache split, because shmem swap cache is meant to be an
> > intermediate state during IO, and shmem drops swap cache once IO
> > compete, and hybrid half-tmpfs-half-swap state is really ugly and
> > should be avoided competely, swap cache lookup is still fine for shmem
> > just make sure the folio is not in shmem's mapping).
>
> The reason looks good to me. Can you remove the shmem in swapcache TODO
> and firmly say shmem in swapcache is not supported due to the above reason
> when you split the code?
Sure, will do. Thanks!
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-03 15:02 ` Zi Yan
2026-08-03 15:07 ` Lorenzo Stoakes (ARM)
@ 2026-08-04 20:47 ` Johannes Weiner
2026-08-04 21:27 ` Andrew Morton
1 sibling, 1 reply; 19+ messages in thread
From: Johannes Weiner @ 2026-08-04 20:47 UTC (permalink / raw)
To: Zi Yan
Cc: Lorenzo Stoakes (ARM), Andrew Morton, Matthew Wilcox,
William Kucharski, David Hildenbrand, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Usama Arif, linux-kernel, linux-fsdevel, linux-mm
On Mon, Aug 03, 2026 at 11:02:30AM -0400, Zi Yan wrote:
> On Sat Aug 1, 2026 at 5:36 AM EDT, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Jul 30, 2026 at 10:18:00PM -0400, Zi Yan wrote:
> >> During a pagecache folio split, an xarray node allocation can happen and
> >> needs to charge at folio's memcg instead of folio split invoker's memcg,
> >> because for example folio split can happen during reclaim and reclaim's
> >> active memcg might not be folio's memcg. Switch to folio's memcg at the
> >> beginning and switch back afterwards.
> >
> > I assume this is the only allocation? I guess in general it makes sense to have
> > the folio's memcg be active here regardless.
> >
> >>
> >> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> >> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
> >
> > Cc: stable?
>
> Like you said above, only xas_split_alloc() is affected. And we have not
> seen related workingset regression report (like what Johannes reported
> in commit 7b785645e8f13 ("mm: fix page cache convergence regression")).
> It might be OK to not backport.
>
> Johannes, what is your take on this?
Yeah, let's Cc stable.
It would be a bear to debug if you ran into this at scale. Which I
think you could with certain workloads.
The patches are straight-forward enough. It favors a backport.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-04 20:47 ` Johannes Weiner
@ 2026-08-04 21:27 ` Andrew Morton
0 siblings, 0 replies; 19+ messages in thread
From: Andrew Morton @ 2026-08-04 21:27 UTC (permalink / raw)
To: Johannes Weiner
Cc: Zi Yan, Lorenzo Stoakes (ARM), Matthew Wilcox, William Kucharski,
David Hildenbrand, Baolin Wang, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
linux-kernel, linux-fsdevel, linux-mm
On Tue, 4 Aug 2026 16:47:52 -0400 Johannes Weiner <hannes@cmpxchg.org> wrote:
> > Like you said above, only xas_split_alloc() is affected. And we have not
> > seen related workingset regression report (like what Johannes reported
> > in commit 7b785645e8f13 ("mm: fix page cache convergence regression")).
> > It might be OK to not backport.
> >
> > Johannes, what is your take on this?
>
> Yeah, let's Cc stable.
>
> It would be a bear to debug if you ran into this at scale. Which I
> think you could with certain workloads.
>
> The patches are straight-forward enough. It favors a backport.
I've added cc:stable to both and updated the [2/2] changelog as
suggested in
https://lore.kernel.org/all/DKFELWPHSJNO.3GGVW270HENEG@nvidia.com// So
afaik this series is good-to-go, thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-03 15:23 ` Zi Yan
2026-08-03 17:25 ` Kairui Song
@ 2026-08-05 10:52 ` Lorenzo Stoakes (ARM)
1 sibling, 0 replies; 19+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-05 10:52 UTC (permalink / raw)
To: Zi Yan
Cc: Johannes Weiner, Andrew Morton, Matthew Wilcox, William Kucharski,
David Hildenbrand, Baolin Wang, Liam R. Howlett, Nico Pache,
Ryan Roberts, Dev Jain, Barry Song, Lance Yang, Usama Arif,
linux-kernel, linux-fsdevel, linux-mm
On Mon, Aug 03, 2026 at 11:23:39AM -0400, Zi Yan wrote:
> On Mon Aug 3, 2026 at 11:07 AM EDT, Lorenzo Stoakes (ARM) wrote:
> > On Mon, Aug 03, 2026 at 11:02:30AM -0400, Zi Yan wrote:
> >> On Sat Aug 1, 2026 at 5:36 AM EDT, Lorenzo Stoakes (ARM) wrote:
> >> > On Thu, Jul 30, 2026 at 10:18:00PM -0400, Zi Yan wrote:
> >> >> During a pagecache folio split, an xarray node allocation can happen and
> >> >> needs to charge at folio's memcg instead of folio split invoker's memcg,
> >> >> because for example folio split can happen during reclaim and reclaim's
> >> >> active memcg might not be folio's memcg. Switch to folio's memcg at the
> >> >> beginning and switch back afterwards.
> >> >
> >> > I assume this is the only allocation? I guess in general it makes sense to have
> >> > the folio's memcg be active here regardless.
> >> >
> >> >>
> >> >> Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
> >> >> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
> >> >
> >> > Cc: stable?
> >>
> >> Like you said above, only xas_split_alloc() is affected. And we have not
> >> seen related workingset regression report (like what Johannes reported
> >> in commit 7b785645e8f13 ("mm: fix page cache convergence regression")).
> >> It might be OK to not backport.
> >>
> >> Johannes, what is your take on this?
> >>
> >> >
> >> >> Signed-off-by: Zi Yan <ziy@nvidia.com>
> >> >
> >> > Change seems reasonable overall.
> >> >
> >> > Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> >> >
> >> >> ---
> >> >> mm/huge_memory.c | 20 ++++++++++++++++----
> >> >> 1 file changed, 16 insertions(+), 4 deletions(-)
> >> >>
> >> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> >> index 04e8a6b553435..b9c2d8908e564 100644
> >> >> --- a/mm/huge_memory.c
> >> >> +++ b/mm/huge_memory.c
> >> >> @@ -4063,34 +4063,42 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> >> >> XA_STATE(xas, &folio->mapping->i_pages, folio->index);
> >> >> struct folio *end_folio = folio_next(folio);
> >> >> bool is_anon = folio_test_anon(folio);
> >> >> + struct mem_cgroup *memcg, *old_memcg;
> >> >> struct address_space *mapping = NULL;
> >> >> struct anon_vma *anon_vma = NULL;
> >> >> int old_order = folio_order(folio);
> >> >> struct folio *new_folio, *next;
> >> >> int nr_shmem_dropped = 0;
> >> >> enum ttu_flags ttu_flags = 0;
> >> >> - int ret;
> >> >> pgoff_t end = 0;
> >> >> + int ret;
> >> >>
> >> >> VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> >> >> VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
> >> >>
> >> >> if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
> >> >> ret = -EINVAL;
> >> >> - goto out;
> >> >> + goto out_no_memcg;
> >> >> }
> >> >>
> >> >> if (new_order >= old_order) {
> >> >> ret = -EINVAL;
> >> >> - goto out;
> >> >> + goto out_no_memcg;
> >> >> }
> >> >>
> >> >> ret = folio_check_splittable(folio, new_order, split_type);
> >> >> if (ret) {
> >> >> VM_WARN_ONCE(ret == -EINVAL, "Tried to split an unsplittable folio");
> >> >> - goto out;
> >> >> + goto out_no_memcg;
> >> >
> >> > This function really badly needs splitting up and probably some cleanup.h work :)
> >>
> >> You mean folio_check_splittable()? You want to move -EINVAL checks a
> >> separate one?
> >
> > No __folio_split().
> >
> > Comment about cleanup.h really was the whole pattern of goto xxx for various
> > levels of unwinding things.
> >
> > But really I mean the folio splitting code in general, there's a lot of
> > massive-complicated-functions with a million things going on at once,
> > __folio_freeze_and_split_unmapped() is another.
> >
> > Feels like we should really have this stuff in something like mm/folio.c anyway
> > too now that's renamed :)
> >
>
> I agree that __folio_split() is handling multiple cases, anon, shmem,
> pagecache, all together. Do you prefer:
>
> 1. split __folio_split() to handle each case in a separate function with
> some code duplication, like xarray for pagecache and shmem,
> freeze/unfreeze folio for all;
>
> or
>
> 2. encapulate per-case code in small functions, like
> if (is_anon)
> split_prepare_anon();
> else
> split_prepare_file_backed();
>
> __folio_freeze_and_split_unmapped();
>
> if (is_anon)
> post_split_anon();
> else
> post_split_file_backed();
Well these 'post' functions are a bit confusing so I guess I'd say experiment
with different approaches and see which ones end up with the nicest code :)
>
>
> --
> Best Regards,
> Yan, Zi
>
--
Cheers, Lorenzo
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()
2026-08-04 3:09 ` Kairui Song
@ 2026-08-05 14:36 ` Zi Yan
0 siblings, 0 replies; 19+ messages in thread
From: Zi Yan @ 2026-08-05 14:36 UTC (permalink / raw)
To: Kairui Song
Cc: Lorenzo Stoakes (ARM), Johannes Weiner, Andrew Morton,
Matthew Wilcox, William Kucharski, David Hildenbrand, Baolin Wang,
Liam R. Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Usama Arif, linux-kernel, linux-fsdevel, linux-mm
On 3 Aug 2026, at 23:09, Kairui Song wrote:
> On Tue, Aug 4, 2026 at 1:55 AM Zi Yan <ziy@nvidia.com> wrote:
>>
>> On 3 Aug 2026, at 13:25, Kairui Song wrote:
>>
>>> On Tue, Aug 4, 2026 at 12:53 AM Zi Yan <ziy@nvidia.com> wrote:
>>>>
>>>> On Mon Aug 3, 2026 at 11:07 AM EDT, Lorenzo Stoakes (ARM) wrote:
>>>>> No __folio_split().
>>>>>
>>>>> Comment about cleanup.h really was the whole pattern of goto xxx for various
>>>>> levels of unwinding things.
>>>>>
>>>>> But really I mean the folio splitting code in general, there's a lot of
>>>>> massive-complicated-functions with a million things going on at once,
>>>>> __folio_freeze_and_split_unmapped() is another.
>>>>>
>>>>> Feels like we should really have this stuff in something like mm/folio.c anyway
>>>>> too now that's renamed :)
>>>>>
>>>>
>>>> I agree that __folio_split() is handling multiple cases, anon, shmem,
>>>> pagecache, all together. Do you prefer:
>>>>
>>>> 1. split __folio_split() to handle each case in a separate function with
>>>> some code duplication, like xarray for pagecache and shmem,
>>>> freeze/unfreeze folio for all;
>>>
>>> Hi all,
>>>
>>> Do you like a __folio_freeze_split_unmap /
>>> __folio_freeze_split_unmap_file split? :), I'm asking this as I'm
>>> currently trying to sort up the mess about swap cache in huge_memory.c
>>> and found it will be much cleaner if we move file related code into
>>> __folio_freeze_split_unmap_file, and let __folio_freeze_split_unmap
>>> handle anon and swap cache, and then saw the discussion here. (A bit
>>
>> Sounds good to me.
>> Maybe s/__folio_freeze_split_unmap/__folio_freeze_split_unmap_anon/
>> to be specific?
>
> We will have to deal with clean (not yet added to anon) swap cache;
> I'm not sure if that counts as anon? I'm fine either way about naming
> though.
Pick the name you think makes most of sense. Naming is hard, we can always
discuss about it when your patch comes. :)
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2026-08-05 14:37 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-31 2:17 [PATCH v2 0/2] Honor XA_FLAGS_ACCOUNT in xas_split_alloc() and charge to folio's memcg Zi Yan
2026-07-31 2:18 ` [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split() Zi Yan
2026-08-01 6:57 ` Johannes Weiner
2026-08-01 9:36 ` Lorenzo Stoakes (ARM)
2026-08-03 15:02 ` Zi Yan
2026-08-03 15:07 ` Lorenzo Stoakes (ARM)
2026-08-03 15:23 ` Zi Yan
2026-08-03 17:25 ` Kairui Song
2026-08-03 17:55 ` Zi Yan
2026-08-04 3:09 ` Kairui Song
2026-08-05 14:36 ` Zi Yan
2026-08-05 10:52 ` Lorenzo Stoakes (ARM)
2026-08-04 20:47 ` Johannes Weiner
2026-08-04 21:27 ` Andrew Morton
2026-08-03 2:39 ` Baolin Wang
2026-07-31 2:18 ` [PATCH v2 2/2] xarray: honor XA_FLAGS_ACCOUNT in xas_split_alloc() Zi Yan
2026-08-01 6:58 ` Johannes Weiner
2026-08-01 9:38 ` Lorenzo Stoakes (ARM)
2026-08-03 15:12 ` Zi Yan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox