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: <ggj5mx662pc6557zthvqfbzcjzbltjntvf5ly5bj3czeof6ute@r3cjumpx3gtp>
Date: Mon, 13 Oct 2025 14:03:15 +0200
From: Jan Kara <jack@...e.cz>
To: Christoph Hellwig <hch@....de>
Cc: "Matthew Wilcox (Oracle)" <willy@...radead.org>, 
	Eric Van Hensbergen <ericvh@...nel.org>, Latchesar Ionkov <lucho@...kov.net>, 
	Dominique Martinet <asmadeus@...ewreck.org>, Christian Schoenebeck <linux_oss@...debyte.com>, 
	Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>, Mark Fasheh <mark@...heh.com>, 
	Joel Becker <jlbec@...lplan.org>, Joseph Qi <joseph.qi@...ux.alibaba.com>, 
	Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, 
	Josef Bacik <josef@...icpanda.com>, Jan Kara <jack@...e.cz>, linux-block@...r.kernel.org, 
	v9fs@...ts.linux.dev, linux-btrfs@...r.kernel.org, linux-ext4@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, jfs-discussion@...ts.sourceforge.net, 
	ocfs2-devel@...ts.linux.dev, linux-xfs@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 09/10] mm: remove __filemap_fdatawrite_range

On Mon 13-10-25 11:58:04, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range and filemap_fdatawrite_range_kick instead
> of the low-level __filemap_fdatawrite_range that requires the caller
> to know the internals of the writeback_control structure and remove
> __filemap_fdatawrite_range now that it is trivial and only two callers
> would be left.
> 
> Signed-off-by: Christoph Hellwig <hch@....de>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/fs-writeback.c       |  6 +++---
>  fs/sync.c               | 11 +++++------
>  include/linux/pagemap.h |  2 --
>  mm/fadvise.c            |  3 +--
>  mm/filemap.c            | 25 +++++++------------------
>  5 files changed, 16 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 2b35e80037fe..8b002ab18103 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -807,9 +807,9 @@ static void wbc_attach_and_unlock_inode(struct writeback_control *wbc,
>   * @wbc: writeback_control of interest
>   * @inode: target inode
>   *
> - * This function is to be used by __filemap_fdatawrite_range(), which is an
> - * alternative entry point into writeback code, and first ensures @inode is
> - * associated with a bdi_writeback and attaches it to @wbc.
> + * This function is to be used by filemap_fdatawrite*(), which write back data
> + * from arbitrary threads instead of the main writeback thread to ensure @inode
> + * is associated with a bdi_writeback and attached to @wbc.
>   */
>  void wbc_attach_fdatawrite_inode(struct writeback_control *wbc,
>  		struct inode *inode)
> diff --git a/fs/sync.c b/fs/sync.c
> index 2955cd4c77a3..6d8b04e04c3c 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -280,14 +280,13 @@ int sync_file_range(struct file *file, loff_t offset, loff_t nbytes,
>  	}
>  
>  	if (flags & SYNC_FILE_RANGE_WRITE) {
> -		int sync_mode = WB_SYNC_NONE;
> -
>  		if ((flags & SYNC_FILE_RANGE_WRITE_AND_WAIT) ==
>  			     SYNC_FILE_RANGE_WRITE_AND_WAIT)
> -			sync_mode = WB_SYNC_ALL;
> -
> -		ret = __filemap_fdatawrite_range(mapping, offset, endbyte,
> -						 sync_mode);
> +			ret = filemap_fdatawrite_range(mapping, offset,
> +					endbyte);
> +		else
> +			ret = filemap_fdatawrite_range_kick(mapping, offset,
> +					endbyte);
>  		if (ret < 0)
>  			goto out;
>  	}
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 742ba1dd3990..664f23f2330a 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -55,8 +55,6 @@ static inline int filemap_fdatawait(struct address_space *mapping)
>  bool filemap_range_has_page(struct address_space *, loff_t lstart, loff_t lend);
>  int filemap_write_and_wait_range(struct address_space *mapping,
>  		loff_t lstart, loff_t lend);
> -int __filemap_fdatawrite_range(struct address_space *mapping,
> -		loff_t start, loff_t end, int sync_mode);
>  int filemap_fdatawrite_range(struct address_space *mapping,
>  		loff_t start, loff_t end);
>  int filemap_check_errors(struct address_space *mapping);
> diff --git a/mm/fadvise.c b/mm/fadvise.c
> index 588fe76c5a14..f1be619f0e58 100644
> --- a/mm/fadvise.c
> +++ b/mm/fadvise.c
> @@ -111,8 +111,7 @@ int generic_fadvise(struct file *file, loff_t offset, loff_t len, int advice)
>  		spin_unlock(&file->f_lock);
>  		break;
>  	case POSIX_FADV_DONTNEED:
> -		__filemap_fdatawrite_range(mapping, offset, endbyte,
> -					   WB_SYNC_NONE);
> +		filemap_fdatawrite_range_kick(mapping, offset, endbyte);
>  
>  		/*
>  		 * First and last FULL page! Partial pages are deliberately
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 26b692dbf091..ec19ed127de2 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -392,32 +392,23 @@ static int __filemap_fdatawrite(struct address_space *mapping, loff_t start,
>  }
>  
>  /**
> - * __filemap_fdatawrite_range - start writeback on mapping dirty pages in range
> + * filemap_fdatawrite_range - start writeback on mapping dirty pages in range
>   * @mapping:	address space structure to write
>   * @start:	offset in bytes where the range starts
>   * @end:	offset in bytes where the range ends (inclusive)
> - * @sync_mode:	enable synchronous operation
>   *
>   * Start writeback against all of a mapping's dirty pages that lie
>   * within the byte offsets <start, end> inclusive.
>   *
> - * If sync_mode is WB_SYNC_ALL then this is a "data integrity" operation, as
> - * opposed to a regular memory cleansing writeback.  The difference between
> - * these two operations is that if a dirty page/buffer is encountered, it must
> - * be waited upon, and not just skipped over.
> + * This is a data integrity operation that waits upon dirty or in writeback
> + * pages.
>   *
>   * Return: %0 on success, negative error code otherwise.
>   */
> -int __filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
> -				loff_t end, int sync_mode)
> -{
> -	return __filemap_fdatawrite(mapping, start, end, sync_mode, NULL);
> -}
> -
>  int filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
>  		loff_t end)
>  {
> -	return __filemap_fdatawrite_range(mapping, start, end, WB_SYNC_ALL);
> +	return __filemap_fdatawrite(mapping, start, end, WB_SYNC_ALL, NULL);
>  }
>  EXPORT_SYMBOL(filemap_fdatawrite_range);
>  
> @@ -441,7 +432,7 @@ EXPORT_SYMBOL(filemap_fdatawrite);
>  int filemap_fdatawrite_range_kick(struct address_space *mapping, loff_t start,
>  				  loff_t end)
>  {
> -	return __filemap_fdatawrite_range(mapping, start, end, WB_SYNC_NONE);
> +	return __filemap_fdatawrite(mapping, start, end, WB_SYNC_NONE, NULL);
>  }
>  EXPORT_SYMBOL_GPL(filemap_fdatawrite_range_kick);
>  
> @@ -689,8 +680,7 @@ int filemap_write_and_wait_range(struct address_space *mapping,
>  		return 0;
>  
>  	if (mapping_needs_writeback(mapping)) {
> -		err = __filemap_fdatawrite_range(mapping, lstart, lend,
> -						 WB_SYNC_ALL);
> +		err = filemap_fdatawrite_range(mapping, lstart, lend);
>  		/*
>  		 * Even if the above returned error, the pages may be
>  		 * written partially (e.g. -ENOSPC), so we wait for it.
> @@ -792,8 +782,7 @@ int file_write_and_wait_range(struct file *file, loff_t lstart, loff_t lend)
>  		return 0;
>  
>  	if (mapping_needs_writeback(mapping)) {
> -		err = __filemap_fdatawrite_range(mapping, lstart, lend,
> -						 WB_SYNC_ALL);
> +		err = filemap_fdatawrite_range(mapping, lstart, lend);
>  		/* See comment of filemap_write_and_wait() */
>  		if (err != -EIO)
>  			__filemap_fdatawait_range(mapping, lstart, lend);
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ