Re: [PATCH 04/17] writeback: only update ->writeback_index for range_cyclic writeback

[Jan Kara]

On Mon 18-12-23 16:35:40, Christoph Hellwig wrote:
> mapping->writeback_index is only [1] used as the starting point for
> range_cyclic writeback, so there is no point in updating it for other
> types of writeback.
> 
> [1] except for btrfs_defrag_file which does really odd things with
> mapping->writeback_index.  But btrfs doesn't use write_cache_pages at
> all, so this isn't relevant here.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  mm/page-writeback.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 7ed6c2bc8dd51c..c798c0d6d0abb4 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -2403,7 +2403,6 @@ int write_cache_pages(struct address_space *mapping,
>  	pgoff_t index;
>  	pgoff_t end;		/* Inclusive */
>  	pgoff_t done_index;
> -	int range_whole = 0;
>  	xa_mark_t tag;
>  
>  	folio_batch_init(&fbatch);
> @@ -2413,8 +2412,6 @@ int write_cache_pages(struct address_space *mapping,
>  	} else {
>  		index = wbc->range_start >> PAGE_SHIFT;
>  		end = wbc->range_end >> PAGE_SHIFT;
> -		if (wbc->range_start == 0 && wbc->range_end == LLONG_MAX)
> -			range_whole = 1;
>  	}
>  	if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages) {
>  		tag_pages_for_writeback(mapping, index, end);
> @@ -2514,14 +2511,21 @@ int write_cache_pages(struct address_space *mapping,
>  	}
>  
>  	/*
> -	 * If we hit the last page and there is more work to be done: wrap
> -	 * back the index back to the start of the file for the next
> -	 * time we are called.
> +	 * For range cyclic writeback we need to remember where we stopped so
> +	 * that we can continue there next time we are called.  If  we hit the
> +	 * last page and there is more work to be done, wrap back to the start
> +	 * of the file.
> +	 *
> +	 * For non-cyclic writeback we always start looking up at the beginning
> +	 * of the file if we are called again, which can only happen due to
> +	 * -ENOMEM from the file system.
>  	 */
> -	if (wbc->range_cyclic && !done)
> -		done_index = 0;
> -	if (wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0))
> -		mapping->writeback_index = done_index;
> +	if (wbc->range_cyclic) {
> +		if (done)
> +			mapping->writeback_index = done_index;
> +		else
> +			mapping->writeback_index = 0;
> +	}
>  
>  	return ret;
>  }
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR