linux-kernel - Re: [PATCH 04/17] writeback: only update ->writeback_index for range

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231221105934.6bhdl73siuu3fct6@quack3>
Date: Thu, 21 Dec 2023 11:59:34 +0100
From: Jan Kara <jack@...e.cz>
To: Christoph Hellwig <hch@....de>
Cc: linux-mm@...ck.org, "Matthew Wilcox (Oracle)" <willy@...radead.org>,
	Jan Kara <jack@...e.com>, David Howells <dhowells@...hat.com>,
	Brian Foster <bfoster@...hat.com>, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/17] writeback: only update ->writeback_index for
 range_cyclic writeback

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

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.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@...e.com>
SUSE Labs, CR