Received-SPF: pass (google.com: domain of linux-kernel+bounces-8255-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 21 Dec 2023 11:59:34 +0100
From: Jan Kara <jack@suse.cz>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-mm@kvack.org, "Matthew Wilcox (Oracle)" <willy@infradead.org>,
	Jan Kara <jack@suse.com>, David Howells <dhowells@redhat.com>,
	Brian Foster <bfoster@redhat.com>, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 04/17] writeback: only update ->writeback_index for
 range_cyclic writeback
Message-ID: <20231221105934.6bhdl73siuu3fct6@quack3>
References: <20231218153553.807799-1-hch@lst.de>
 <20231218153553.807799-5-hch@lst.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231218153553.807799-5-hch@lst.de>

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@lst.de>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								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@suse.com>
SUSE Labs, CR