diff mbox series

[PATCHv8,3/5] iomap: Refactor iomap_write_delalloc_punch() function out

Message ID ee74cd3dfb34ec321f70ee67c3a5d1f40fdc585f.1686050333.git.ritesh.list@gmail.com (mailing list archive)
State New, archived
Headers show
Series iomap: Add support for per-block dirty state to improve write performance | expand

Commit Message

Ritesh Harjani (IBM) June 6, 2023, 11:43 a.m. UTC 
This patch moves iomap_write_delalloc_punch() out of
iomap_write_delalloc_scan(). No functionality change in this patch.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 fs/iomap/buffered-io.c | 54 ++++++++++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 20 deletions(-)

Comments

Christoph Hellwig June 7, 2023, 6:52 a.m. UTC | #1 
On Tue, Jun 06, 2023 at 05:13:50PM +0530, Ritesh Harjani (IBM) wrote:
> This patch moves iomap_write_delalloc_punch() out of
> iomap_write_delalloc_scan(). No functionality change in this patch.

Please chose one refactor (the existing function), or factor (the
new function) out.  The mix doesn't make much sense.

Also please explain why you're doing that.  The fact tha a new helper
is split out is pretty obvious from the patch, but I have no idea why
you want it.
Ritesh Harjani (IBM) June 7, 2023, 10:55 a.m. UTC | #2 
Christoph Hellwig <hch@infradead.org> writes:

> On Tue, Jun 06, 2023 at 05:13:50PM +0530, Ritesh Harjani (IBM) wrote:
>> This patch moves iomap_write_delalloc_punch() out of
>> iomap_write_delalloc_scan(). No functionality change in this patch.
>
> Please chose one refactor (the existing function), or factor (the
> new function) out.  The mix doesn't make much sense.
>
> Also please explain why you're doing that.  The fact tha a new helper
> is split out is pretty obvious from the patch, but I have no idea why
> you want it.

This patch factors iomap_write_delalloc_punch() function out. This function
is resposible for actual punch out operation.
The reason for this is, to avoid deep indentation when we bring
punch-out of individual non-dirty blocks within a dirty folio in a later patch
to avoid delalloc block leak. This later patch is what adds per-block
dirty status handling to iomap.

I will rephrase commit message to above ^^^.
Let me know if any other changes requried.

-ritesh
diff mbox series

Patch

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 08f2a1cf0a66..89489aed49c0 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -888,6 +888,33 @@  iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i,
 }
 EXPORT_SYMBOL_GPL(iomap_file_buffered_write);
 
+static int iomap_write_delalloc_punch(struct inode *inode, struct folio *folio,
+		loff_t *punch_start_byte, loff_t start_byte, loff_t end_byte,
+		int (*punch)(struct inode *inode, loff_t offset, loff_t length))
+{
+	int ret = 0;
+
+	if (!folio_test_dirty(folio))
+		return ret;
+
+	/* if dirty, punch up to offset */
+	if (start_byte > *punch_start_byte) {
+		ret = punch(inode, *punch_start_byte,
+				start_byte - *punch_start_byte);
+		if (ret)
+			goto out;
+	}
+	/*
+	 * Make sure the next punch start is correctly bound to
+	 * the end of this data range, not the end of the folio.
+	 */
+	*punch_start_byte = min_t(loff_t, end_byte,
+				  folio_next_index(folio) << PAGE_SHIFT);
+
+out:
+	return ret;
+}
+
 /*
  * Scan the data range passed to us for dirty page cache folios. If we find a
  * dirty folio, punch out the preceeding range and update the offset from which
@@ -911,6 +938,7 @@  static int iomap_write_delalloc_scan(struct inode *inode,
 {
 	while (start_byte < end_byte) {
 		struct folio	*folio;
+		int ret;
 
 		/* grab locked page */
 		folio = filemap_lock_folio(inode->i_mapping,
@@ -921,26 +949,12 @@  static int iomap_write_delalloc_scan(struct inode *inode,
 			continue;
 		}
 
-		/* if dirty, punch up to offset */
-		if (folio_test_dirty(folio)) {
-			if (start_byte > *punch_start_byte) {
-				int	error;
-
-				error = punch(inode, *punch_start_byte,
-						start_byte - *punch_start_byte);
-				if (error) {
-					folio_unlock(folio);
-					folio_put(folio);
-					return error;
-				}
-			}
-
-			/*
-			 * Make sure the next punch start is correctly bound to
-			 * the end of this data range, not the end of the folio.
-			 */
-			*punch_start_byte = min_t(loff_t, end_byte,
-					folio_next_index(folio) << PAGE_SHIFT);
+		ret = iomap_write_delalloc_punch(inode, folio, punch_start_byte,
+						 start_byte, end_byte, punch);
+		if (ret) {
+			folio_unlock(folio);
+			folio_put(folio);
+			return ret;
 		}
 
 		/* move offset to start of next folio in range */