mm/shmem: fix THP allocation and fallback loop

PATCH Applied Kairui Song 6 participants 8 messages Oct 23, 2025 – Oct 24, 2025

Subsystems (3)

Stable branch Memory management tmpfs (shmem filesystem)

lore

Share & tools

lore

Kairui Song

author

2025-10-23 06:59 UTC #0 lore raw

PATCH v3 1/1 mm/shmem: fix THP allocation and fallback loop

From: Kairui Song <[email protected]>

The order check and fallback loop is updating the index value on every loop, this will cause the index to be wrongly aligned by a larger value while the loop shrinks the order.

This may result in inserting and returning a folio of the wrong index and cause data corruption with some userspace workloads [1].

Cc [email protected]

Link https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/ [1] open here

Fixes e7a2ab7b3bb5d ("mm: shmem: add mTHP support for anonymous shmem")

Signed-off-by Kairui Song <[email protected]>

---

Changes from V2:
- Introduce a temporary variable to improve code,
no behavior change, generated code is identical.
- Link to V2: https://lore.kernel.org/linux-mm/[email protected]/

Changes from V1:
- Remove unnecessary cleanup and simplify the commit message.
- Link to V1: https://lore.kernel.org/linux-mm/[email protected]/

---
 mm/shmem.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

mm/shmem.c +6 -3

index b50ce7dbc84a..e1dc2d8e939c 100644
@@ -1882,6 +1882,7 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 	struct shmem_inode_info *info = SHMEM_I(inode);
 	unsigned long suitable_orders = 0;
 	struct folio *folio = NULL;
+	pgoff_t aligned_index;
 	long pages;
 	int error, order;

@@ -1895,10 +1896,12 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 		order = highest_order(suitable_orders);
 		while (suitable_orders) {
 			pages = 1UL << order;
-			index = round_down(index, pages);
-			folio = shmem_alloc_folio(gfp, order, info, index);
-			if (folio)
+			aligned_index = round_down(index, pages);
+			folio = shmem_alloc_folio(gfp, order, info, aligned_index);
+			if (folio) {
+				index = aligned_index;
 				goto allocated;
+			}
 
 			if (pages == HPAGE_PMD_NR)
 				count_vm_event(THP_FILE_FALLBACK);

Signature · 1 line

2.51.0

David Hildenbrand

Acked-by on v3 1/1

2025-10-23 16:13 UTC #1 lore raw

Quoted 44 lines · Kairui Song

From: Kairui Song <[email protected]>
The order check and fallback loop is updating the index value on every loop, this will cause the index to be wrongly aligned by a larger value while the loop shrinks the order.
This may result in inserting and returning a folio of the wrong index and cause data corruption with some userspace workloads [1].
Cc [email protected]
Link https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/ [1] open here
Fixes e7a2ab7b3bb5d ("mm: shmem: add mTHP support for anonymous shmem")
Signed-off-by Kairui Song <[email protected]>
---
Changes from V2:
- Introduce a temporary variable to improve code,
no behavior change, generated code is identical.
- Link to V2: https://lore.kernel.org/linux-mm/[email protected]/
Changes from V1:
- Remove unnecessary cleanup and simplify the commit message.
- Link to V1: https://lore.kernel.org/linux-mm/[email protected]/
---
  mm/shmem.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)
mm/shmem.c +1 -0
index b50ce7dbc84a..e1dc2d8e939c 100644
@@ -1882,6 +1882,7 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
  	struct shmem_inode_info *info = SHMEM_I(inode);
  	unsigned long suitable_orders = 0;
  	struct folio *folio = NULL;
+	pgoff_t aligned_index;
  	long pages;
  	int error, order;
 
@@ -1895,10 +1896,12 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
  		order = highest_order(suitable_orders);
  		while (suitable_orders) {

mm/shmem.c static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, · line 1904

Quoted 10 lines

mm/shmem.c +5 -3

  			pages = 1UL << order;
-			index = round_down(index, pages);
-			folio = shmem_alloc_folio(gfp, order, info, index);
-			if (folio)
+			aligned_index = round_down(index, pages);
+			folio = shmem_alloc_folio(gfp, order, info, aligned_index);
+			if (folio) {
+				index = aligned_index;
  				goto allocated;
+			}

Was the found by code inspection or was there a report about this?

Acked-by David Hildenbrand <[email protected]>

Signature · 3 lines

Cheers

David / dhildenb

David Hildenbrand

2025-10-23 16:14 UTC #2 lore raw

Quoted 42 lines · David Hildenbrand

Quoted 41 lines · Kairui Song
From: Kairui Song <[email protected]>
The order check and fallback loop is updating the index value on every loop, this will cause the index to be wrongly aligned by a larger value while the loop shrinks the order.
This may result in inserting and returning a folio of the wrong index and cause data corruption with some userspace workloads [1].
Cc [email protected]
Link https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/ [1] open here
Fixes e7a2ab7b3bb5d ("mm: shmem: add mTHP support for anonymous shmem")
Signed-off-by Kairui Song <[email protected]>
---
Changes from V2:
- Introduce a temporary variable to improve code,
no behavior change, generated code is identical.
- Link to V2: https://lore.kernel.org/linux-mm/[email protected]/
Changes from V1:
- Remove unnecessary cleanup and simplify the commit message.
- Link to V1: https://lore.kernel.org/linux-mm/[email protected]/
---
   mm/shmem.c | 9 ++++++---
   1 file changed, 6 insertions(+), 3 deletions(-)
mm/shmem.c +1 -0
index b50ce7dbc84a..e1dc2d8e939c 100644
@@ -1882,6 +1882,7 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
   	struct shmem_inode_info *info = SHMEM_I(inode);
   	unsigned long suitable_orders = 0;
   	struct folio *folio = NULL;
+	pgoff_t aligned_index;
   	long pages;
   	int error, order;
 

Quoted 15 lines

mm/shmem.c static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, · line 1904

Quoted 13 lines

mm/shmem.c +5 -3

@@ -1895,10 +1896,12 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
   		order = highest_order(suitable_orders);
   		while (suitable_orders) {
   			pages = 1UL << order;
-			index = round_down(index, pages);
-			folio = shmem_alloc_folio(gfp, order, info, index);
-			if (folio)
+			aligned_index = round_down(index, pages);
+			folio = shmem_alloc_folio(gfp, order, info, aligned_index);
+			if (folio) {
+				index = aligned_index;
   				goto allocated;
+			}

Was the found by code inspection or was there a report about this?

Answering my own question, the "Link:" above should be

Closes https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/ open here

Signature · 3 lines

Cheers

David / dhildenb

Kairui Song

author

2025-10-23 17:42 UTC #3 lore raw

Quoted 60 lines · David Hildenbrand

Quoted 42 lines · David Hildenbrand
Quoted 41 lines · Kairui Song
From: Kairui Song <[email protected]>
The order check and fallback loop is updating the index value on every loop, this will cause the index to be wrongly aligned by a larger value while the loop shrinks the order.
This may result in inserting and returning a folio of the wrong index and cause data corruption with some userspace workloads [1].
Cc [email protected]
Link https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/ [1] open here
Fixes e7a2ab7b3bb5d ("mm: shmem: add mTHP support for anonymous shmem")
Signed-off-by Kairui Song <[email protected]>
---
Changes from V2:
- Introduce a temporary variable to improve code,
no behavior change, generated code is identical.
- Link to V2: https://lore.kernel.org/linux-mm/[email protected]/
Changes from V1:
- Remove unnecessary cleanup and simplify the commit message.
- Link to V1: https://lore.kernel.org/linux-mm/[email protected]/
---
   mm/shmem.c | 9 ++++++---
   1 file changed, 6 insertions(+), 3 deletions(-)
mm/shmem.c +1 -0
index b50ce7dbc84a..e1dc2d8e939c 100644
@@ -1882,6 +1882,7 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
     struct shmem_inode_info *info = SHMEM_I(inode);
     unsigned long suitable_orders = 0;
     struct folio *folio = NULL;
+    pgoff_t aligned_index;
     long pages;
     int error, order;
 
Quoted 15 lines
mm/shmem.c static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, · line 1904
Quoted 13 lines
mm/shmem.c +5 -3
@@ -1895,10 +1896,12 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
             order = highest_order(suitable_orders);
             while (suitable_orders) {
                     pages = 1UL << order;
-                    index = round_down(index, pages);
-                    folio = shmem_alloc_folio(gfp, order, info, index);
-                    if (folio)
+                    aligned_index = round_down(index, pages);
+                    folio = shmem_alloc_folio(gfp, order, info, aligned_index);
+                    if (folio) {
+                            index = aligned_index;
                             goto allocated;
+                    }
Was the found by code inspection or was there a report about this?

Quoted 5 lines

Answering my own question, the "Link:" above should be
Closes:
https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/

Thanks for the review. It's reported by and fixed by me, so I didn't include an extra Report-By & Closes, I thought that's kind of redundant. Do we need that? Maybe Andrew can help add it :) ?

Barry Song

Reviewed-by

2025-10-24 00:47 UTC #4 lore raw

Quoted 10 lines

Quoted 6 lines
Answering my own question, the "Link:" above should be
Closes:
https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/
Thanks for the review. It's reported by and fixed by me, so I didn't include an extra Report-By & Closes, I thought that's kind of redundant. Do we need that? Maybe Andrew can help add it :) ?

I also think it’s better to use “Closes:”. In that case, we might need to slightly
adjust the commit log to remove "[1]" here.

" This may result in inserting and returning a folio of the wrong index
and cause data corruption with some userspace workloads [1]."

With that,

Reviewed-by Barry Song <[email protected]>

Thanks
Barry

Zi Yan

Acked-by on v3 1/1

2025-10-23 17:48 UTC #5 lore raw

In reply to Kairui Song

Quoted 21 lines · Kairui Song

From: Kairui Song <[email protected]>
The order check and fallback loop is updating the index value on every loop, this will cause the index to be wrongly aligned by a larger value while the loop shrinks the order.
This may result in inserting and returning a folio of the wrong index and cause data corruption with some userspace workloads [1].
Cc [email protected]
Link https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/ [1] open here
Fixes e7a2ab7b3bb5d ("mm: shmem: add mTHP support for anonymous shmem")
Signed-off-by Kairui Song <[email protected]>
---
Changes from V2:
- Introduce a temporary variable to improve code,
no behavior change, generated code is identical.
- Link to V2: https://lore.kernel.org/linux-mm/[email protected]/

Quoted 8 lines

Changes from V1:
- Remove unnecessary cleanup and simplify the commit message.
- Link to V1: https://lore.kernel.org/linux-mm/[email protected]/
---
 mm/shmem.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Acked-by Zi Yan <[email protected]>

--
Best Regards,
Yan, Zi

Baolin Wang

reviewer Reviewed-by on v3 1/1

2025-10-24 00:47 UTC #6 lore raw

In reply to Kairui Song

Quoted 9 lines · Kairui Song

From: Kairui Song <[email protected]>
The order check and fallback loop is updating the index value on every loop, this will cause the index to be wrongly aligned by a larger value while the loop shrinks the order.
This may result in inserting and returning a folio of the wrong index and cause data corruption with some userspace workloads [1].

Quoted 6 lines

Cc [email protected]
Link https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/ [1] open here
Fixes e7a2ab7b3bb5d ("mm: shmem: add mTHP support for anonymous shmem")
Signed-off-by Kairui Song <[email protected]>
---

LGTM. Thanks.

Reviewed-by Baolin Wang <[email protected]>

Lorenzo Stoakes

Reviewed-by on v3 1/1

2025-10-24 14:03 UTC #7 lore raw

In reply to Kairui Song

Quoted 6 lines · Kairui Song

From: Kairui Song <[email protected]>
The order check and fallback loop is updating the index value on every loop, this will cause the index to be wrongly aligned by a larger value while the loop shrinks the order.

Quoted 7 lines

This may result in inserting and returning a folio of the wrong index and cause data corruption with some userspace workloads [1].
Cc [email protected]
Link https://lore.kernel.org/linux-mm/CAMgjq7DqgAmj25nDUwwu1U2cSGSn8n4-Hqpgottedy0S6YYeUw@mail.gmail.com/ [1] open here
Fixes e7a2ab7b3bb5d ("mm: shmem: add mTHP support for anonymous shmem")
Signed-off-by Kairui Song <[email protected]>

Yikes... LGTM so:

Reviewed-by Lorenzo Stoakes <[email protected]>

See below for a small nit.

Quoted 21 lines

---
Changes from V2:
- Introduce a temporary variable to improve code,
no behavior change, generated code is identical.
- Link to V2: https://lore.kernel.org/linux-mm/[email protected]/
Changes from V1:
- Remove unnecessary cleanup and simplify the commit message.
- Link to V1: https://lore.kernel.org/linux-mm/[email protected]/
---
 mm/shmem.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
mm/shmem.c +0 -0
index b50ce7dbc84a..e1dc2d8e939c 100644

mm/shmem.c static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf, · line 1885

Quoted 4 lines

mm/shmem.c +1 -0

@@ -1882,6 +1882,7 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 	struct shmem_inode_info *info = SHMEM_I(inode);
 	unsigned long suitable_orders = 0;
 	struct folio *folio = NULL;
+	pgoff_t aligned_index;

Nit, but can't we just declare this in the loop? That makes it even clearer that we don't reuse the value.

Quoted 22 lines

mm/shmem.c +5 -4

 	long pages;
 	int error, order;

@@ -1895,10 +1896,12 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
 		order = highest_order(suitable_orders);
 		while (suitable_orders) {
 			pages = 1UL << order;
-			index = round_down(index, pages);
-			folio = shmem_alloc_folio(gfp, order, info, index);
-			if (folio)
+			aligned_index = round_down(index, pages);
+			folio = shmem_alloc_folio(gfp, order, info, aligned_index);
+			if (folio) {
+				index = aligned_index;
 				goto allocated;
+			}
 
 			if (pages == HPAGE_PMD_NR)
 				count_vm_event(THP_FILE_FALLBACK);
--

2.51.0