[Devel] [PATCH VZ9 1/2] fs/ext4: relax inappropriate memory alignment check

Alexey Kuznetsov kuznet at virtuozzo.com
Tue Feb 27 19:58:19 MSK 2024


The bug is ancient, ext4 has been broken in mainstream since 2014,
I guess this means nobody cares. Though right place of this
patch is mainstream of course.

It destroys performance of direct io in vz9 compared to vz7, where
this place was still correct. Note, our memory buffers are aligned
to 512 and we physically cannot get it even more coarse
and there are exactly no reasons it would be not enough. Well,
except for stupid bugs like this.

Comment from the patch:

/* Memory alignment has nothing to do with io alignment. All the goal
 * of ext4_unaligned_io() is to check when direct io requires some
 * fuss about unaligned modifications to ext4 file layout. Memorywise,
 * iomap handles all the required dma alignment troubles. So,
 * this check is entirely wrong and I can guess it was a copy-n-paste
 * error in original patch 9b884164.. "convert ext4 to ->write_iter()."
 * Yet, just to be on safe side let's leave minimal test for alignment
 * to 512 as bdev level used not to handle this well and 512
 * is old good traditional limit.
 */

https://pmc.acronis.work/browse/VSTOR-79527

Signed-off-by: Alexey Kuznetsov <kuznet at virtuozzo.com>
---
 fs/ext4/file.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 579efd9..cebaa31 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -63,6 +63,9 @@ static bool ext4_should_use_dio(struct kiocb *iocb, struct iov_iter *iter)
 	if (dio_align == 1)
 		return true;
 
+	/* Not reached. So, line below is meaningless */
+	WARN_ON_ONCE(1);
+
 	return IS_ALIGNED(iocb->ki_pos | iov_iter_alignment(iter), dio_align);
 }
 
@@ -187,7 +190,20 @@ static int ext4_release_file(struct inode *inode, struct file *filp)
 	struct super_block *sb = inode->i_sb;
 	unsigned long blockmask = sb->s_blocksize - 1;
 
-	if ((pos | iov_iter_alignment(from)) & blockmask)
+	if (pos & blockmask)
+		return true;
+
+	/* Memory alignment has nothing to do with io alignment. All the goal
+	 * of ext4_unaligned_io() is to check when direct io requires some
+	 * fuss about unaligned modifications to ext4 file layout. Memorywise,
+	 * iomap handles all the required dma alignment troubles. So,
+	 * this check is entirely wrong and I can guess it was a copy-n-paste
+	 * error in original patch 9b884164.. "convert ext4 to ->write_iter()."
+	 * Yet, just to be on safe side let's leave minimal test for alignment
+	 * to 512 as bdev level used not to handle this well and 512
+	 * is old good traditional limit.
+	 */
+	if (iov_iter_alignment(from) & 511)
 		return true;
 
 	return false;
-- 
1.8.3.1



More information about the Devel mailing list