Re: [PATCH v2 2/3] tmpfs: combine !uptodate and post-eof zeroing logic at swapout

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 2025/11/13 00:25, Brian Foster wrote:
> shmem_writeout() zeroes folios that are !uptodate (before marking
> them uptodate) or that extend beyond EOF to preserve data integrity
> according to POSIX. This is handled in a couple different blocks.
> Fold the !uptodate zeroing into the post-eof block so we zero from
> one place.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>   mm/shmem.c | 40 +++++++++++++++++++---------------------
>   1 file changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 5fb3c911894f..7925ced8a05d 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1627,25 +1627,20 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug,
>   	 * good idea to continue anyway, once we're pushing into swap.  So
>   	 * reactivate the folio, and let shmem_fallocate() quit when too many.
>   	 */
> -	if (!folio_test_uptodate(folio)) {
> -		if (inode->i_private) {
> -			struct shmem_falloc *shmem_falloc;
> -			spin_lock(&inode->i_lock);
> -			shmem_falloc = inode->i_private;
> -			if (shmem_falloc &&
> -			    !shmem_falloc->waitq &&
> -			    index >= shmem_falloc->start &&
> -			    index < shmem_falloc->next)
> -				shmem_falloc->nr_unswapped += nr_pages;
> -			else
> -				shmem_falloc = NULL;
> -			spin_unlock(&inode->i_lock);
> -			if (shmem_falloc)
> -				goto redirty;
> -		}
> -		folio_zero_range(folio, 0, folio_size(folio));
> -		flush_dcache_folio(folio);
> -		folio_mark_uptodate(folio);
> +	if (!folio_test_uptodate(folio) && inode->i_private) {
> +		struct shmem_falloc *shmem_falloc;
> +		spin_lock(&inode->i_lock);
> +		shmem_falloc = inode->i_private;
> +		if (shmem_falloc &&
> +		    !shmem_falloc->waitq &&
> +		    index >= shmem_falloc->start &&
> +		    index < shmem_falloc->next)
> +			shmem_falloc->nr_unswapped += nr_pages;
> +		else
> +			shmem_falloc = NULL;
> +		spin_unlock(&inode->i_lock);
> +		if (shmem_falloc)
> +			goto redirty;
>   	}
>   
>   	/*
> @@ -1653,11 +1648,14 @@ int shmem_writeout(struct folio *folio, struct swap_iocb **plug,
>   	 * traditional writeback behavior and facilitates zeroing on file size
>   	 * changes without having to swap back in.
>   	 */
> -	if (folio_next_index(folio) >= end_index) {
> +	if (!folio_test_uptodate(folio) ||
> +	    folio_next_index(folio) >= end_index) {
>   		size_t from = offset_in_folio(folio, i_size);
>   
> -		if (index >= end_index) {
> +		if (!folio_test_uptodate(folio) || index >= end_index) {
>   			folio_zero_segment(folio, 0, folio_size(folio));
> +			flush_dcache_folio(folio);
> +			folio_mark_uptodate(folio);
>   		} else if (from)
>   			folio_zero_segment(folio, from, folio_size(folio));
>   	}

Overall, this looks correct to me. However, I have some concerns about 
this cleanup, as it involves handling different logic for !uptodate 
folios of the fallocate() and EOF zeroing. I'm not sure if combining 
them together makes the code more readable, since, as discussed in patch 
1, there are multiple scenarios to consider for EOF zeroing. Let's see 
how Hugh views this cleanup.