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] [day] [month] [year] [list]
Message-ID: <d066aa0f-fdfc-8962-5e77-e2a98bcf6981@huawei.com>
Date: Mon, 3 Nov 2025 20:24:49 +0800
From: yangerkun <yangerkun@...wei.com>
To: Zhang Yi <yi.zhang@...weicloud.com>, <linux-ext4@...r.kernel.org>,
	<tytso@....edu>, <adilger.kernel@...ger.ca>, <jack@...e.cz>
CC: <libaokun1@...wei.com>, <yangerkun@...weicloud.com>
Subject: Re: [PATCH 2/4] ext4: rename EXT4_GET_BLOCKS_PRE_IO



在 2025/11/3 13:01, Zhang Yi 写道:
> On 10/27/2025 8:23 PM, Yang Erkun wrote:
>> This flag has been generalized to split an unwritten extent when we do
>> dio or dioread_nolock writeback, or to avoid merge new extents which was
>> created by extents split. Update some related comments too.
>>
>> Signed-off-by: Yang Erkun <yangerkun@...wei.com>
> 
> Thanks for the patch! This overall looks good to me besides one nit
> below.
> 
>> ---
>>   fs/ext4/ext4.h              | 21 +++++++++++++++------
>>   fs/ext4/extents.c           | 16 ++++++++--------
>>   include/trace/events/ext4.h |  2 +-
>>   3 files changed, 24 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 57087da6c7be..5a035d0e2761 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -694,13 +694,22 @@ enum {
>>   	/* Caller is from the delayed allocation writeout path
>>   	 * finally doing the actual allocation of delayed blocks */
>>   #define EXT4_GET_BLOCKS_DELALLOC_RESERVE	0x0004
>> -	/* caller is from the direct IO path, request to creation of an
>> -	unwritten extents if not allocated, split the unwritten
>> -	extent if blocks has been preallocated already*/
>> -#define EXT4_GET_BLOCKS_PRE_IO			0x0008
>> -#define EXT4_GET_BLOCKS_CONVERT			0x0010
>> -#define EXT4_GET_BLOCKS_IO_CREATE_EXT		(EXT4_GET_BLOCKS_PRE_IO|\
>> +	/*
>> +	 * Means we cannot merge new allocated extent or split the unwritten
>> +	 * extent if we found one
>> +	 */
> 
> There is something wrong with this comment, it should be as follows.
> 
> This means that we cannot merge newly allocated extents, and if we
> found an unwritten extent, we need to split it.

Thanks a lot for your review, you are right, the comment here is wrong!

> 
> Thanks,
> Yi.
> 
>> +#define EXT4_GET_BLOCKS_SPLIT_NOMERGE		0x0008
>> +	/*
>> +	 * Caller is from the dio or dioread_nolock buffer writeback,
>> +	 * request to creation of an unwritten extent if not exist or split
>> +	 * the found unwritten extent. Also do not merge the new create
>> +	 * unwritten extent, io end will convert unwritten to written, and
>> +	 * try to merge the written extent.
>> +	 */
>> +#define EXT4_GET_BLOCKS_IO_CREATE_EXT		(EXT4_GET_BLOCKS_SPLIT_NOMERGE|\
>>   					 EXT4_GET_BLOCKS_CREATE_UNWRIT_EXT)
>> +	/* Convert unwritten extent to initialized. */
>> +#define EXT4_GET_BLOCKS_CONVERT			0x0010
>>   	/* Eventual metadata allocation (due to growing extent tree)
>>   	 * should not fail, so try to use reserved blocks for that.*/
>>   #define EXT4_GET_BLOCKS_METADATA_NOFAIL		0x0020
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index ca5499e9412b..241b5f5d29ad 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -333,7 +333,7 @@ ext4_force_split_extent_at(handle_t *handle, struct inode *inode,
>>   			   int nofail)
>>   {
>>   	int unwritten = ext4_ext_is_unwritten(path[path->p_depth].p_ext);
>> -	int flags = EXT4_EX_NOCACHE | EXT4_GET_BLOCKS_PRE_IO;
>> +	int flags = EXT4_EX_NOCACHE | EXT4_GET_BLOCKS_SPLIT_NOMERGE;
>>   
>>   	if (nofail)
>>   		flags |= EXT4_GET_BLOCKS_METADATA_NOFAIL | EXT4_EX_NOFAIL;
>> @@ -2002,7 +2002,7 @@ ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
>>   	}
>>   
>>   	/* try to insert block into found extent and return */
>> -	if (ex && !(gb_flags & EXT4_GET_BLOCKS_PRE_IO)) {
>> +	if (ex && !(gb_flags & EXT4_GET_BLOCKS_SPLIT_NOMERGE)) {
>>   
>>   		/*
>>   		 * Try to see whether we should rather test the extent on
>> @@ -2181,7 +2181,7 @@ ext4_ext_insert_extent(handle_t *handle, struct inode *inode,
>>   
>>   merge:
>>   	/* try to merge extents */
>> -	if (!(gb_flags & EXT4_GET_BLOCKS_PRE_IO))
>> +	if (!(gb_flags & EXT4_GET_BLOCKS_SPLIT_NOMERGE))
>>   		ext4_ext_try_to_merge(handle, inode, path, nearex);
>>   
>>   	/* time to correct all indexes above */
>> @@ -3224,7 +3224,7 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>>   		else
>>   			ext4_ext_mark_initialized(ex);
>>   
>> -		if (!(flags & EXT4_GET_BLOCKS_PRE_IO))
>> +		if (!(flags & EXT4_GET_BLOCKS_SPLIT_NOMERGE))
>>   			ext4_ext_try_to_merge(handle, inode, path, ex);
>>   
>>   		err = ext4_ext_dirty(handle, inode, path + path->p_depth);
>> @@ -3368,7 +3368,7 @@ static struct ext4_ext_path *ext4_split_extent(handle_t *handle,
>>   
>>   	if (map->m_lblk + map->m_len < ee_block + ee_len) {
>>   		split_flag1 = split_flag & EXT4_EXT_MAY_ZEROOUT;
>> -		flags1 = flags | EXT4_GET_BLOCKS_PRE_IO;
>> +		flags1 = flags | EXT4_GET_BLOCKS_SPLIT_NOMERGE;
>>   		if (unwritten)
>>   			split_flag1 |= EXT4_EXT_MARK_UNWRIT1 |
>>   				       EXT4_EXT_MARK_UNWRIT2;
>> @@ -3739,7 +3739,7 @@ static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle,
>>   			      EXT4_EXT_MAY_ZEROOUT : 0;
>>   		split_flag |= (EXT4_EXT_MARK_UNWRIT2 | EXT4_EXT_DATA_VALID2);
>>   	}
>> -	flags |= EXT4_GET_BLOCKS_PRE_IO;
>> +	flags |= EXT4_GET_BLOCKS_SPLIT_NOMERGE;
>>   	return ext4_split_extent(handle, inode, path, map, split_flag, flags,
>>   				 allocated);
>>   }
>> @@ -3911,7 +3911,7 @@ ext4_ext_handle_unwritten_extents(handle_t *handle, struct inode *inode,
>>   						*allocated, newblock);
>>   
>>   	/* get_block() before submitting IO, split the extent */
>> -	if (flags & EXT4_GET_BLOCKS_PRE_IO) {
>> +	if (flags & EXT4_GET_BLOCKS_SPLIT_NOMERGE) {
>>   		path = ext4_split_convert_extents(handle, inode, map, path,
>>   				flags | EXT4_GET_BLOCKS_CONVERT, allocated);
>>   		if (IS_ERR(path))
>> @@ -5618,7 +5618,7 @@ static int ext4_insert_range(struct file *file, loff_t offset, loff_t len)
>>   			path = ext4_split_extent_at(handle, inode, path,
>>   					start_lblk, split_flag,
>>   					EXT4_EX_NOCACHE |
>> -					EXT4_GET_BLOCKS_PRE_IO |
>> +					EXT4_GET_BLOCKS_SPLIT_NOMERGE |
>>   					EXT4_GET_BLOCKS_METADATA_NOFAIL);
>>   		}
>>   
>> diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
>> index a374e7ea7e57..ada2b9223df5 100644
>> --- a/include/trace/events/ext4.h
>> +++ b/include/trace/events/ext4.h
>> @@ -39,7 +39,7 @@ struct partial_cluster;
>>   	{ EXT4_GET_BLOCKS_CREATE,		"CREATE" },		\
>>   	{ EXT4_GET_BLOCKS_UNWRIT_EXT,		"UNWRIT" },		\
>>   	{ EXT4_GET_BLOCKS_DELALLOC_RESERVE,	"DELALLOC" },		\
>> -	{ EXT4_GET_BLOCKS_PRE_IO,		"PRE_IO" },		\
>> +	{ EXT4_GET_BLOCKS_SPLIT_NOMERGE,	"SPLIT_NOMERGE" },	\
>>   	{ EXT4_GET_BLOCKS_CONVERT,		"CONVERT" },		\
>>   	{ EXT4_GET_BLOCKS_METADATA_NOFAIL,	"METADATA_NOFAIL" },	\
>>   	{ EXT4_GET_BLOCKS_NO_NORMALIZE,		"NO_NORMALIZE" },	\
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ