diff mbox series

iomap: writeback_control pointer part of iomap_writepage_ctx

Message ID 326b2b66114c97b892dbcf83f3d41b86c64e93d6.1729266269.git.rgoldwyn@suse.com (mailing list archive)
State New
Headers show
Series iomap: writeback_control pointer part of iomap_writepage_ctx | expand

Commit Message

Goldwyn Rodrigues Oct. 18, 2024, 3:55 p.m. UTC 
Reduces the number of arguments to functions iomap_writepages() and
all functions in the writeback path which require both wpc and wbc.
The filesystems need to initialize wpc with wbc before calling
iomap_writepages().

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 block/fops.c           |  6 ++++--
 fs/gfs2/aops.c         |  6 ++++--
 fs/iomap/buffered-io.c | 31 +++++++++++++++----------------
 fs/xfs/xfs_aops.c      |  8 ++++++--
 fs/zonefs/file.c       |  6 ++++--
 include/linux/iomap.h  |  3 ++-
 6 files changed, 35 insertions(+), 25 deletions(-)

Comments

Dave Chinner Oct. 21, 2024, 10:30 p.m. UTC | #1 
On Fri, Oct 18, 2024 at 11:55:50AM -0400, Goldwyn Rodrigues wrote:
> Reduces the number of arguments to functions iomap_writepages() and
> all functions in the writeback path which require both wpc and wbc.
> The filesystems need to initialize wpc with wbc before calling
> iomap_writepages().
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>

Looks reasonable to me - there's only one path this comes in since
we got rid of iomap_writepage()...

Reviewed-by: Dave Chinner <dchinner@redhat.com>
Christoph Hellwig Oct. 22, 2024, 5:47 a.m. UTC | #2 
On Fri, Oct 18, 2024 at 11:55:50AM -0400, Goldwyn Rodrigues wrote:
> Reduces the number of arguments to functions iomap_writepages() and
> all functions in the writeback path which require both wpc and wbc.
> The filesystems need to initialize wpc with wbc before calling
> iomap_writepages().

While this looks obviously correct, I'm not sure what the point of
it is as it adds slightly more lines of code.  Does it generate
better binary code?  Do you have future changes that depend on it?
Goldwyn Rodrigues Oct. 23, 2024, 12:49 a.m. UTC | #3 
On 22:47 21/10, Christoph Hellwig wrote:
> On Fri, Oct 18, 2024 at 11:55:50AM -0400, Goldwyn Rodrigues wrote:
> > Reduces the number of arguments to functions iomap_writepages() and
> > all functions in the writeback path which require both wpc and wbc.
> > The filesystems need to initialize wpc with wbc before calling
> > iomap_writepages().
> 
> While this looks obviously correct, I'm not sure what the point of
> it is as it adds slightly more lines of code.  Does it generate
> better binary code?  Do you have future changes that depend on it?
> 

No future updates depending on it. It just makes the code
more readable.

No point bouncing two pointers to different functions in the write-
back path, when one can be encompassed into another.
diff mbox series

Patch

diff --git a/block/fops.c b/block/fops.c
index e696ae53bf1e..3425bb72e887 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -513,9 +513,11 @@  static const struct iomap_writeback_ops blkdev_writeback_ops = {
 static int blkdev_writepages(struct address_space *mapping,
 		struct writeback_control *wbc)
 {
-	struct iomap_writepage_ctx wpc = { };
+	struct iomap_writepage_ctx wpc = {
+		.wbc	=	wbc,
+	};
 
-	return iomap_writepages(mapping, wbc, &wpc, &blkdev_writeback_ops);
+	return iomap_writepages(mapping, &wpc, &blkdev_writeback_ops);
 }
 
 const struct address_space_operations def_blk_aops = {
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 68fc8af14700..e741bd34453d 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -149,7 +149,9 @@  static int gfs2_writepages(struct address_space *mapping,
 			   struct writeback_control *wbc)
 {
 	struct gfs2_sbd *sdp = gfs2_mapping2sbd(mapping);
-	struct iomap_writepage_ctx wpc = { };
+	struct iomap_writepage_ctx wpc = {
+			.wbc	= wbc,
+	};
 	int ret;
 
 	/*
@@ -158,7 +160,7 @@  static int gfs2_writepages(struct address_space *mapping,
 	 * want balance_dirty_pages() to loop indefinitely trying to write out
 	 * pages held in the ail that it can't find.
 	 */
-	ret = iomap_writepages(mapping, wbc, &wpc, &gfs2_writeback_ops);
+	ret = iomap_writepages(mapping, &wpc, &gfs2_writeback_ops);
 	if (ret == 0 && wbc->nr_to_write > 0)
 		set_bit(SDF_FORCE_AIL_FLUSH, &sdp->sd_flags);
 	return ret;
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 78ebd265f425..9c199a34b017 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -1757,17 +1757,17 @@  static int iomap_submit_ioend(struct iomap_writepage_ctx *wpc, int error)
 }
 
 static struct iomap_ioend *iomap_alloc_ioend(struct iomap_writepage_ctx *wpc,
-		struct writeback_control *wbc, struct inode *inode, loff_t pos)
+		struct inode *inode, loff_t pos)
 {
 	struct iomap_ioend *ioend;
 	struct bio *bio;
 
 	bio = bio_alloc_bioset(wpc->iomap.bdev, BIO_MAX_VECS,
-			       REQ_OP_WRITE | wbc_to_write_flags(wbc),
+			       REQ_OP_WRITE | wbc_to_write_flags(wpc->wbc),
 			       GFP_NOFS, &iomap_ioend_bioset);
 	bio->bi_iter.bi_sector = iomap_sector(&wpc->iomap, pos);
 	bio->bi_end_io = iomap_writepage_end_bio;
-	wbc_init_bio(wbc, bio);
+	wbc_init_bio(wpc->wbc, bio);
 	bio->bi_write_hint = inode->i_write_hint;
 
 	ioend = iomap_ioend_from_bio(bio);
@@ -1817,8 +1817,8 @@  static bool iomap_can_add_to_ioend(struct iomap_writepage_ctx *wpc, loff_t pos)
  * writepage context that the caller will need to submit.
  */
 static int iomap_add_to_ioend(struct iomap_writepage_ctx *wpc,
-		struct writeback_control *wbc, struct folio *folio,
-		struct inode *inode, loff_t pos, unsigned len)
+		struct folio *folio, struct inode *inode,
+		loff_t pos, unsigned len)
 {
 	struct iomap_folio_state *ifs = folio->private;
 	size_t poff = offset_in_folio(folio, pos);
@@ -1829,7 +1829,7 @@  static int iomap_add_to_ioend(struct iomap_writepage_ctx *wpc,
 		error = iomap_submit_ioend(wpc, 0);
 		if (error)
 			return error;
-		wpc->ioend = iomap_alloc_ioend(wpc, wbc, inode, pos);
+		wpc->ioend = iomap_alloc_ioend(wpc, inode, pos);
 	}
 
 	if (!bio_add_folio(&wpc->ioend->io_bio, folio, len, poff))
@@ -1838,14 +1838,13 @@  static int iomap_add_to_ioend(struct iomap_writepage_ctx *wpc,
 	if (ifs)
 		atomic_add(len, &ifs->write_bytes_pending);
 	wpc->ioend->io_size += len;
-	wbc_account_cgroup_owner(wbc, &folio->page, len);
+	wbc_account_cgroup_owner(wpc->wbc, &folio->page, len);
 	return 0;
 }
 
 static int iomap_writepage_map_blocks(struct iomap_writepage_ctx *wpc,
-		struct writeback_control *wbc, struct folio *folio,
-		struct inode *inode, u64 pos, unsigned dirty_len,
-		unsigned *count)
+		struct folio *folio, struct inode *inode, u64 pos,
+		unsigned dirty_len, unsigned *count)
 {
 	int error;
 
@@ -1869,7 +1868,7 @@  static int iomap_writepage_map_blocks(struct iomap_writepage_ctx *wpc,
 		case IOMAP_HOLE:
 			break;
 		default:
-			error = iomap_add_to_ioend(wpc, wbc, folio, inode, pos,
+			error = iomap_add_to_ioend(wpc, folio, inode, pos,
 					map_len);
 			if (!error)
 				(*count)++;
@@ -1952,7 +1951,7 @@  static bool iomap_writepage_handle_eof(struct folio *folio, struct inode *inode,
 }
 
 static int iomap_writepage_map(struct iomap_writepage_ctx *wpc,
-		struct writeback_control *wbc, struct folio *folio)
+		struct folio *folio)
 {
 	struct iomap_folio_state *ifs = folio->private;
 	struct inode *inode = folio->mapping->host;
@@ -2000,7 +1999,7 @@  static int iomap_writepage_map(struct iomap_writepage_ctx *wpc,
 	 * Walk through the folio to find dirty areas to write back.
 	 */
 	while ((rlen = iomap_find_dirty_range(folio, &pos, end_pos))) {
-		error = iomap_writepage_map_blocks(wpc, wbc, folio, inode,
+		error = iomap_writepage_map_blocks(wpc, folio, inode,
 				pos, rlen, &count);
 		if (error)
 			break;
@@ -2037,7 +2036,7 @@  static int iomap_writepage_map(struct iomap_writepage_ctx *wpc,
 }
 
 int
-iomap_writepages(struct address_space *mapping, struct writeback_control *wbc,
+iomap_writepages(struct address_space *mapping,
 		struct iomap_writepage_ctx *wpc,
 		const struct iomap_writeback_ops *ops)
 {
@@ -2053,8 +2052,8 @@  iomap_writepages(struct address_space *mapping, struct writeback_control *wbc,
 		return -EIO;
 
 	wpc->ops = ops;
-	while ((folio = writeback_iter(mapping, wbc, folio, &error)))
-		error = iomap_writepage_map(wpc, wbc, folio);
+	while ((folio = writeback_iter(mapping, wpc->wbc, folio, &error)))
+		error = iomap_writepage_map(wpc, folio);
 	return iomap_submit_ioend(wpc, error);
 }
 EXPORT_SYMBOL_GPL(iomap_writepages);
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 6dead20338e2..5d758910a843 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -471,10 +471,14 @@  xfs_vm_writepages(
 	struct address_space	*mapping,
 	struct writeback_control *wbc)
 {
-	struct xfs_writepage_ctx wpc = { };
+	struct xfs_writepage_ctx wpc = {
+		.ctx = {
+			.wbc	= wbc,
+		},
+	};
 
 	xfs_iflags_clear(XFS_I(mapping->host), XFS_ITRUNCATED);
-	return iomap_writepages(mapping, wbc, &wpc.ctx, &xfs_writeback_ops);
+	return iomap_writepages(mapping, &wpc.ctx, &xfs_writeback_ops);
 }
 
 STATIC int
diff --git a/fs/zonefs/file.c b/fs/zonefs/file.c
index 35166c92420c..51b03689b976 100644
--- a/fs/zonefs/file.c
+++ b/fs/zonefs/file.c
@@ -152,9 +152,11 @@  static const struct iomap_writeback_ops zonefs_writeback_ops = {
 static int zonefs_writepages(struct address_space *mapping,
 			     struct writeback_control *wbc)
 {
-	struct iomap_writepage_ctx wpc = { };
+	struct iomap_writepage_ctx wpc = {
+		.wbc	= wbc,
+	};
 
-	return iomap_writepages(mapping, wbc, &wpc, &zonefs_writeback_ops);
+	return iomap_writepages(mapping, &wpc, &zonefs_writeback_ops);
 }
 
 static int zonefs_swap_activate(struct swap_info_struct *sis,
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 4ad12a3c8bae..2435ad63d1ad 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -341,6 +341,7 @@  struct iomap_writeback_ops {
 struct iomap_writepage_ctx {
 	struct iomap		iomap;
 	struct iomap_ioend	*ioend;
+	struct writeback_control *wbc;
 	const struct iomap_writeback_ops *ops;
 	u32			nr_folios;	/* folios added to the ioend */
 };
@@ -350,7 +351,7 @@  void iomap_ioend_try_merge(struct iomap_ioend *ioend,
 		struct list_head *more_ioends);
 void iomap_sort_ioends(struct list_head *ioend_list);
 int iomap_writepages(struct address_space *mapping,
-		struct writeback_control *wbc, struct iomap_writepage_ctx *wpc,
+		struct iomap_writepage_ctx *wpc,
 		const struct iomap_writeback_ops *ops);
 
 /*