[PATCH 8/8] ext4: optimize inflight dio tracking for zeroout

[Ojaswin Mujoo <ojaswin@linux.ibm.com>]

Earlier zeroout used inode->i_dio_count to detect in-flight DIOs
that might have valid data underneath an unwritten extent. However,
i_dio_count accounts for every DIO on the inode, including reads and writes
that never touch an unwritten extent, which makes it prone to false
positives and needlessly suppresses the zeroout fallback.

Optimize this by moving DIO accounting to i_conv_pending_ios, which is only
incremented for DIOs that actually encounter an unwritten extent, and
decremented once at endio after the conversion is done.

The main issue is that ext4_iomap_begin() doesn't have any way to track
something only once per dio, hence, we use a custom ext4_iomap_next() that
utilizes iter->private to ensure we increment the i_conv_pending_ios only
once per a dio that encounters unwritten extents.

Now that dio has moved to i_conv_pending_ios, we need to pass NO_CONV_ACC
get blocks flag to ext4_ext_convert_to_initialized(). This path is
reachable from dio but doesn't need to account in i_conv_pending_ios
because we don't issue any IO before conversion, hence the flag helps
zeroout to make the right call.

Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/ext4.h    |  2 --
 fs/ext4/extents.c | 11 ++-----
 fs/ext4/file.c    | 12 +++++++-
 fs/ext4/inode.c   | 85 ++++++++++++++++++++++++++++---------------------------
 4 files changed, 58 insertions(+), 52 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index bf390b3b5606..44cd9f12d350 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -4002,8 +4002,6 @@ extern void ext4_conv_pending_sum(struct inode *inode, int n);
 extern int ext4_conv_pending_inc_trylock(struct inode *inode);
 extern void ext4_conv_pending_dec(struct inode *inode);
 extern void ext4_conv_pending_sub(struct inode *inode, int n);
-extern void ext4_conv_pending_barrier(struct inode *inode);
-extern int ext4_conv_pending_barrier_trylock(struct inode *inode);
 extern int ext4_ext_calc_credits_for_single_extent(struct inode *inode,
 						   int num,
 						   struct ext4_ext_path *path);
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d546001fedc6..3150754b9766 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -3333,13 +3333,7 @@ static bool ext4_conv_pending_zeroout_lock(struct inode *inode, int flags)
 	pending = atomic_read(&ei->i_conv_pending_ios);
 
 	WARN_ON_ONCE(pending < 0);
-	if (pending < 0) {
-		up_write(&ei->i_conv_pending_sem);
-		return false;
-	}
-
-	pending += atomic_read(&inode->i_dio_count);
-	if (pending > thresh) {
+	if (pending > thresh || pending < 0) {
 		up_write(&ei->i_conv_pending_sem);
 		return false;
 	}
@@ -4117,7 +4111,8 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
 	 * discovered unwritten extent to written.
 	 */
 	path = ext4_ext_convert_to_initialized(handle, inode, map, path,
-					       flags, allocated);
+					       flags | EXT4_GET_BLOCKS_NO_CONV_ACC,
+					       allocated);
 	if (IS_ERR(path))
 		return path;
 	ext4_update_inode_fsync_trans(handle, inode, 1);
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 06ef7a9b9f83..52b1fb99484d 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -458,6 +458,15 @@ static int ext4_dio_write_end_io(struct kiocb *iocb, ssize_t size,
 	else if (!error && size && flags & IOMAP_DIO_UNWRITTEN)
 		error = ext4_convert_unwritten_extents(NULL, inode, pos, size,
 						       NULL, 0);
+
+	/*
+	 * If this dio covered an unwritten extent, we would have incremented
+	 * i_conv_pending_ios in ext4_iomap_next(). Decrement it now that
+	 * the conversion has completed (or failed).
+	 */
+	if (flags & IOMAP_DIO_UNWRITTEN)
+		ext4_conv_pending_dec(inode);
+
 	if (error)
 		return error;
 	/*
@@ -599,6 +608,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	size_t count = iov_iter_count(from);
 	bool extend = false;
 	bool ilock_shared = true;
+	bool conv_pending_inc = false;
 	int dio_flags = 0;
 
 	/*
@@ -664,7 +674,7 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	}
 
 	ret = iomap_dio_rw(iocb, from, &ext4_iomap_ops, &ext4_dio_write_ops,
-			   dio_flags, NULL, 0);
+			   dio_flags, &conv_pending_inc, 0);
 	if (ret == -ENOTBLK)
 		ret = 0;
 	if (extend) {
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 0c924ad8210b..46f91d6d8ed5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -692,9 +692,9 @@ static void _ext4_conv_pending_mod(struct inode *inode, int n)
  * they might have data underneath which is pending endio conversion. Zeroing
  * out such an extent can cause silent corruption.
  *
- * To fix this we track the no. of buffered IOs which are about to undergo
- * conversion in ei->i_conv_pending_ios. The zeroout code then uses this counter
- * to determine if it's safe to zeroout.
+ * To fix this we track the IOs which are about to undergo conversion in
+ * ei->i_conv_pending_ios. The zeroout code then uses this counter to
+ * determine if it's safe to zeroout.
  *
  * Design Notes:
  *
@@ -715,9 +715,6 @@ static void _ext4_conv_pending_mod(struct inode *inode, int n)
  * don't see any heavy lock contention. Atomic counters help parallel
  * modifications to be serialized.
  *
- * Note that dio does not use i_conv_pending_ios; instead we use i_dio_count to
- * disable zeroout if there are any other inflight dios.
- *
  * ** Tolerance for false positives **
  *
  * We trade false positives for simplicity of implementation because having a
@@ -759,30 +756,6 @@ int ext4_conv_pending_inc_trylock(struct inode *inode)
 	return 0;
 }
 
-/*
- * Rotate i_conv_pending_sem. This guarantees mutual exclusion against a
- * concurrent endio zeroout, which holds i_conv_pending_sem exclusive. This
- * barrier ensures either the zeroout has completed before we return, or it will
- * start after we have accounted for the io and will see it while zeroing out.
- * This can be used for paths that might not be changing ei->i_conv_pending_ios
- */
-void ext4_conv_pending_barrier(struct inode *inode)
-{
-	down_read(&EXT4_I(inode)->i_conv_pending_sem);
-	up_read(&EXT4_I(inode)->i_conv_pending_sem);
-}
-
-/*
- * Same as ext4_conv_pending_barrier() but doesn't sleep on i_conv_pending_sem.
- */
-int ext4_conv_pending_barrier_trylock(struct inode *inode)
-{
-	if (!down_read_trylock(&EXT4_I(inode)->i_conv_pending_sem))
-		return -EAGAIN;
-	up_read(&EXT4_I(inode)->i_conv_pending_sem);
-	return 0;
-}
-
 void ext4_conv_pending_dec(struct inode *inode)
 {
 	_ext4_conv_pending_mod(inode, -1);
@@ -4100,16 +4073,6 @@ int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 	if (ret < 0)
 		return ret;
 out:
-	if ((flags & IOMAP_WRITE) && (map.m_flags & EXT4_MAP_UNWRITTEN)) {
-		if (flags & IOMAP_NOWAIT) {
-			ret = ext4_conv_pending_barrier_trylock(inode);
-			if (ret)
-				return ret;
-		} else {
-			ext4_conv_pending_barrier(inode);
-		}
-	}
-
 	/*
 	 * Sometimes I/O to an encrypted file has to be broken up to guarantee
 	 * DUN contiguity.  Handle this by limiting the length of the mapping
@@ -4132,7 +4095,47 @@ int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 	return 0;
 }
 
-static DEFINE_IOMAP_ITER_NEXT(ext4_iomap_next, ext4_iomap_begin);
+static int ext4_iomap_next(const struct iomap_iter *iter, struct iomap *iomap,
+			   struct iomap *srcmap)
+{
+	int ret = iomap_iter_next(iter, iomap, srcmap, ext4_iomap_begin, NULL);
+
+	/*
+	 * For dio, increment i_conv_pending_ios the first time we see an
+	 * unwritten mapping for this IO, so that it cleanly balances with the
+	 * single endio call. We use iter->private to track state to ensure we
+	 * only increment once per dio regardless of how many extents it spans.
+	 * The decrement happens in ext4_dio_write_end_io() if
+	 * IOMAP_DIO_UNWRITTEN is set.
+	 *
+	 * Hack alert: iomap_dio_bio_iter() fails a misaligned mapping with
+	 * -EINVAL before it sets IOMAP_DIO_UNWRITTEN, so endio would skip the
+	 * decrement. Don't increment for such a mapping. The alignment check
+	 * mirrors the one in iomap_dio_bio_iter()
+	 */
+	if (ret > 0 && iter->private &&
+	    (iter->flags & IOMAP_WRITE) && (iomap->type == IOMAP_UNWRITTEN) &&
+	    IS_ALIGNED(iter->pos | iomap_length(iter),
+		       bdev_logical_block_size(iomap->bdev))) {
+		bool *inc_done = (bool *)iter->private;
+
+		if (!*inc_done) {
+			/* NOWAIT dio must not sleep on i_conv_pending_sem */
+			if (iter->flags & IOMAP_NOWAIT) {
+				int err;
+
+				err = ext4_conv_pending_inc_trylock(iter->inode);
+				if (err)
+					return err;
+			} else {
+				ext4_conv_pending_inc(iter->inode);
+			}
+			*inc_done = true;
+		}
+	}
+
+	return ret;
+}
 
 const struct iomap_ops ext4_iomap_ops = {
 	.iomap_next		= ext4_iomap_next,

-- 
2.55.0