diff mbox series

[2/2] ocfs2: Do not zero pages beyond i_size

Message ID 20211025151332.11301-2-jack@suse.cz (mailing list archive)
State New, archived
Headers show
Series ocfs2: Truncate data corruption fix | expand

Commit Message

Jan Kara Oct. 25, 2021, 3:13 p.m. UTC
ocfs2_zero_range_for_truncate() can try to zero pages beyond current
inode size despite the fact that underlying blocks should be already
zeroed out and writeback will skip writing such pages anyway. Avoid the
pointless work.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ocfs2/alloc.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Joseph Qi Nov. 2, 2021, 2:58 a.m. UTC | #1
On 10/25/21 11:13 PM, Jan Kara wrote:
> ocfs2_zero_range_for_truncate() can try to zero pages beyond current
> inode size despite the fact that underlying blocks should be already
> zeroed out and writeback will skip writing such pages anyway. Avoid the
> pointless work.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Looks good.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/alloc.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 5d9ae17bd443..ff5ff0e7de44 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -6921,13 +6921,12 @@ static int ocfs2_grab_eof_pages(struct inode *inode, loff_t start, loff_t end,
>  }
>  
>  /*
> - * Zero the area past i_size but still within an allocated
> - * cluster. This avoids exposing nonzero data on subsequent file
> - * extends.
> + * Zero partial cluster for a hole punch or truncate. This avoids exposing
> + * nonzero data on subsequent file extends.
>   *
>   * We need to call this before i_size is updated on the inode because
>   * otherwise block_write_full_page() will skip writeout of pages past
> - * i_size. The new_i_size parameter is passed for this reason.
> + * i_size.
>   */
>  int ocfs2_zero_range_for_truncate(struct inode *inode, handle_t *handle,
>  				  u64 range_start, u64 range_end)
> @@ -6945,6 +6944,15 @@ int ocfs2_zero_range_for_truncate(struct inode *inode, handle_t *handle,
>  	if (!ocfs2_sparse_alloc(OCFS2_SB(sb)))
>  		return 0;
>  
> +	/*
> +	 * Avoid zeroing pages fully beyond current i_size. It is pointless as
> +	 * underlying blocks of those pages should be already zeroed out and
> +	 * page writeback will skip them anyway.
> +	 */
> +	range_end = min_t(u64, range_end, i_size_read(inode));
> +	if (range_start >= range_end)
> +		return 0;
> +
>  	pages = kcalloc(ocfs2_pages_per_cluster(sb),
>  			sizeof(struct page *), GFP_NOFS);
>  	if (pages == NULL) {
> @@ -6953,9 +6961,6 @@ int ocfs2_zero_range_for_truncate(struct inode *inode, handle_t *handle,
>  		goto out;
>  	}
>  
> -	if (range_start == range_end)
> -		goto out;
> -
>  	ret = ocfs2_extent_map_get_blocks(inode,
>  					  range_start >> sb->s_blocksize_bits,
>  					  &phys, NULL, &ext_flags);
>
diff mbox series

Patch

diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 5d9ae17bd443..ff5ff0e7de44 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -6921,13 +6921,12 @@  static int ocfs2_grab_eof_pages(struct inode *inode, loff_t start, loff_t end,
 }
 
 /*
- * Zero the area past i_size but still within an allocated
- * cluster. This avoids exposing nonzero data on subsequent file
- * extends.
+ * Zero partial cluster for a hole punch or truncate. This avoids exposing
+ * nonzero data on subsequent file extends.
  *
  * We need to call this before i_size is updated on the inode because
  * otherwise block_write_full_page() will skip writeout of pages past
- * i_size. The new_i_size parameter is passed for this reason.
+ * i_size.
  */
 int ocfs2_zero_range_for_truncate(struct inode *inode, handle_t *handle,
 				  u64 range_start, u64 range_end)
@@ -6945,6 +6944,15 @@  int ocfs2_zero_range_for_truncate(struct inode *inode, handle_t *handle,
 	if (!ocfs2_sparse_alloc(OCFS2_SB(sb)))
 		return 0;
 
+	/*
+	 * Avoid zeroing pages fully beyond current i_size. It is pointless as
+	 * underlying blocks of those pages should be already zeroed out and
+	 * page writeback will skip them anyway.
+	 */
+	range_end = min_t(u64, range_end, i_size_read(inode));
+	if (range_start >= range_end)
+		return 0;
+
 	pages = kcalloc(ocfs2_pages_per_cluster(sb),
 			sizeof(struct page *), GFP_NOFS);
 	if (pages == NULL) {
@@ -6953,9 +6961,6 @@  int ocfs2_zero_range_for_truncate(struct inode *inode, handle_t *handle,
 		goto out;
 	}
 
-	if (range_start == range_end)
-		goto out;
-
 	ret = ocfs2_extent_map_get_blocks(inode,
 					  range_start >> sb->s_blocksize_bits,
 					  &phys, NULL, &ext_flags);