Re: [PATCH 08/16] mm/shmem: stop retrying folios that cannot be split

["David Hildenbrand (Arm)" <david@kernel.org>]

On 10/27/26 02:34, Zi Yan wrote:
> On Mon Oct 26, 2026 at 5:40 AM EDT, David Hildenbrand (Arm) wrote:
>> From: Chris Mason <mason@kernel.org>
>>
>> A fault or collapse can insert a large folio crossing EOF after
>> shmem_undo_range() has handled the boundary folios. If splitting that
>> folio fails, truncate_inode_partial_folio() retains it when it is dirty.
>> The final scan then repeatedly finds it because it restarts from an
>> unchanged start.
>>
>> Truncation keeps the inode lock and ignores fatal signals while looping.
>> A concurrent unlink() can then block other directory operations while
>> holding the directory lock and waiting for the file's lock.
>>
>> Advance start past the retained folio, whose truncated part has already
>> been zeroed. Do not move start backwards if the failed split has shrunk
>> the folio.
>>
>> Fix generated by an LLM and sent by Chris Mason in private.
>>
>> Fixes: 55ac8bbe358b ("mm/shmem: fix race in shmem_undo_range w/THP")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Chris Mason <mason@kernel.org>
>> [ david: split fixes, simplify descriptions, adjust comments ]
>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>> ---
>>  mm/shmem.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
> 
> I rebased your patch on mm-stable, which has Zhang Yi's fix, and
> 1. tweaked the comment,
> 2. skipped the dropped folio (this is to avoid a narrow case below).
> 
> Since after Zhang Yi's patchset, truncate_inode_partial_folio() is
> changed to return false when it drops the entire folio. Restetting
> start for a dropped folio in the scenario below would leave zeroed
> folios still in the memory, causing a waste, although it is very narrow:
> 
> folio A=[0, 512), B=[512, 1024), C=[1024, 1536],
> 
> 1. A, B and, C are inserted after the same_folio block check, so only the
>    code at whole_folios label sees all of them,
> 2. A is split partially and the split failed, A becomes [0, 32) and
>    [32, 512) zeroed,
>    truncate_inode_partial_folio() returns false, start is set to 32,
> 3. B is dropped entirely, false is returned, start is set to 1024,
> 4. C is split successfully
> 
> After 4, index is start=1024. The [32, 512) folio is not dropped. That
> is why I added the folio->mapping check to avoid changing start in 3.
> 
> 
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index fadf23f9e21ed..efd54a624974c 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1480,6 +1480,16 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  						index = start;
>  						break;
>  					}
> +				} else {
> +					/*
> +					 * Splitting failed, but the part in the
> +					 * range is already zeroed. If the folio
> +					 * was not dropped, skip it when
> +					 * restarting the scan to avoid retrying
> +					 * it forever.
> +					 */
> +					if (folio->mapping)
> +						start = max(start, folio_next_index(folio));
>  				}
>  			}
>  			folio_unlock(folio);
> 
> 

Thanks, makes my life a lot easier!

I'll change that to an effective:

diff --git a/mm/shmem.c b/mm/shmem.c
index 1a81f6dca7510..c263f02fe07cf 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1481,6 +1481,15 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
                                                index = start;
                                                break;
                                        }
+                               } else if (folio->mapping) {
+                                       /*
+                                        * Splitting failed, but the part in the
+                                        * range is already zeroed. The folio
+                                        * was not dropped, so skip it when
+                                        * restarting the scan to avoid retrying
+                                        * it forever.
+                                        */
+                                       start = max(start, folio_next_index(folio));
                                }
                        }

-- 
Cheers,

David