lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bq37dhlk3uxrwmejrhqkvjthlq7j3fac7dggkchpdzx7m223jl@njfevshlrkgh>
Date: Wed, 2 Jul 2025 16:50:21 +0200
From: Jan Kara <jack@...e.cz>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz, 
	ojaswin@...ux.ibm.com, sashal@...nel.org, yi.zhang@...wei.com, libaokun1@...wei.com, 
	yukuai3@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH v3 09/10] ext4: replace ext4_writepage_trans_blocks()

On Tue 01-07-25 21:06:34, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@...wei.com>
> 
> After ext4 supports large folios, the semantics of reserving credits in
> pages is no longer applicable. In most scenarios, reserving credits in
> extents is sufficient. Therefore, introduce ext4_chunk_trans_extent()
> to replace ext4_writepage_trans_blocks(). move_extent_per_page() is the
> only remaining location where we are still processing extents in pages.
> 
> Suggested-by: Jan Kara <jack@...e.cz>
> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> ---
>  fs/ext4/ext4.h        |  2 +-
>  fs/ext4/extents.c     |  6 +++---
>  fs/ext4/inline.c      |  6 +++---
>  fs/ext4/inode.c       | 33 +++++++++++++++------------------
>  fs/ext4/move_extent.c |  3 ++-
>  fs/ext4/xattr.c       |  2 +-
>  6 files changed, 25 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 18373de980f2..f705046ba6c6 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3064,9 +3064,9 @@ extern int ext4_punch_hole(struct file *file, loff_t offset, loff_t length);
>  extern void ext4_set_inode_flags(struct inode *, bool init);
>  extern int ext4_alloc_da_blocks(struct inode *inode);
>  extern void ext4_set_aops(struct inode *inode);
> -extern int ext4_writepage_trans_blocks(struct inode *);
>  extern int ext4_normal_submit_inode_data_buffers(struct jbd2_inode *jinode);
>  extern int ext4_chunk_trans_blocks(struct inode *, int nrblocks);
> +extern int ext4_chunk_trans_extent(struct inode *inode, int nrblocks);
>  extern int ext4_meta_trans_blocks(struct inode *inode, int lblocks,
>  				  int pextents);
>  extern int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index b543a46fc809..f0f155458697 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5171,7 +5171,7 @@ ext4_ext_shift_path_extents(struct ext4_ext_path *path, ext4_lblk_t shift,
>  				credits = depth + 2;
>  			}
>  
> -			restart_credits = ext4_writepage_trans_blocks(inode);
> +			restart_credits = ext4_chunk_trans_extent(inode, 0);
>  			err = ext4_datasem_ensure_credits(handle, inode, credits,
>  					restart_credits, 0);
>  			if (err) {
> @@ -5431,7 +5431,7 @@ static int ext4_collapse_range(struct file *file, loff_t offset, loff_t len)
>  
>  	truncate_pagecache(inode, start);
>  
> -	credits = ext4_writepage_trans_blocks(inode);
> +	credits = ext4_chunk_trans_extent(inode, 0);
>  	handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
>  	if (IS_ERR(handle))
>  		return PTR_ERR(handle);
> @@ -5527,7 +5527,7 @@ static int ext4_insert_range(struct file *file, loff_t offset, loff_t len)
>  
>  	truncate_pagecache(inode, start);
>  
> -	credits = ext4_writepage_trans_blocks(inode);
> +	credits = ext4_chunk_trans_extent(inode, 0);
>  	handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
>  	if (IS_ERR(handle))
>  		return PTR_ERR(handle);
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index a1bbcdf40824..d5b32d242495 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -562,7 +562,7 @@ static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
>  		return 0;
>  	}
>  
> -	needed_blocks = ext4_writepage_trans_blocks(inode);
> +	needed_blocks = ext4_chunk_trans_extent(inode, 1);
>  
>  	ret = ext4_get_inode_loc(inode, &iloc);
>  	if (ret)
> @@ -1864,7 +1864,7 @@ int ext4_inline_data_truncate(struct inode *inode, int *has_inline)
>  	};
>  
>  
> -	needed_blocks = ext4_writepage_trans_blocks(inode);
> +	needed_blocks = ext4_chunk_trans_extent(inode, 1);
>  	handle = ext4_journal_start(inode, EXT4_HT_INODE, needed_blocks);
>  	if (IS_ERR(handle))
>  		return PTR_ERR(handle);
> @@ -1979,7 +1979,7 @@ int ext4_convert_inline_data(struct inode *inode)
>  			return 0;
>  	}
>  
> -	needed_blocks = ext4_writepage_trans_blocks(inode);
> +	needed_blocks = ext4_chunk_trans_extent(inode, 1);
>  
>  	iloc.bh = NULL;
>  	error = ext4_get_inode_loc(inode, &iloc);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index ceaede80d791..572a70b6a934 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1295,7 +1295,8 @@ static int ext4_write_begin(struct file *file, struct address_space *mapping,
>  	 * Reserve one block more for addition to orphan list in case
>  	 * we allocate blocks but write fails for some reason
>  	 */
> -	needed_blocks = ext4_writepage_trans_blocks(inode) + 1;
> +	needed_blocks = ext4_chunk_trans_extent(inode,
> +			ext4_journal_blocks_per_folio(inode)) + 1;
>  	index = pos >> PAGE_SHIFT;
>  
>  	if (ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) {
> @@ -4462,7 +4463,7 @@ int ext4_punch_hole(struct file *file, loff_t offset, loff_t length)
>  		return ret;
>  
>  	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> -		credits = ext4_writepage_trans_blocks(inode);
> +		credits = ext4_chunk_trans_extent(inode, 2);
>  	else
>  		credits = ext4_blocks_for_truncate(inode);
>  	handle = ext4_journal_start(inode, EXT4_HT_TRUNCATE, credits);
> @@ -4611,7 +4612,7 @@ int ext4_truncate(struct inode *inode)
>  	}
>  
>  	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
> -		credits = ext4_writepage_trans_blocks(inode);
> +		credits = ext4_chunk_trans_extent(inode, 1);
>  	else
>  		credits = ext4_blocks_for_truncate(inode);
>  
> @@ -6238,25 +6239,19 @@ int ext4_meta_trans_blocks(struct inode *inode, int lblocks, int pextents)
>  }
>  
>  /*
> - * Calculate the total number of credits to reserve to fit
> - * the modification of a single pages into a single transaction,
> - * which may include multiple chunks of block allocations.
> - *
> - * This could be called via ext4_write_begin()
> - *
> - * We need to consider the worse case, when
> - * one new block per extent.
> + * Calculate the journal credits for modifying the number of blocks
> + * in a single extent within one transaction. 'nrblocks' is used only
> + * for non-extent inodes. For extent type inodes, 'nrblocks' can be
> + * zero if the exact number of blocks is unknown.
>   */
> -int ext4_writepage_trans_blocks(struct inode *inode)
> +int ext4_chunk_trans_extent(struct inode *inode, int nrblocks)
>  {
> -	int bpp = ext4_journal_blocks_per_folio(inode);
>  	int ret;
>  
> -	ret = ext4_meta_trans_blocks(inode, bpp, bpp);
> -
> +	ret = ext4_meta_trans_blocks(inode, nrblocks, 1);
>  	/* Account for data blocks for journalled mode */
>  	if (ext4_should_journal_data(inode))
> -		ret += bpp;
> +		ret += nrblocks;
>  	return ret;
>  }
>  
> @@ -6634,10 +6629,12 @@ static int ext4_block_page_mkwrite(struct inode *inode, struct folio *folio,
>  	handle_t *handle;
>  	loff_t size;
>  	unsigned long len;
> +	int credits;
>  	int ret;
>  
> -	handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE,
> -				    ext4_writepage_trans_blocks(inode));
> +	credits = ext4_chunk_trans_extent(inode,
> +			ext4_journal_blocks_per_folio(inode));
> +	handle = ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, credits);
>  	if (IS_ERR(handle))
>  		return PTR_ERR(handle);
>  
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 1f8493a56e8f..adae3caf175a 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -280,7 +280,8 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
>  	 */
>  again:
>  	*err = 0;
> -	jblocks = ext4_writepage_trans_blocks(orig_inode) * 2;
> +	jblocks = ext4_meta_trans_blocks(orig_inode, block_len_in_page,
> +					 block_len_in_page) * 2;
>  	handle = ext4_journal_start(orig_inode, EXT4_HT_MOVE_EXTENTS, jblocks);
>  	if (IS_ERR(handle)) {
>  		*err = PTR_ERR(handle);
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 8d15acbacc20..3fb93247330d 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -962,7 +962,7 @@ int __ext4_xattr_set_credits(struct super_block *sb, struct inode *inode,
>  	 * so we need to reserve credits for this eventuality
>  	 */
>  	if (inode && ext4_has_inline_data(inode))
> -		credits += ext4_writepage_trans_blocks(inode) + 1;
> +		credits += ext4_chunk_trans_extent(inode, 1) + 1;
>  
>  	/* We are done if ea_inode feature is not enabled. */
>  	if (!ext4_has_feature_ea_inode(sb))
> -- 
> 2.46.1
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ