Skip to contentSkip to conversation

mm/huge_memory: Initialise workingset state before folio split

PATCH Applied Matt Fleming 3 participants 3 messages Jul 25 – Jul 27

Subsystems (3)
Stable branch Memory management THP
lore
Share & tools
lore

Matt Fleming

author
#0 lore raw

PATCH v2 1/1 mm/huge_memory: Initialise workingset state before folio split

From: Matt Fleming <[email protected]>

xas_try_split() adds __GFP_ACCOUNT for page-cache xa_nodes, but __folio_split() leaves the xa_state's xa_lru unset. That lets a live, memcg-charged xa_node exist without being linked into the mapping's shadow_nodes list_lru; when reclaim later walks the list_lru it trips VM_WARN_ON(!css_is_dying()).

Use mapping_set_update() to install both the workingset update callback and the shadow_nodes list_lru on the xa_state.

Reported-by [email protected]
Closes https://syzkaller.appspot.com/bug?extid=c5b060ce82921a2fd500
Fixes 58729c04cf10 ("mm/huge_memory: add buddy allocator like (non-uniform) folio_split()")
Cc [email protected]
Reviewed-by Zi Yan <[email protected]>
Signed-off-by Matt Fleming <[email protected]>

---
Changes in v2:
- Move mapping_set_update() after filemap_release_folio() succeeds.
- Add Zi Yan's Reviewed-by.

Link https://lore.kernel.org/linux-mm/[email protected]/ open here
---
 mm/huge_memory.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/huge_memory.c +3 -1
index b5d1e9d4463d..4ddbc72e92fd 100644
@@ -4033,7 +4033,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
 		gfp_t gfp;
 
 		mapping = folio->mapping;
-		min_order = mapping_min_folio_order(folio->mapping);
+		min_order = mapping_min_folio_order(mapping);
 		if (new_order < min_order) {
 			ret = -EINVAL;
 			goto out;

@@ -4047,6 +4047,8 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
 			goto out;
 		}
 
+		mapping_set_update(&xas, mapping);
+
 		if (split_type == SPLIT_TYPE_UNIFORM) {
 			xas_set_order(&xas, folio->index, new_order);
 			xas_split_alloc(&xas, folio, old_order, gfp);
Signature · 1 line
2.43.0
maintainer
#1 lore raw
Quoted 8 lines · On Sat, 25 Jul 2026 11:14:19 +0100 Matt Fleming <[email protected]> wrote:

xas_try_split() adds __GFP_ACCOUNT for page-cache xa_nodes, but __folio_split() leaves the xa_state's xa_lru unset. That lets a live, memcg-charged xa_node exist without being linked into the mapping's shadow_nodes list_lru; when reclaim later walks the list_lru it trips VM_WARN_ON(!css_is_dying()).

Use mapping_set_update() to install both the workingset update callback and the shadow_nodes list_lru on the xa_state.

Thanks, I'll queue this as a hotfix.

Sashiko is wrong, methinks. "Calling local_irq_disable() strictly disables hardware interrupts" isn't true on PREEMPT_RT.

	https://sashiko.dev/#/patchset/[email protected]
maintainer Acked-by on v2 1/1
#2 lore raw
In reply to Matt Fleming
Quoted 41 lines · Matt Fleming

From: Matt Fleming <[email protected]>

xas_try_split() adds __GFP_ACCOUNT for page-cache xa_nodes, but __folio_split() leaves the xa_state's xa_lru unset. That lets a live, memcg-charged xa_node exist without being linked into the mapping's shadow_nodes list_lru; when reclaim later walks the list_lru it trips VM_WARN_ON(!css_is_dying()).

Use mapping_set_update() to install both the workingset update callback and the shadow_nodes list_lru on the xa_state.

Reported-by [email protected]
Closes https://syzkaller.appspot.com/bug?extid=c5b060ce82921a2fd500
Fixes 58729c04cf10 ("mm/huge_memory: add buddy allocator like (non-uniform) folio_split()")
Cc [email protected]
Reviewed-by Zi Yan <[email protected]>
Signed-off-by Matt Fleming <[email protected]>

---
Changes in v2:
- Move mapping_set_update() after filemap_release_folio() succeeds.
- Add Zi Yan's Reviewed-by.

Link https://lore.kernel.org/linux-mm/[email protected]/ open here
---
 mm/huge_memory.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/huge_memory.c +1 -1
index b5d1e9d4463d..4ddbc72e92fd 100644
@@ -4033,7 +4033,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
 		gfp_t gfp;
 
 		mapping = folio->mapping;
-		min_order = mapping_min_folio_order(folio->mapping);
+		min_order = mapping_min_folio_order(mapping);
 		if (new_order < min_order) {
 			ret = -EINVAL;
 			goto out;

mm/huge_memory.c static int __folio_split(struct folio *folio, unsigned int new_order, · line 4051

Quoted 5 lines
mm/huge_memory.c +2 -0
@@ -4047,6 +4047,8 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
 			goto out;
 		}
 
+		mapping_set_update(&xas, mapping);
+

It's entirely unclear when mapping_set_update() should/must be called. ... or even what it does.

Why is it called "mapping_set_*" when, in fact, we modify the xas?

The more I look at it, the more angry it makes me :)

Can someone please try finding a better way to handle this?

E.g., can we somehow remember in the xarray that we have !dax_mapping(mapping) && !shmem_mapping(mapping) early, and just do the right thing from xas code? Or is there some way we could have a mixture in the same xarray?

Anyhow, for this patch here, I guess it's fine. I am convinced nobody can say that with confidence. Black magic.

Acked-by David Hildenbrand (Arm) <[email protected]>
Signature · 3 lines
Cheers,

David