filemap_* writeback interface cleanups

filemap_* writeback interface cleanups

[Christoph Hellwig]

Hi all,

while looking at the filemap writeback code, I think adding
filemap_fdatawrite_wbc ended up being a mistake, as all but the original
btrfs caller should be using better high level interfaces instead.  This
series removes all these, switches btrfs to a more specific interfaces
and also cleans up another too low-level interface.  With this the
writeback_control that is passed to the writeback code is only
initialized in three places, although there are a lot more places in
file system code that never reach the common writeback code.

Diffstat:
 block/bdev.c            |    2 
 fs/9p/vfs_file.c        |   17 +------
 fs/btrfs/defrag.c       |    4 -
 fs/btrfs/extent_io.c    |    3 -
 fs/btrfs/file.c         |    2 
 fs/btrfs/inode.c        |   52 +++++++---------------
 fs/btrfs/reflink.c      |    2 
 fs/btrfs/super.c        |    2 
 fs/ext4/inline.c        |    2 
 fs/ext4/inode.c         |    6 +-
 fs/fat/inode.c          |    2 
 fs/fs-writeback.c       |    6 +-
 fs/jfs/jfs_logmgr.c     |    2 
 fs/ocfs2/journal.c      |   11 ----
 fs/sync.c               |   11 ++--
 fs/xfs/xfs_file.c       |    2 
 include/linux/pagemap.h |    8 +--
 mm/fadvise.c            |    3 -
 mm/filemap.c            |  109 +++++++++++++++++++-----------------------------
 mm/khugepaged.c         |    2 
 20 files changed, 95 insertions(+), 153 deletions(-)

[PATCH 01/10] mm: don't opencode filemap_fdatawrite_range in filemap_invalidate_inode

[Christoph Hellwig]

Use filemap_fdatawrite_range instead of opencoding the logic using
filemap_fdatawrite_wbc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 mm/filemap.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 13f0259d993c..99d6919af60d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -4457,16 +4457,8 @@ int filemap_invalidate_inode(struct inode *inode, bool flush,
 	unmap_mapping_pages(mapping, first, nr, false);
 
 	/* Write back the data if we're asked to. */
-	if (flush) {
-		struct writeback_control wbc = {
-			.sync_mode	= WB_SYNC_ALL,
-			.nr_to_write	= LONG_MAX,
-			.range_start	= start,
-			.range_end	= end,
-		};
-
-		filemap_fdatawrite_wbc(mapping, &wbc);
-	}
+	if (flush)
+		filemap_fdatawrite_range(mapping, start, end);
 
 	/* Wait for writeback to complete on all folios and discard. */
 	invalidate_inode_pages2_range(mapping, start / PAGE_SIZE, end / PAGE_SIZE);
-- 
2.47.3

[PATCH 02/10] 9p: don't opencode filemap_fdatawrite_range in v9fs_mmap_vm_close

[Christoph Hellwig]

Use filemap_fdatawrite_range instead of opencoding the logic using
filemap_fdatawrite_wbc.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/9p/vfs_file.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index eb0b083da269..612a230bc012 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -483,24 +483,15 @@ v9fs_vm_page_mkwrite(struct vm_fault *vmf)
 
 static void v9fs_mmap_vm_close(struct vm_area_struct *vma)
 {
-	struct inode *inode;
-
-	struct writeback_control wbc = {
-		.nr_to_write = LONG_MAX,
-		.sync_mode = WB_SYNC_ALL,
-		.range_start = (loff_t)vma->vm_pgoff * PAGE_SIZE,
-		 /* absolute end, byte at end included */
-		.range_end = (loff_t)vma->vm_pgoff * PAGE_SIZE +
-			(vma->vm_end - vma->vm_start - 1),
-	};
-
 	if (!(vma->vm_flags & VM_SHARED))
 		return;
 
 	p9_debug(P9_DEBUG_VFS, "9p VMA close, %p, flushing", vma);
 
-	inode = file_inode(vma->vm_file);
-	filemap_fdatawrite_wbc(inode->i_mapping, &wbc);
+	filemap_fdatawrite_range(file_inode(vma->vm_file)->i_mapping,
+			(loff_t)vma->vm_pgoff * PAGE_SIZE,
+			(loff_t)vma->vm_pgoff * PAGE_SIZE +
+				(vma->vm_end - vma->vm_start - 1));
 }
 
 static const struct vm_operations_struct v9fs_mmap_file_vm_ops = {
-- 
2.47.3

[PATCH 03/10] ocfs2: don't opencode filemap_fdatawrite_range in ocfs2_journal_submit_inode_data_buffers

[Christoph Hellwig]

Use filemap_fdatawrite_range instead of opencoding the logic using
filemap_fdatawrite_wbc.  There is a slight change in the conversion
as nr_to_write is now set to LONG_MAX instead of double the number
of the pages in the range.  LONG_MAX is the usual nr_to_write for
WB_SYNC_ALL writeback, and the value expected by lower layers here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/ocfs2/journal.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index e5f58ff2175f..85239807dec7 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -902,15 +902,8 @@ int ocfs2_journal_alloc(struct ocfs2_super *osb)
 
 static int ocfs2_journal_submit_inode_data_buffers(struct jbd2_inode *jinode)
 {
-	struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
-	struct writeback_control wbc = {
-		.sync_mode =  WB_SYNC_ALL,
-		.nr_to_write = mapping->nrpages * 2,
-		.range_start = jinode->i_dirty_start,
-		.range_end = jinode->i_dirty_end,
-	};
-
-	return filemap_fdatawrite_wbc(mapping, &wbc);
+	return filemap_fdatawrite_range(jinode->i_vfs_inode->i_mapping,
+			jinode->i_dirty_start, jinode->i_dirty_end);
 }
 
 int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)
-- 
2.47.3

[PATCH 04/10] btrfs: use the local tmp_inode variable in start_delalloc_inodes

[Christoph Hellwig]

start_delalloc_inodes has a struct inode * pointer available in the
main loop, use it instead of re-calculating it from the btrfs inode.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/inode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3b1b3a0553ee..9edb78fc57fc 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8744,9 +8744,9 @@ static int start_delalloc_inodes(struct btrfs_root *root,
 		if (snapshot)
 			set_bit(BTRFS_INODE_SNAPSHOT_FLUSH, &inode->runtime_flags);
 		if (full_flush) {
-			work = btrfs_alloc_delalloc_work(&inode->vfs_inode);
+			work = btrfs_alloc_delalloc_work(tmp_inode);
 			if (!work) {
-				iput(&inode->vfs_inode);
+				iput(tmp_inode);
 				ret = -ENOMEM;
 				goto out;
 			}
@@ -8754,7 +8754,7 @@ static int start_delalloc_inodes(struct btrfs_root *root,
 			btrfs_queue_work(root->fs_info->flush_workers,
 					 &work->work);
 		} else {
-			ret = filemap_fdatawrite_wbc(inode->vfs_inode.i_mapping, wbc);
+			ret = filemap_fdatawrite_wbc(tmp_inode->i_mapping, wbc);
 			btrfs_add_delayed_iput(inode);
 			if (ret || wbc->nr_to_write <= 0)
 				goto out;
-- 
2.47.3

[PATCH 05/10] btrfs: push struct writeback_control into start_delalloc_inodes

[Christoph Hellwig]

In preparation for changing the filemap_fdatawrite_wbc API to not expose
the writeback_control to the callers, push the wbc declaration next to
the filemap_fdatawrite_wbc call and just pass thr nr_to_write value to
start_delalloc_inodes.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/inode.c | 51 ++++++++++++++++++++----------------------------
 1 file changed, 21 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 9edb78fc57fc..b97d6c1f7772 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8709,15 +8709,13 @@ static struct btrfs_delalloc_work *btrfs_alloc_delalloc_work(struct inode *inode
  * some fairly slow code that needs optimization. This walks the list
  * of all the inodes with pending delalloc and forces them to disk.
  */
-static int start_delalloc_inodes(struct btrfs_root *root,
-				 struct writeback_control *wbc, bool snapshot,
-				 bool in_reclaim_context)
+static int start_delalloc_inodes(struct btrfs_root *root, long *nr_to_write,
+				 bool snapshot, bool in_reclaim_context)
 {
 	struct btrfs_delalloc_work *work, *next;
 	LIST_HEAD(works);
 	LIST_HEAD(splice);
 	int ret = 0;
-	bool full_flush = wbc->nr_to_write == LONG_MAX;
 
 	mutex_lock(&root->delalloc_mutex);
 	spin_lock(&root->delalloc_lock);
@@ -8743,7 +8741,7 @@ static int start_delalloc_inodes(struct btrfs_root *root,
 
 		if (snapshot)
 			set_bit(BTRFS_INODE_SNAPSHOT_FLUSH, &inode->runtime_flags);
-		if (full_flush) {
+		if (nr_to_write == NULL) {
 			work = btrfs_alloc_delalloc_work(tmp_inode);
 			if (!work) {
 				iput(tmp_inode);
@@ -8754,9 +8752,20 @@ static int start_delalloc_inodes(struct btrfs_root *root,
 			btrfs_queue_work(root->fs_info->flush_workers,
 					 &work->work);
 		} else {
-			ret = filemap_fdatawrite_wbc(tmp_inode->i_mapping, wbc);
+			struct writeback_control wbc = {
+				.nr_to_write = *nr_to_write,
+				.sync_mode = WB_SYNC_NONE,
+				.range_start = 0,
+				.range_end = LLONG_MAX,
+			};
+
+			ret = filemap_fdatawrite_wbc(tmp_inode->i_mapping,
+					&wbc);
 			btrfs_add_delayed_iput(inode);
-			if (ret || wbc->nr_to_write <= 0)
+
+			if (*nr_to_write != LONG_MAX)
+				*nr_to_write = wbc.nr_to_write;
+			if (ret || *nr_to_write <= 0)
 				goto out;
 		}
 		cond_resched();
@@ -8782,29 +8791,17 @@ static int start_delalloc_inodes(struct btrfs_root *root,
 
 int btrfs_start_delalloc_snapshot(struct btrfs_root *root, bool in_reclaim_context)
 {
-	struct writeback_control wbc = {
-		.nr_to_write = LONG_MAX,
-		.sync_mode = WB_SYNC_NONE,
-		.range_start = 0,
-		.range_end = LLONG_MAX,
-	};
 	struct btrfs_fs_info *fs_info = root->fs_info;
 
 	if (BTRFS_FS_ERROR(fs_info))
 		return -EROFS;
-
-	return start_delalloc_inodes(root, &wbc, true, in_reclaim_context);
+	return start_delalloc_inodes(root, NULL, true, in_reclaim_context);
 }
 
 int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, long nr,
 			       bool in_reclaim_context)
 {
-	struct writeback_control wbc = {
-		.nr_to_write = nr,
-		.sync_mode = WB_SYNC_NONE,
-		.range_start = 0,
-		.range_end = LLONG_MAX,
-	};
+	long *nr_to_write = nr == LONG_MAX ? NULL : &nr;
 	struct btrfs_root *root;
 	LIST_HEAD(splice);
 	int ret;
@@ -8816,13 +8813,6 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, long nr,
 	spin_lock(&fs_info->delalloc_root_lock);
 	list_splice_init(&fs_info->delalloc_roots, &splice);
 	while (!list_empty(&splice)) {
-		/*
-		 * Reset nr_to_write here so we know that we're doing a full
-		 * flush.
-		 */
-		if (nr == LONG_MAX)
-			wbc.nr_to_write = LONG_MAX;
-
 		root = list_first_entry(&splice, struct btrfs_root,
 					delalloc_root);
 		root = btrfs_grab_root(root);
@@ -8831,9 +8821,10 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, long nr,
 			       &fs_info->delalloc_roots);
 		spin_unlock(&fs_info->delalloc_root_lock);
 
-		ret = start_delalloc_inodes(root, &wbc, false, in_reclaim_context);
+		ret = start_delalloc_inodes(root, nr_to_write, false,
+				in_reclaim_context);
 		btrfs_put_root(root);
-		if (ret < 0 || wbc.nr_to_write <= 0)
+		if (ret < 0 || nr <= 0)
 			goto out;
 		spin_lock(&fs_info->delalloc_root_lock);
 	}
-- 
2.47.3

[PATCH 06/10] mm,btrfs: add a filemap_fdatawrite_kick_nr helper

[Christoph Hellwig]

Abstract out the btrfs-specific behavior of kicking off I/O on a number
of pages on an address_space into a well-defined helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/btrfs/inode.c        | 13 ++-----------
 include/linux/pagemap.h |  2 ++
 mm/filemap.c            | 22 ++++++++++++++++++++++
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b97d6c1f7772..b63d77154c45 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8752,19 +8752,10 @@ static int start_delalloc_inodes(struct btrfs_root *root, long *nr_to_write,
 			btrfs_queue_work(root->fs_info->flush_workers,
 					 &work->work);
 		} else {
-			struct writeback_control wbc = {
-				.nr_to_write = *nr_to_write,
-				.sync_mode = WB_SYNC_NONE,
-				.range_start = 0,
-				.range_end = LLONG_MAX,
-			};
-
-			ret = filemap_fdatawrite_wbc(tmp_inode->i_mapping,
-					&wbc);
+			ret = filemap_fdatawrite_kick_nr(tmp_inode->i_mapping,
+					nr_to_write);
 			btrfs_add_delayed_iput(inode);
 
-			if (*nr_to_write != LONG_MAX)
-				*nr_to_write = wbc.nr_to_write;
 			if (ret || *nr_to_write <= 0)
 				goto out;
 		}
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 09b581c1d878..fc060ce2d31d 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -38,6 +38,8 @@ int filemap_invalidate_pages(struct address_space *mapping,
 int write_inode_now(struct inode *, int sync);
 int filemap_fdatawrite(struct address_space *);
 int filemap_flush(struct address_space *);
+int filemap_fdatawrite_kick_nr(struct address_space *mapping,
+		long *nr_to_write);
 int filemap_fdatawait_keep_errors(struct address_space *mapping);
 int filemap_fdatawait_range(struct address_space *, loff_t lstart, loff_t lend);
 int filemap_fdatawait_range_keep_errors(struct address_space *mapping,
diff --git a/mm/filemap.c b/mm/filemap.c
index 99d6919af60d..b95e71774131 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -474,6 +474,28 @@ int filemap_flush(struct address_space *mapping)
 }
 EXPORT_SYMBOL(filemap_flush);
 
+/*
+ * Start writeback on @nr_to_write pages from @mapping.  No one but the existing
+ * btrfs caller should be using this.  Talk to linux-mm if you think adding a
+ * new caller is a good idea.
+ */
+int filemap_fdatawrite_kick_nr(struct address_space *mapping, long *nr_to_write)
+{
+	struct writeback_control wbc = {
+		.nr_to_write = *nr_to_write,
+		.sync_mode = WB_SYNC_NONE,
+		.range_start = 0,
+		.range_end = LLONG_MAX,
+	};
+	int ret;
+
+	ret = filemap_fdatawrite_wbc(mapping, &wbc);
+	if (!ret)
+		*nr_to_write = wbc.nr_to_write;
+	return ret;
+}
+EXPORT_SYMBOL_FOR_MODULES(filemap_fdatawrite_kick_nr, "btrfs");
+
 /**
  * filemap_range_has_page - check if a page exists in range.
  * @mapping:           address space within which to check
-- 
2.47.3

[PATCH 07/10] mm: remove __filemap_fdatawrite

[Christoph Hellwig]

And rewrite filemap_fdatawrite to use filemap_fdatawrite_range instead
to have a simpler call chain.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 mm/filemap.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index b95e71774131..bbd5d5eaa661 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -422,25 +422,19 @@ int __filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
 	return filemap_fdatawrite_wbc(mapping, &wbc);
 }
 
-static inline int __filemap_fdatawrite(struct address_space *mapping,
-	int sync_mode)
+int filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
+		loff_t end)
 {
-	return __filemap_fdatawrite_range(mapping, 0, LLONG_MAX, sync_mode);
+	return __filemap_fdatawrite_range(mapping, start, end, WB_SYNC_ALL);
 }
+EXPORT_SYMBOL(filemap_fdatawrite_range);
 
 int filemap_fdatawrite(struct address_space *mapping)
 {
-	return __filemap_fdatawrite(mapping, WB_SYNC_ALL);
+	return filemap_fdatawrite_range(mapping, 0, LONG_MAX);
 }
 EXPORT_SYMBOL(filemap_fdatawrite);
 
-int filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
-				loff_t end)
-{
-	return __filemap_fdatawrite_range(mapping, start, end, WB_SYNC_ALL);
-}
-EXPORT_SYMBOL(filemap_fdatawrite_range);
-
 /**
  * filemap_fdatawrite_range_kick - start writeback on a range
  * @mapping:	target address_space
@@ -470,7 +464,7 @@ EXPORT_SYMBOL_GPL(filemap_fdatawrite_range_kick);
  */
 int filemap_flush(struct address_space *mapping)
 {
-	return __filemap_fdatawrite(mapping, WB_SYNC_NONE);
+	return filemap_fdatawrite_range_kick(mapping, 0, LLONG_MAX);
 }
 EXPORT_SYMBOL(filemap_flush);
 
-- 
2.47.3

[PATCH 08/10] mm: remove filemap_fdatawrite_wbc

[Christoph Hellwig]

Replace filemap_fdatawrite_wbc, which exposes a writeback_control to the
callers with a __filemap_fdatawrite helper that takes all the possible
arguments and declares the writeback_control itself.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/pagemap.h |  2 --
 mm/filemap.c            | 54 ++++++++++++++---------------------------
 2 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index fc060ce2d31d..742ba1dd3990 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -61,8 +61,6 @@ int filemap_fdatawrite_range(struct address_space *mapping,
 		loff_t start, loff_t end);
 int filemap_check_errors(struct address_space *mapping);
 void __filemap_set_wb_err(struct address_space *mapping, int err);
-int filemap_fdatawrite_wbc(struct address_space *mapping,
-			   struct writeback_control *wbc);
 int kiocb_write_and_wait(struct kiocb *iocb, size_t count);
 
 static inline int filemap_write_and_wait(struct address_space *mapping)
diff --git a/mm/filemap.c b/mm/filemap.c
index bbd5d5eaa661..26b692dbf091 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -366,31 +366,30 @@ static int filemap_check_and_keep_errors(struct address_space *mapping)
 	return 0;
 }
 
-/**
- * filemap_fdatawrite_wbc - start writeback on mapping dirty pages in range
- * @mapping:	address space structure to write
- * @wbc:	the writeback_control controlling the writeout
- *
- * Call writepages on the mapping using the provided wbc to control the
- * writeout.
- *
- * Return: %0 on success, negative error code otherwise.
- */
-int filemap_fdatawrite_wbc(struct address_space *mapping,
-			   struct writeback_control *wbc)
+static int __filemap_fdatawrite(struct address_space *mapping, loff_t start,
+		loff_t end, enum writeback_sync_modes sync_mode,
+		long *nr_to_write)
 {
+	struct writeback_control wbc = {
+		.sync_mode	= sync_mode,
+		.nr_to_write	= nr_to_write ? *nr_to_write : LONG_MAX,
+		.range_start	= start,
+		.range_end	= end,
+	};
 	int ret;
 
 	if (!mapping_can_writeback(mapping) ||
 	    !mapping_tagged(mapping, PAGECACHE_TAG_DIRTY))
 		return 0;
 
-	wbc_attach_fdatawrite_inode(wbc, mapping->host);
-	ret = do_writepages(mapping, wbc);
-	wbc_detach_inode(wbc);
+	wbc_attach_fdatawrite_inode(&wbc, mapping->host);
+	ret = do_writepages(mapping, &wbc);
+	wbc_detach_inode(&wbc);
+
+	if (!ret && nr_to_write)
+		*nr_to_write = wbc.nr_to_write;
 	return ret;
 }
-EXPORT_SYMBOL(filemap_fdatawrite_wbc);
 
 /**
  * __filemap_fdatawrite_range - start writeback on mapping dirty pages in range
@@ -412,14 +411,7 @@ EXPORT_SYMBOL(filemap_fdatawrite_wbc);
 int __filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
 				loff_t end, int sync_mode)
 {
-	struct writeback_control wbc = {
-		.sync_mode = sync_mode,
-		.nr_to_write = LONG_MAX,
-		.range_start = start,
-		.range_end = end,
-	};
-
-	return filemap_fdatawrite_wbc(mapping, &wbc);
+	return __filemap_fdatawrite(mapping, start, end, sync_mode, NULL);
 }
 
 int filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
@@ -475,18 +467,8 @@ EXPORT_SYMBOL(filemap_flush);
  */
 int filemap_fdatawrite_kick_nr(struct address_space *mapping, long *nr_to_write)
 {
-	struct writeback_control wbc = {
-		.nr_to_write = *nr_to_write,
-		.sync_mode = WB_SYNC_NONE,
-		.range_start = 0,
-		.range_end = LLONG_MAX,
-	};
-	int ret;
-
-	ret = filemap_fdatawrite_wbc(mapping, &wbc);
-	if (!ret)
-		*nr_to_write = wbc.nr_to_write;
-	return ret;
+	return __filemap_fdatawrite(mapping, 0, LLONG_MAX, WB_SYNC_NONE,
+			nr_to_write);
 }
 EXPORT_SYMBOL_FOR_MODULES(filemap_fdatawrite_kick_nr, "btrfs");
 
-- 
2.47.3

[PATCH 09/10] mm: remove __filemap_fdatawrite_range

[Christoph Hellwig]

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

[PATCH 10/10] mm: rename filemap_flush to filemap_fdatawrite_kick

[Christoph Hellwig]

Make the naming consistent with the other helpers and get away from
the flush terminology that is way to overloaded.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bdev.c            | 2 +-
 fs/btrfs/defrag.c       | 4 ++--
 fs/btrfs/extent_io.c    | 3 ++-
 fs/btrfs/file.c         | 2 +-
 fs/btrfs/inode.c        | 6 +++---
 fs/btrfs/reflink.c      | 2 +-
 fs/btrfs/super.c        | 2 +-
 fs/ext4/inline.c        | 2 +-
 fs/ext4/inode.c         | 6 +++---
 fs/fat/inode.c          | 2 +-
 fs/jfs/jfs_logmgr.c     | 2 +-
 fs/xfs/xfs_file.c       | 2 +-
 include/linux/pagemap.h | 2 +-
 mm/filemap.c            | 6 +++---
 mm/khugepaged.c         | 2 +-
 15 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/block/bdev.c b/block/bdev.c
index 810707cca970..e8a564120ac7 100644
--- a/block/bdev.c
+++ b/block/bdev.c
@@ -245,7 +245,7 @@ int sync_blockdev_nowait(struct block_device *bdev)
 {
 	if (!bdev)
 		return 0;
-	return filemap_flush(bdev->bd_mapping);
+	return filemap_fdatawrite_kick(bdev->bd_mapping);
 }
 EXPORT_SYMBOL_GPL(sync_blockdev_nowait);
 
diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c
index 7b277934f66f..e2f3027060db 100644
--- a/fs/btrfs/defrag.c
+++ b/fs/btrfs/defrag.c
@@ -1483,10 +1483,10 @@ int btrfs_defrag_file(struct btrfs_inode *inode, struct file_ra_state *ra,
 		 * need to be written back immediately.
 		 */
 		if (range->flags & BTRFS_DEFRAG_RANGE_START_IO) {
-			filemap_flush(inode->vfs_inode.i_mapping);
+			filemap_fdatawrite_kick(inode->vfs_inode.i_mapping);
 			if (test_bit(BTRFS_INODE_HAS_ASYNC_EXTENT,
 				     &inode->runtime_flags))
-				filemap_flush(inode->vfs_inode.i_mapping);
+				filemap_fdatawrite_kick(inode->vfs_inode.i_mapping);
 		}
 		if (range->compress_type == BTRFS_COMPRESS_LZO)
 			btrfs_set_fs_incompat(fs_info, COMPRESS_LZO);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index c123a3ef154a..c86cb27876a4 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2450,7 +2450,8 @@ static int extent_write_cache_pages(struct address_space *mapping,
 
 	/*
 	 * We do the tagged writepage as long as the snapshot flush bit is set
-	 * and we are the first one who do the filemap_flush() on this inode.
+	 * and we are the first one who do the filemap_fdatawrite_kick() on this
+	 * inode.
 	 *
 	 * The nr_to_write == LONG_MAX is needed to make sure other flushers do
 	 * not race in and drop the bit.
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 7efd1f8a1912..9190488f1aae 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1498,7 +1498,7 @@ int btrfs_release_file(struct inode *inode, struct file *filp)
 	 */
 	if (test_and_clear_bit(BTRFS_INODE_FLUSH_ON_CLOSE,
 			       &BTRFS_I(inode)->runtime_flags))
-			filemap_flush(inode->i_mapping);
+			filemap_fdatawrite_kick(inode->i_mapping);
 	return 0;
 }
 
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b63d77154c45..4192056f4d42 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8434,7 +8434,7 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 	 * now so  we don't add too much work to the end of the transaction
 	 */
 	if (new_inode && S_ISREG(old_inode->i_mode) && new_inode->i_size)
-		filemap_flush(old_inode->i_mapping);
+		filemap_fdatawrite_kick(old_inode->i_mapping);
 
 	if (flags & RENAME_WHITEOUT) {
 		whiteout_args.inode = new_whiteout_inode(idmap, old_dir);
@@ -8680,10 +8680,10 @@ static void btrfs_run_delalloc_work(struct btrfs_work *work)
 	delalloc_work = container_of(work, struct btrfs_delalloc_work,
 				     work);
 	inode = delalloc_work->inode;
-	filemap_flush(inode->i_mapping);
+	filemap_fdatawrite_kick(inode->i_mapping);
 	if (test_bit(BTRFS_INODE_HAS_ASYNC_EXTENT,
 				&BTRFS_I(inode)->runtime_flags))
-		filemap_flush(inode->i_mapping);
+		filemap_fdatawrite_kick(inode->i_mapping);
 
 	iput(inode);
 	complete(&delalloc_work->completion);
diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index 5465a5eae9b2..72f9b5faa2e8 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -834,7 +834,7 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in,
 	 * Also we don't need to check ASYNC_EXTENT, as async extent will be
 	 * CoWed anyway, not affecting nocow part.
 	 */
-	ret = filemap_flush(inode_in->vfs_inode.i_mapping);
+	ret = filemap_fdatawrite_kick(inode_in->vfs_inode.i_mapping);
 	if (ret < 0)
 		return ret;
 
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index d6e496436539..e634d93a908c 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1022,7 +1022,7 @@ int btrfs_sync_fs(struct super_block *sb, int wait)
 	trace_btrfs_sync_fs(fs_info, wait);
 
 	if (!wait) {
-		filemap_flush(fs_info->btree_inode->i_mapping);
+		filemap_fdatawrite_kick(fs_info->btree_inode->i_mapping);
 		return 0;
 	}
 
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 1b094a4f3866..692d28dbee64 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1951,7 +1951,7 @@ int ext4_convert_inline_data(struct inode *inode)
 		 * inline data to delay allocated block. Just force writeout
 		 * here to finish conversion.
 		 */
-		error = filemap_flush(inode->i_mapping);
+		error = filemap_fdatawrite_kick(inode->i_mapping);
 		if (error)
 			return error;
 		if (!ext4_has_inline_data(inode))
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index f9e4ac87211e..f4bcbcfd4531 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3302,7 +3302,7 @@ int ext4_alloc_da_blocks(struct inode *inode)
 		return 0;
 
 	/*
-	 * We do something simple for now.  The filemap_flush() will
+	 * We do something simple for now.  The filemap_fdatawrite_kick() will
 	 * also start triggering a write of the data blocks, which is
 	 * not strictly speaking necessary (and for users of
 	 * laptop_mode, not even desirable).  However, to do otherwise
@@ -3328,11 +3328,11 @@ int ext4_alloc_da_blocks(struct inode *inode)
 	 * logical block extents, call the multi-block allocator, and
 	 * then update the buffer heads with the block allocations.
 	 *
-	 * For now, though, we'll cheat by calling filemap_flush(),
+	 * For now, though, we'll cheat by calling filemap_fdatawrite_kick(),
 	 * which will map the blocks, and start the I/O, but not
 	 * actually wait for the I/O to complete.
 	 */
-	return filemap_flush(inode->i_mapping);
+	return filemap_fdatawrite_kick(inode->i_mapping);
 }
 
 /*
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 9648ed097816..f1ecb4c211bc 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -1877,7 +1877,7 @@ static int writeback_inode(struct inode *inode)
  * write data and metadata corresponding to i1 and i2.  The io is
  * started but we do not wait for any of it to finish.
  *
- * filemap_flush is used for the block device, so if there is a dirty
+ * filemap_fdatawrite_kick is used for the block device, so if there is a dirty
  * page for a block already in flight, we will not wait and start the
  * io over again
  */
diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c
index b343c5ea1159..4cdb0bb6c921 100644
--- a/fs/jfs/jfs_logmgr.c
+++ b/fs/jfs/jfs_logmgr.c
@@ -934,7 +934,7 @@ static int lmLogSync(struct jfs_log * log, int hard_sync)
 	if (hard_sync)
 		write_special_inodes(log, filemap_fdatawrite);
 	else
-		write_special_inodes(log, filemap_flush);
+		write_special_inodes(log, filemap_fdatawrite_kick);
 
 	/*
 	 *	forward syncpt
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 2702fef2c90c..bf39bfd4a8cd 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1615,7 +1615,7 @@ xfs_file_release(
 	if (xfs_iflags_test_and_clear(ip, XFS_ITRUNCATED)) {
 		xfs_iflags_clear(ip, XFS_EOFBLOCKS_RELEASED);
 		if (ip->i_delayed_blks > 0)
-			filemap_flush(inode->i_mapping);
+			filemap_fdatawrite_kick(inode->i_mapping);
 	}
 
 	/*
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 664f23f2330a..e7c4d8bba952 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -37,7 +37,7 @@ int filemap_invalidate_pages(struct address_space *mapping,
 
 int write_inode_now(struct inode *, int sync);
 int filemap_fdatawrite(struct address_space *);
-int filemap_flush(struct address_space *);
+int filemap_fdatawrite_kick(struct address_space *);
 int filemap_fdatawrite_kick_nr(struct address_space *mapping,
 		long *nr_to_write);
 int filemap_fdatawait_keep_errors(struct address_space *mapping);
diff --git a/mm/filemap.c b/mm/filemap.c
index ec19ed127de2..3ad6698c39c9 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -437,7 +437,7 @@ int filemap_fdatawrite_range_kick(struct address_space *mapping, loff_t start,
 EXPORT_SYMBOL_GPL(filemap_fdatawrite_range_kick);
 
 /**
- * filemap_flush - mostly a non-blocking flush
+ * filemap_fdatawrite_kick - mostly a non-blocking flush
  * @mapping:	target address_space
  *
  * This is a mostly non-blocking flush.  Not suitable for data-integrity
@@ -445,11 +445,11 @@ EXPORT_SYMBOL_GPL(filemap_fdatawrite_range_kick);
  *
  * Return: %0 on success, negative error code otherwise.
  */
-int filemap_flush(struct address_space *mapping)
+int filemap_fdatawrite_kick(struct address_space *mapping)
 {
 	return filemap_fdatawrite_range_kick(mapping, 0, LLONG_MAX);
 }
-EXPORT_SYMBOL(filemap_flush);
+EXPORT_SYMBOL(filemap_fdatawrite_kick);
 
 /*
  * Start writeback on @nr_to_write pages from @mapping.  No one but the existing
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index abe54f0043c7..5a4a43e979c4 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1949,7 +1949,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
 				 * forcing writeback in loop.
 				 */
 				xas_unlock_irq(&xas);
-				filemap_flush(mapping);
+				filemap_fdatawrite_kick(mapping);
 				result = SCAN_FAIL;
 				goto xa_unlocked;
 			} else if (folio_test_writeback(folio)) {
-- 
2.47.3

Re: [PATCH 01/10] mm: don't opencode filemap_fdatawrite_range in filemap_invalidate_inode

[Damien Le Moal]

On 2025/10/13 11:57, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range instead of opencoding the logic using
> filemap_fdatawrite_wbc.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 02/10] 9p: don't opencode filemap_fdatawrite_range in v9fs_mmap_vm_close

[Damien Le Moal]

On 2025/10/13 11:57, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range instead of opencoding the logic using
> filemap_fdatawrite_wbc.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 03/10] ocfs2: don't opencode filemap_fdatawrite_range in ocfs2_journal_submit_inode_data_buffers

[Damien Le Moal]

On 2025/10/13 11:57, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range instead of opencoding the logic using
> filemap_fdatawrite_wbc.  There is a slight change in the conversion
> as nr_to_write is now set to LONG_MAX instead of double the number
> of the pages in the range.  LONG_MAX is the usual nr_to_write for
> WB_SYNC_ALL writeback, and the value expected by lower layers here.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 04/10] btrfs: use the local tmp_inode variable in start_delalloc_inodes

[Damien Le Moal]

On 2025/10/13 11:57, Christoph Hellwig wrote:
> start_delalloc_inodes has a struct inode * pointer available in the
> main loop, use it instead of re-calculating it from the btrfs inode.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 05/10] btrfs: push struct writeback_control into start_delalloc_inodes

[Damien Le Moal]

On 2025/10/13 11:58, Christoph Hellwig wrote:
> In preparation for changing the filemap_fdatawrite_wbc API to not expose
> the writeback_control to the callers, push the wbc declaration next to
> the filemap_fdatawrite_wbc call and just pass thr nr_to_write value to

s/thr/the

> start_delalloc_inodes.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

...

> @@ -8831,9 +8821,10 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, long nr,
>  			       &fs_info->delalloc_roots);
>  		spin_unlock(&fs_info->delalloc_root_lock);
>  
> -		ret = start_delalloc_inodes(root, &wbc, false, in_reclaim_context);
> +		ret = start_delalloc_inodes(root, nr_to_write, false,
> +				in_reclaim_context);
>  		btrfs_put_root(root);
> -		if (ret < 0 || wbc.nr_to_write <= 0)
> +		if (ret < 0 || nr <= 0)

Before this change, wbc.nr_to_write will indicate what's remaining, not what you
asked for. So I think you need a change like you did in start_delalloc_inodes(),
no ?

>  			goto out;
>  		spin_lock(&fs_info->delalloc_root_lock);
>  	}


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 06/10] mm,btrfs: add a filemap_fdatawrite_kick_nr helper

[Damien Le Moal]

On 2025/10/13 11:58, Christoph Hellwig wrote:
> Abstract out the btrfs-specific behavior of kicking off I/O on a number
> of pages on an address_space into a well-defined helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

One nit below.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


> +/*
> + * Start writeback on @nr_to_write pages from @mapping.  No one but the existing
> + * btrfs caller should be using this.  Talk to linux-mm if you think adding a
> + * new caller is a good idea.
> + */
> +int filemap_fdatawrite_kick_nr(struct address_space *mapping, long *nr_to_write)

Not a huge fan of this name. Maybe filemap_fdatawrite_nrpages() ?


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 07/10] mm: remove __filemap_fdatawrite

[Damien Le Moal]

On 2025/10/13 11:58, Christoph Hellwig wrote:
> And rewrite filemap_fdatawrite to use filemap_fdatawrite_range instead
> to have a simpler call chain.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  mm/filemap.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index b95e71774131..bbd5d5eaa661 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -422,25 +422,19 @@ int __filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
>  	return filemap_fdatawrite_wbc(mapping, &wbc);
>  }
>  
> -static inline int __filemap_fdatawrite(struct address_space *mapping,
> -	int sync_mode)
> +int filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
> +		loff_t end)
>  {
> -	return __filemap_fdatawrite_range(mapping, 0, LLONG_MAX, sync_mode);
> +	return __filemap_fdatawrite_range(mapping, start, end, WB_SYNC_ALL);
>  }
> +EXPORT_SYMBOL(filemap_fdatawrite_range);
>  
>  int filemap_fdatawrite(struct address_space *mapping)
>  {
> -	return __filemap_fdatawrite(mapping, WB_SYNC_ALL);
> +	return filemap_fdatawrite_range(mapping, 0, LONG_MAX);

This should be LLONG_MAX, no ?

>  }
>  EXPORT_SYMBOL(filemap_fdatawrite);
>  
> -int filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
> -				loff_t end)
> -{
> -	return __filemap_fdatawrite_range(mapping, start, end, WB_SYNC_ALL);
> -}
> -EXPORT_SYMBOL(filemap_fdatawrite_range);
> -
>  /**
>   * filemap_fdatawrite_range_kick - start writeback on a range
>   * @mapping:	target address_space
> @@ -470,7 +464,7 @@ EXPORT_SYMBOL_GPL(filemap_fdatawrite_range_kick);
>   */
>  int filemap_flush(struct address_space *mapping)
>  {
> -	return __filemap_fdatawrite(mapping, WB_SYNC_NONE);
> +	return filemap_fdatawrite_range_kick(mapping, 0, LLONG_MAX);
>  }
>  EXPORT_SYMBOL(filemap_flush);
>  


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 08/10] mm: remove filemap_fdatawrite_wbc

[Damien Le Moal]

On 2025/10/13 11:58, Christoph Hellwig wrote:
> Replace filemap_fdatawrite_wbc, which exposes a writeback_control to the
> callers with a __filemap_fdatawrite helper that takes all the possible
> arguments and declares the writeback_control itself.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 09/10] mm: remove __filemap_fdatawrite_range

[Damien Le Moal]

On 2025/10/13 11:58, 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@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 04/10] btrfs: use the local tmp_inode variable in start_delalloc_inodes

[Johannes Thumshirn]

On 10/13/25 4:59 AM, Christoph Hellwig wrote:
> start_delalloc_inodes has a struct inode * pointer available in the
> main loop, use it instead of re-calculating it from the btrfs inode.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   fs/btrfs/inode.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 3b1b3a0553ee..9edb78fc57fc 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -8744,9 +8744,9 @@ static int start_delalloc_inodes(struct btrfs_root *root,
>   		if (snapshot)
>   			set_bit(BTRFS_INODE_SNAPSHOT_FLUSH, &inode->runtime_flags);
>   		if (full_flush) {
> -			work = btrfs_alloc_delalloc_work(&inode->vfs_inode);
> +			work = btrfs_alloc_delalloc_work(tmp_inode);
>   			if (!work) {
> -				iput(&inode->vfs_inode);
> +				iput(tmp_inode);
>   				ret = -ENOMEM;
>   				goto out;
>   			}
> @@ -8754,7 +8754,7 @@ static int start_delalloc_inodes(struct btrfs_root *root,
>   			btrfs_queue_work(root->fs_info->flush_workers,
>   					 &work->work);
>   		} else {
> -			ret = filemap_fdatawrite_wbc(inode->vfs_inode.i_mapping, wbc);
> +			ret = filemap_fdatawrite_wbc(tmp_inode->i_mapping, wbc);
>   			btrfs_add_delayed_iput(inode);
>   			if (ret || wbc->nr_to_write <= 0)
>   				goto out;

If you have to repost this for some reason, can you rename tmp_inode to 
vfs_inode or sth like that?

The name is really confusing and the commit introducing it doesn't 
describe it really either.

Re: filemap_* writeback interface cleanups

[Johannes Thumshirn]

On 10/13/25 4:58 AM, Christoph Hellwig wrote:
> Hi all,
>
> while looking at the filemap writeback code, I think adding
> filemap_fdatawrite_wbc ended up being a mistake, as all but the original
> btrfs caller should be using better high level interfaces instead.  This
> series removes all these, switches btrfs to a more specific interfaces
> and also cleans up another too low-level interface.  With this the
> writeback_control that is passed to the writeback code is only
> initialized in three places, although there are a lot more places in
> file system code that never reach the common writeback code.
>
> Diffstat:
>   block/bdev.c            |    2
>   fs/9p/vfs_file.c        |   17 +------
>   fs/btrfs/defrag.c       |    4 -
>   fs/btrfs/extent_io.c    |    3 -
>   fs/btrfs/file.c         |    2
>   fs/btrfs/inode.c        |   52 +++++++---------------
>   fs/btrfs/reflink.c      |    2
>   fs/btrfs/super.c        |    2
>   fs/ext4/inline.c        |    2
>   fs/ext4/inode.c         |    6 +-
>   fs/fat/inode.c          |    2
>   fs/fs-writeback.c       |    6 +-
>   fs/jfs/jfs_logmgr.c     |    2
>   fs/ocfs2/journal.c      |   11 ----
>   fs/sync.c               |   11 ++--
>   fs/xfs/xfs_file.c       |    2
>   include/linux/pagemap.h |    8 +--
>   mm/fadvise.c            |    3 -
>   mm/filemap.c            |  109 +++++++++++++++++++-----------------------------
>   mm/khugepaged.c         |    2
>   20 files changed, 95 insertions(+), 153 deletions(-)
>
>
Looks good,

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 10/10] mm: rename filemap_flush to filemap_fdatawrite_kick

[Damien Le Moal]

On 2025/10/13 11:58, Christoph Hellwig wrote:
> Make the naming consistent with the other helpers and get away from
> the flush terminology that is way to overloaded.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 05/10] btrfs: push struct writeback_control into start_delalloc_inodes

[Daniel Vacek]

On Mon, 13 Oct 2025 at 09:56, Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 2025/10/13 11:58, Christoph Hellwig wrote:
> > In preparation for changing the filemap_fdatawrite_wbc API to not expose
> > the writeback_control to the callers, push the wbc declaration next to
> > the filemap_fdatawrite_wbc call and just pass thr nr_to_write value to
>
> s/thr/the
>
> > start_delalloc_inodes.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> ...
>
> > @@ -8831,9 +8821,10 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, long nr,
> >                              &fs_info->delalloc_roots);
> >               spin_unlock(&fs_info->delalloc_root_lock);
> >
> > -             ret = start_delalloc_inodes(root, &wbc, false, in_reclaim_context);
> > +             ret = start_delalloc_inodes(root, nr_to_write, false,
> > +                             in_reclaim_context);
> >               btrfs_put_root(root);
> > -             if (ret < 0 || wbc.nr_to_write <= 0)
> > +             if (ret < 0 || nr <= 0)
>
> Before this change, wbc.nr_to_write will indicate what's remaining, not what you
> asked for. So I think you need a change like you did in start_delalloc_inodes(),
> no ?

I understand nr is updated to what's remaining using the nr_to_write
pointer in start_delalloc_inodes(). Right?

--nX

> >                       goto out;
> >               spin_lock(&fs_info->delalloc_root_lock);
> >       }
>
>
> --
> Damien Le Moal
> Western Digital Research
>

Re: [PATCH 05/10] btrfs: push struct writeback_control into start_delalloc_inodes

[Damien Le Moal]

On 2025/10/13 17:15, Daniel Vacek wrote:
> On Mon, 13 Oct 2025 at 09:56, Damien Le Moal <dlemoal@kernel.org> wrote:
>>
>> On 2025/10/13 11:58, Christoph Hellwig wrote:
>>> In preparation for changing the filemap_fdatawrite_wbc API to not expose
>>> the writeback_control to the callers, push the wbc declaration next to
>>> the filemap_fdatawrite_wbc call and just pass thr nr_to_write value to
>>
>> s/thr/the
>>
>>> start_delalloc_inodes.
>>>
>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>
>> ...
>>
>>> @@ -8831,9 +8821,10 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, long nr,
>>>                              &fs_info->delalloc_roots);
>>>               spin_unlock(&fs_info->delalloc_root_lock);
>>>
>>> -             ret = start_delalloc_inodes(root, &wbc, false, in_reclaim_context);
>>> +             ret = start_delalloc_inodes(root, nr_to_write, false,
>>> +                             in_reclaim_context);
>>>               btrfs_put_root(root);
>>> -             if (ret < 0 || wbc.nr_to_write <= 0)
>>> +             if (ret < 0 || nr <= 0)
>>
>> Before this change, wbc.nr_to_write will indicate what's remaining, not what you
>> asked for. So I think you need a change like you did in start_delalloc_inodes(),
>> no ?
> 
> I understand nr is updated to what's remaining using the nr_to_write
> pointer in start_delalloc_inodes(). Right?

Oh! Yes, nr_to_write points to nr... Sorry about the noise.

So Reviewed-by: Damien Le Moal <dlemoal@kernel.org>



-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 10/10] mm: rename filemap_flush to filemap_fdatawrite_kick

[Damien Le Moal]

On 2025/10/13 11:58, Christoph Hellwig wrote:
> Make the naming consistent with the other helpers and get away from
> the flush terminology that is way to overloaded.

s/to/too

> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 03/10] ocfs2: don't opencode filemap_fdatawrite_range in ocfs2_journal_submit_inode_data_buffers

[Joseph Qi]

On 2025/10/13 10:57, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range instead of opencoding the logic using
> filemap_fdatawrite_wbc.  There is a slight change in the conversion
> as nr_to_write is now set to LONG_MAX instead of double the number
> of the pages in the range.  LONG_MAX is the usual nr_to_write for
> WB_SYNC_ALL writeback, and the value expected by lower layers here.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks fine to me.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/journal.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index e5f58ff2175f..85239807dec7 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -902,15 +902,8 @@ int ocfs2_journal_alloc(struct ocfs2_super *osb)
>  
>  static int ocfs2_journal_submit_inode_data_buffers(struct jbd2_inode *jinode)
>  {
> -	struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
> -	struct writeback_control wbc = {
> -		.sync_mode =  WB_SYNC_ALL,
> -		.nr_to_write = mapping->nrpages * 2,
> -		.range_start = jinode->i_dirty_start,
> -		.range_end = jinode->i_dirty_end,
> -	};
> -
> -	return filemap_fdatawrite_wbc(mapping, &wbc);
> +	return filemap_fdatawrite_range(jinode->i_vfs_inode->i_mapping,
> +			jinode->i_dirty_start, jinode->i_dirty_end);
>  }
>  
>  int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)

Re: [PATCH 01/10] mm: don't opencode filemap_fdatawrite_range in filemap_invalidate_inode

[Jan Kara]

On Mon 13-10-25 11:57:56, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range instead of opencoding the logic using
> filemap_fdatawrite_wbc.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

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

								Honza

> ---
>  mm/filemap.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 13f0259d993c..99d6919af60d 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -4457,16 +4457,8 @@ int filemap_invalidate_inode(struct inode *inode, bool flush,
>  	unmap_mapping_pages(mapping, first, nr, false);
>  
>  	/* Write back the data if we're asked to. */
> -	if (flush) {
> -		struct writeback_control wbc = {
> -			.sync_mode	= WB_SYNC_ALL,
> -			.nr_to_write	= LONG_MAX,
> -			.range_start	= start,
> -			.range_end	= end,
> -		};
> -
> -		filemap_fdatawrite_wbc(mapping, &wbc);
> -	}
> +	if (flush)
> +		filemap_fdatawrite_range(mapping, start, end);
>  
>  	/* Wait for writeback to complete on all folios and discard. */
>  	invalidate_inode_pages2_range(mapping, start / PAGE_SIZE, end / PAGE_SIZE);
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 02/10] 9p: don't opencode filemap_fdatawrite_range in v9fs_mmap_vm_close

[Jan Kara]

On Mon 13-10-25 11:57:57, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range instead of opencoding the logic using
> filemap_fdatawrite_wbc.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/9p/vfs_file.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
> index eb0b083da269..612a230bc012 100644
> --- a/fs/9p/vfs_file.c
> +++ b/fs/9p/vfs_file.c
> @@ -483,24 +483,15 @@ v9fs_vm_page_mkwrite(struct vm_fault *vmf)
>  
>  static void v9fs_mmap_vm_close(struct vm_area_struct *vma)
>  {
> -	struct inode *inode;
> -
> -	struct writeback_control wbc = {
> -		.nr_to_write = LONG_MAX,
> -		.sync_mode = WB_SYNC_ALL,
> -		.range_start = (loff_t)vma->vm_pgoff * PAGE_SIZE,
> -		 /* absolute end, byte at end included */
> -		.range_end = (loff_t)vma->vm_pgoff * PAGE_SIZE +
> -			(vma->vm_end - vma->vm_start - 1),
> -	};
> -
>  	if (!(vma->vm_flags & VM_SHARED))
>  		return;
>  
>  	p9_debug(P9_DEBUG_VFS, "9p VMA close, %p, flushing", vma);
>  
> -	inode = file_inode(vma->vm_file);
> -	filemap_fdatawrite_wbc(inode->i_mapping, &wbc);
> +	filemap_fdatawrite_range(file_inode(vma->vm_file)->i_mapping,
> +			(loff_t)vma->vm_pgoff * PAGE_SIZE,
> +			(loff_t)vma->vm_pgoff * PAGE_SIZE +
> +				(vma->vm_end - vma->vm_start - 1));
>  }
>  
>  static const struct vm_operations_struct v9fs_mmap_file_vm_ops = {
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 03/10] ocfs2: don't opencode filemap_fdatawrite_range in ocfs2_journal_submit_inode_data_buffers

[Jan Kara]

On Mon 13-10-25 11:57:58, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range instead of opencoding the logic using
> filemap_fdatawrite_wbc.  There is a slight change in the conversion
> as nr_to_write is now set to LONG_MAX instead of double the number
> of the pages in the range.  LONG_MAX is the usual nr_to_write for
> WB_SYNC_ALL writeback, and the value expected by lower layers here.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/ocfs2/journal.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index e5f58ff2175f..85239807dec7 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -902,15 +902,8 @@ int ocfs2_journal_alloc(struct ocfs2_super *osb)
>  
>  static int ocfs2_journal_submit_inode_data_buffers(struct jbd2_inode *jinode)
>  {
> -	struct address_space *mapping = jinode->i_vfs_inode->i_mapping;
> -	struct writeback_control wbc = {
> -		.sync_mode =  WB_SYNC_ALL,
> -		.nr_to_write = mapping->nrpages * 2,
> -		.range_start = jinode->i_dirty_start,
> -		.range_end = jinode->i_dirty_end,
> -	};
> -
> -	return filemap_fdatawrite_wbc(mapping, &wbc);
> +	return filemap_fdatawrite_range(jinode->i_vfs_inode->i_mapping,
> +			jinode->i_dirty_start, jinode->i_dirty_end);
>  }
>  
>  int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty)
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 06/10] mm,btrfs: add a filemap_fdatawrite_kick_nr helper

[Jan Kara]

On Mon 13-10-25 17:01:01, Damien Le Moal wrote:
> On 2025/10/13 11:58, Christoph Hellwig wrote:
> > Abstract out the btrfs-specific behavior of kicking off I/O on a number
> > of pages on an address_space into a well-defined helper.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> One nit below.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> 
> 
> > +/*
> > + * Start writeback on @nr_to_write pages from @mapping.  No one but the existing
> > + * btrfs caller should be using this.  Talk to linux-mm if you think adding a
> > + * new caller is a good idea.
> > + */
> > +int filemap_fdatawrite_kick_nr(struct address_space *mapping, long *nr_to_write)
> 
> Not a huge fan of this name. Maybe filemap_fdatawrite_nrpages() ?

I don't love filemap_fdatawrite_kick_nr() either. Your
filemap_fdatawrite_nrpages() is better but so far we had the distinction
that filemap_fdatawrite* is for data integrity writeback and filemap_flush
is for memory cleaning writeback. And in some places this is important
distinction which I'd like to keep obvious in the naming. So I'd prefer
something like filemap_flush_nrpages() (to stay consistent with previous
naming) or if Christoph doesn't like flush (as that's kind of overloaded
word) we could have filemap_writeback_nrpages().

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 07/10] mm: remove __filemap_fdatawrite

[Jan Kara]

On Mon 13-10-25 11:58:02, Christoph Hellwig wrote:
> And rewrite filemap_fdatawrite to use filemap_fdatawrite_range instead
> to have a simpler call chain.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

...

>  int filemap_fdatawrite(struct address_space *mapping)
>  {
> -	return __filemap_fdatawrite(mapping, WB_SYNC_ALL);
> +	return filemap_fdatawrite_range(mapping, 0, LONG_MAX);

As Damien pointed out, here should be LLONG_MAX.


> @@ -470,7 +464,7 @@ EXPORT_SYMBOL_GPL(filemap_fdatawrite_range_kick);
>   */
>  int filemap_flush(struct address_space *mapping)
>  {
> -	return __filemap_fdatawrite(mapping, WB_SYNC_NONE);
> +	return filemap_fdatawrite_range_kick(mapping, 0, LLONG_MAX);
>  }
>  EXPORT_SYMBOL(filemap_flush);

filemap_fdatawrite_range_kick() doesn't exist at this point in the series.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 08/10] mm: remove filemap_fdatawrite_wbc

[Jan Kara]

On Mon 13-10-25 11:58:03, Christoph Hellwig wrote:
> Replace filemap_fdatawrite_wbc, which exposes a writeback_control to the
> callers with a __filemap_fdatawrite helper that takes all the possible
> arguments and declares the writeback_control itself.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

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

								Honza

> ---
>  include/linux/pagemap.h |  2 --
>  mm/filemap.c            | 54 ++++++++++++++---------------------------
>  2 files changed, 18 insertions(+), 38 deletions(-)
> 
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index fc060ce2d31d..742ba1dd3990 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -61,8 +61,6 @@ int filemap_fdatawrite_range(struct address_space *mapping,
>  		loff_t start, loff_t end);
>  int filemap_check_errors(struct address_space *mapping);
>  void __filemap_set_wb_err(struct address_space *mapping, int err);
> -int filemap_fdatawrite_wbc(struct address_space *mapping,
> -			   struct writeback_control *wbc);
>  int kiocb_write_and_wait(struct kiocb *iocb, size_t count);
>  
>  static inline int filemap_write_and_wait(struct address_space *mapping)
> diff --git a/mm/filemap.c b/mm/filemap.c
> index bbd5d5eaa661..26b692dbf091 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -366,31 +366,30 @@ static int filemap_check_and_keep_errors(struct address_space *mapping)
>  	return 0;
>  }
>  
> -/**
> - * filemap_fdatawrite_wbc - start writeback on mapping dirty pages in range
> - * @mapping:	address space structure to write
> - * @wbc:	the writeback_control controlling the writeout
> - *
> - * Call writepages on the mapping using the provided wbc to control the
> - * writeout.
> - *
> - * Return: %0 on success, negative error code otherwise.
> - */
> -int filemap_fdatawrite_wbc(struct address_space *mapping,
> -			   struct writeback_control *wbc)
> +static int __filemap_fdatawrite(struct address_space *mapping, loff_t start,
> +		loff_t end, enum writeback_sync_modes sync_mode,
> +		long *nr_to_write)
>  {
> +	struct writeback_control wbc = {
> +		.sync_mode	= sync_mode,
> +		.nr_to_write	= nr_to_write ? *nr_to_write : LONG_MAX,
> +		.range_start	= start,
> +		.range_end	= end,
> +	};
>  	int ret;
>  
>  	if (!mapping_can_writeback(mapping) ||
>  	    !mapping_tagged(mapping, PAGECACHE_TAG_DIRTY))
>  		return 0;
>  
> -	wbc_attach_fdatawrite_inode(wbc, mapping->host);
> -	ret = do_writepages(mapping, wbc);
> -	wbc_detach_inode(wbc);
> +	wbc_attach_fdatawrite_inode(&wbc, mapping->host);
> +	ret = do_writepages(mapping, &wbc);
> +	wbc_detach_inode(&wbc);
> +
> +	if (!ret && nr_to_write)
> +		*nr_to_write = wbc.nr_to_write;
>  	return ret;
>  }
> -EXPORT_SYMBOL(filemap_fdatawrite_wbc);
>  
>  /**
>   * __filemap_fdatawrite_range - start writeback on mapping dirty pages in range
> @@ -412,14 +411,7 @@ EXPORT_SYMBOL(filemap_fdatawrite_wbc);
>  int __filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
>  				loff_t end, int sync_mode)
>  {
> -	struct writeback_control wbc = {
> -		.sync_mode = sync_mode,
> -		.nr_to_write = LONG_MAX,
> -		.range_start = start,
> -		.range_end = end,
> -	};
> -
> -	return filemap_fdatawrite_wbc(mapping, &wbc);
> +	return __filemap_fdatawrite(mapping, start, end, sync_mode, NULL);
>  }
>  
>  int filemap_fdatawrite_range(struct address_space *mapping, loff_t start,
> @@ -475,18 +467,8 @@ EXPORT_SYMBOL(filemap_flush);
>   */
>  int filemap_fdatawrite_kick_nr(struct address_space *mapping, long *nr_to_write)
>  {
> -	struct writeback_control wbc = {
> -		.nr_to_write = *nr_to_write,
> -		.sync_mode = WB_SYNC_NONE,
> -		.range_start = 0,
> -		.range_end = LLONG_MAX,
> -	};
> -	int ret;
> -
> -	ret = filemap_fdatawrite_wbc(mapping, &wbc);
> -	if (!ret)
> -		*nr_to_write = wbc.nr_to_write;
> -	return ret;
> +	return __filemap_fdatawrite(mapping, 0, LLONG_MAX, WB_SYNC_NONE,
> +			nr_to_write);
>  }
>  EXPORT_SYMBOL_FOR_MODULES(filemap_fdatawrite_kick_nr, "btrfs");
>  
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 09/10] mm: remove __filemap_fdatawrite_range

[Jan Kara]

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

Looks good. Feel free to add:

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

Re: [PATCH 01/10] mm: don't opencode filemap_fdatawrite_range in filemap_invalidate_inode

[David Hildenbrand]

On 13.10.25 04:57, Christoph Hellwig wrote:
> Use filemap_fdatawrite_range instead of opencoding the logic using
> filemap_fdatawrite_wbc.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH 06/10] mm,btrfs: add a filemap_fdatawrite_kick_nr helper

[David Hildenbrand]

> @@ -474,6 +474,28 @@ int filemap_flush(struct address_space *mapping)
>   }
>   EXPORT_SYMBOL(filemap_flush);
>   
> +/*
> + * Start writeback on @nr_to_write pages from @mapping.  No one but the existing
> + * btrfs caller should be using this.  Talk to linux-mm if you think adding a
> + * new caller is a good idea.
> + */

Nit: We seem to prefer proper kerneldoc for filemap_fdatawrite* functions.

> +int filemap_fdatawrite_kick_nr(struct address_space *mapping, long *nr_to_write)
> +{
> +	struct writeback_control wbc = {
> +		.nr_to_write = *nr_to_write,
> +		.sync_mode = WB_SYNC_NONE,
> +		.range_start = 0,
> +		.range_end = LLONG_MAX,
> +	};
> +	int ret;
> +
> +	ret = filemap_fdatawrite_wbc(mapping, &wbc);
> +	if (!ret)
> +		*nr_to_write = wbc.nr_to_write;
> +	return ret;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(filemap_fdatawrite_kick_nr, "btrfs");
> +
>   /**
>    * filemap_range_has_page - check if a page exists in range.
>    * @mapping:           address space within which to check


I think there is still a discussion on the name, but in general LGTM

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH 08/10] mm: remove filemap_fdatawrite_wbc

[David Hildenbrand]

On 13.10.25 04:58, Christoph Hellwig wrote:
> Replace filemap_fdatawrite_wbc, which exposes a writeback_control to the
> callers with a __filemap_fdatawrite helper that takes all the possible
> arguments and declares the writeback_control itself.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH 04/10] btrfs: use the local tmp_inode variable in start_delalloc_inodes

[hch]

On Mon, Oct 13, 2025 at 08:11:35AM +0000, Johannes Thumshirn wrote:
> If you have to repost this for some reason, can you rename tmp_inode to 
> vfs_inode or sth like that?
> 
> The name is really confusing and the commit introducing it doesn't 
> describe it really either.

It is.  vfs_inode is kinda weird, too.  The problem is that inode
is used for the btrfs_inode.  But if there's consensus on a name
I'll happily change it.

Re: [PATCH 06/10] mm,btrfs: add a filemap_fdatawrite_kick_nr helper

[Christoph Hellwig]

On Mon, Oct 13, 2025 at 01:58:15PM +0200, Jan Kara wrote:
> I don't love filemap_fdatawrite_kick_nr() either. Your
> filemap_fdatawrite_nrpages() is better but so far we had the distinction
> that filemap_fdatawrite* is for data integrity writeback and filemap_flush
> is for memory cleaning writeback. And in some places this is important
> distinction which I'd like to keep obvious in the naming. So I'd prefer
> something like filemap_flush_nrpages() (to stay consistent with previous
> naming) or if Christoph doesn't like flush (as that's kind of overloaded
> word) we could have filemap_writeback_nrpages().

Not a big fan of flush, but the important point in this series is
to have consistent naming.  If we don't like the kick naming
we should standardize on _flush (or whatever) and have the _range and
_nrpages variants of whatever we pick for the base name.

Anyone with strong feelings and or good ideas about naming please speak
up now.

Re: [PATCH 06/10] mm,btrfs: add a filemap_fdatawrite_kick_nr helper

[Christoph Hellwig]

On Mon, Oct 13, 2025 at 02:48:48PM +0200, David Hildenbrand wrote:
>>   +/*
>> + * Start writeback on @nr_to_write pages from @mapping.  No one but the existing
>> + * btrfs caller should be using this.  Talk to linux-mm if you think adding a
>> + * new caller is a good idea.
>> + */
>
> Nit: We seem to prefer proper kerneldoc for filemap_fdatawrite* functions.

Because this is mentioned as only export for btrfs and using
EXPORT_SYMBOL_FOR_MODULES I explicitly do not want it to show up in
the generated documentation, so this was intentional.  Unless we want
to make this a fully supported part of the API, in which case the export
type should change, and it should grow a kerneldoc comment.

Re: [PATCH 07/10] mm: remove __filemap_fdatawrite

[Christoph Hellwig]

On Mon, Oct 13, 2025 at 05:02:50PM +0900, Damien Le Moal wrote:
> >  int filemap_fdatawrite(struct address_space *mapping)
> >  {
> > -	return __filemap_fdatawrite(mapping, WB_SYNC_ALL);
> > +	return filemap_fdatawrite_range(mapping, 0, LONG_MAX);
> 
> This should be LLONG_MAX, no ?

Yes, fixed.

Re: [PATCH 07/10] mm: remove __filemap_fdatawrite

[Christoph Hellwig]

On Mon, Oct 13, 2025 at 01:59:21PM +0200, Jan Kara wrote:
> > -	return __filemap_fdatawrite(mapping, WB_SYNC_NONE);
> > +	return filemap_fdatawrite_range_kick(mapping, 0, LLONG_MAX);
> >  }
> >  EXPORT_SYMBOL(filemap_flush);
> 
> filemap_fdatawrite_range_kick() doesn't exist at this point in the series.

It does exist even in the current upstream kernel.
filemap_fdatawrite_kick doesn't exist yet.

Re: [PATCH 04/10] btrfs: use the local tmp_inode variable in start_delalloc_inodes

[Johannes Thumshirn]

On 10/14/25 6:44 AM, hch wrote:
> On Mon, Oct 13, 2025 at 08:11:35AM +0000, Johannes Thumshirn wrote:
>> If you have to repost this for some reason, can you rename tmp_inode to
>> vfs_inode or sth like that?
>>
>> The name is really confusing and the commit introducing it doesn't
>> describe it really either.
> It is.  vfs_inode is kinda weird, too.  The problem is that inode
> is used for the btrfs_inode.  But if there's consensus on a name
> I'll happily change it.
>
I unfortunately don't have one :( David?

Re: [PATCH 06/10] mm,btrfs: add a filemap_fdatawrite_kick_nr helper

[Jan Kara]

On Tue 14-10-25 06:47:23, Christoph Hellwig wrote:
> On Mon, Oct 13, 2025 at 01:58:15PM +0200, Jan Kara wrote:
> > I don't love filemap_fdatawrite_kick_nr() either. Your
> > filemap_fdatawrite_nrpages() is better but so far we had the distinction
> > that filemap_fdatawrite* is for data integrity writeback and filemap_flush
> > is for memory cleaning writeback. And in some places this is important
> > distinction which I'd like to keep obvious in the naming. So I'd prefer
> > something like filemap_flush_nrpages() (to stay consistent with previous
> > naming) or if Christoph doesn't like flush (as that's kind of overloaded
> > word) we could have filemap_writeback_nrpages().
> 
> Not a big fan of flush, but the important point in this series is
> to have consistent naming.

I fully agree on that.

>  If we don't like the kick naming we should standardize on _flush (or
>  whatever) and have the _range and _nrpages variants of whatever we pick
>  for the base name.
> 
> Anyone with strong feelings and or good ideas about naming please speak
> up now.

I agree with either keeping filemap_flush* or using filemap_writeback* (and
renaming filemap_flush to filemap_writeback).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 07/10] mm: remove __filemap_fdatawrite

[Jan Kara]

On Tue 14-10-25 06:53:25, Christoph Hellwig wrote:
> On Mon, Oct 13, 2025 at 01:59:21PM +0200, Jan Kara wrote:
> > > -	return __filemap_fdatawrite(mapping, WB_SYNC_NONE);
> > > +	return filemap_fdatawrite_range_kick(mapping, 0, LLONG_MAX);
> > >  }
> > >  EXPORT_SYMBOL(filemap_flush);
> > 
> > filemap_fdatawrite_range_kick() doesn't exist at this point in the series.
> 
> It does exist even in the current upstream kernel.
> filemap_fdatawrite_kick doesn't exist yet.

Aha, I was wondering where that "kick" naming was coming from :) I've
totally missed adding of filemap_fdatawrite_range_kick(). Sorry for the
noise but I still think filemap_flush / filemap_writeback better express
the intention than filemap_fdatawrite_kick.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 06/10] mm,btrfs: add a filemap_fdatawrite_kick_nr helper

[David Hildenbrand]

On 14.10.25 06:49, Christoph Hellwig wrote:
> On Mon, Oct 13, 2025 at 02:48:48PM +0200, David Hildenbrand wrote:
>>>    +/*
>>> + * Start writeback on @nr_to_write pages from @mapping.  No one but the existing
>>> + * btrfs caller should be using this.  Talk to linux-mm if you think adding a
>>> + * new caller is a good idea.
>>> + */
>>
>> Nit: We seem to prefer proper kerneldoc for filemap_fdatawrite* functions.
> 
> Because this is mentioned as only export for btrfs and using
> EXPORT_SYMBOL_FOR_MODULES I explicitly do not want it to show up in
> the generated documentation, so this was intentional.  Unless we want
> to make this a fully supported part of the API, in which case the export
> type should change, and it should grow a kerneldoc comment.


Ah okay, mentioning the intention with not adding kernel doc in the 
patch description would have been nice :)

-- 
Cheers

David / dhildenb