Document handing of unwritten bh in ext4_block_zero_page_range()

The call to filemap_write_and_wait_range() assumes the range passed to be inclusive, so fix the call to make sure we follow that.

Signed-off-by Ojaswin Mujoo <[email protected]>

---
 fs/ext4/extents.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

fs/ext4/extents.c +4 -2

index 880f383df684..265ae30a51b9 100644
@@ -4522,7 +4522,8 @@ static long ext4_zero_range(struct file *file, loff_t offset,
 	 * Round up offset. This is not fallocate, we need to zero out
 	 * blocks, so convert interior block aligned part of the range to
 	 * unwritten and possibly manually zero out unaligned parts of the
-	 * range.
+	 * range. Here, start and partial_begin are inclusive, end and
+	 * partial_end are exclusive.
 	 */
 	start = round_up(offset, 1 << blkbits);
 	end = round_down((offset + len), 1 << blkbits);

@@ -4608,7 +4609,8 @@ static long ext4_zero_range(struct file *file, loff_t offset,
 		 * disk in case of crash before zeroing trans is committed.
 		 */
 		if (ext4_should_journal_data(inode)) {
-			ret = filemap_write_and_wait_range(mapping, start, end);
+			ret = filemap_write_and_wait_range(mapping, start,
+							   end - 1);
 			if (ret) {
 				filemap_invalidate_unlock(mapping);
 				goto out_mutex;

Signature · 1 line

2.39.3

As an optimization, I was trying to work on exiting early from this function if dealing with unwritten extent since they anyways read 0. However, it was realised that there are certain code paths that can end up calling ext4_block_zero_page_range() for an unwritten bh that might still have data in pagecache. In this case, we can't exit early and we do require to process the bh and zero out the pagecache to ensure that a writeback can't kick in at a later time and flush the stale pagecache to disk.

Since, adding the logic to exit early for unwritten bh was turning out to be much more nuanced and the current code already handles it well, just add a comment to explicitly document this behavior.

Suggested-by Jan Kara <[email protected]>

Signed-off-by Ojaswin Mujoo <[email protected]>

---
 fs/ext4/inode.c | 6 ++++++
 1 file changed, 6 insertions(+)

fs/ext4/inode.c +6 -0

index d7732320431a..76921e834dd4 100644
@@ -3632,6 +3632,12 @@ void ext4_set_aops(struct inode *inode)
 		inode->i_mapping->a_ops = &ext4_aops;
 }
 
+/*
+ * Here we can't skip an unwritten buffer even though it usually reads zero
+ * because it might have data in pagecache (eg, if called from ext4_zero_range,
+ * ext4_punch_hole, etc) which needs to be properly zeroed out. Otherwise a
+ * racing writeback can come later and flush the stale pagecache to disk.
+ */
 static int __ext4_block_zero_page_range(handle_t *handle,
 		struct address_space *mapping, loff_t from, loff_t length)
 {

Signature · 1 line

2.39.3

** Changes v1 -> v2 **

* Instead of exiting early in ext4_block_zero_page_range() for unwrit bhs like in v1, lets let the current logic be as it is and instead document the handing of unwritten buffer heads to make the intent clear

[...]