linux-kernel - Re: [PATCH v2 02/13] ext4: subdivide EXT4_EXT_DATA

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: <67f710d0-2ea3-425a-b604-651bf1009e75@huaweicloud.com>
Date: Thu, 27 Nov 2025 14:09:17 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Ojaswin Mujoo <ojaswin@...ux.ibm.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, yi.zhang@...wei.com, yizhang089@...il.com,
 libaokun1@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH v2 02/13] ext4: subdivide EXT4_EXT_DATA_VALID1

Hi, thank you again for reviewing this series!

On 11/26/2025 7:55 PM, Ojaswin Mujoo wrote:
> On Wed, Nov 26, 2025 at 04:57:27PM +0530, Ojaswin Mujoo wrote:
>> On Fri, Nov 21, 2025 at 02:08:00PM +0800, Zhang Yi wrote:
>>> From: Zhang Yi <yi.zhang@...wei.com>
>>>
>>> When splitting an extent, if the EXT4_GET_BLOCKS_CONVERT flag is set and
>>> it is necessary to split the target extent in the middle,
>>> ext4_split_extent() first handles splitting the latter half of the
>>> extent and passes the EXT4_EXT_DATA_VALID1 flag. This flag implies that
>>> all blocks before the split point contain valid data; however, this
>>> assumption is incorrect.
>>>
>>> Therefore, subdivid EXT4_EXT_DATA_VALID1 into
>>> EXT4_EXT_DATA_ENTIRE_VALID1 and EXT4_EXT_DATA_PARTIAL_VALID1, which
>>> indicate that the first half of the extent is either entirely valid or
>>> only partially valid, respectively. These two flags cannot be set
>>> simultaneously.
>>>
>>> This patch does not use EXT4_EXT_DATA_PARTIAL_VALID1, it only replaces
>>> EXT4_EXT_DATA_VALID1 with EXT4_EXT_DATA_ENTIRE_VALID1 at the location
>>> where it is set, no logical changes.
>>>
>>> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
>>
>> Looks good, feel free to add:
>> Reviewed-by: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
>>
>>> ---
>>>  fs/ext4/extents.c | 18 ++++++++++++------
>>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>>> index 91682966597d..f7aa497e5d6c 100644
>>> --- a/fs/ext4/extents.c
>>> +++ b/fs/ext4/extents.c
>>> @@ -43,8 +43,13 @@
>>>  #define EXT4_EXT_MARK_UNWRIT1	0x2  /* mark first half unwritten */
>>>  #define EXT4_EXT_MARK_UNWRIT2	0x4  /* mark second half unwritten */
>>>  
>>> -#define EXT4_EXT_DATA_VALID1	0x8  /* first half contains valid data */
>>> -#define EXT4_EXT_DATA_VALID2	0x10 /* second half contains valid data */
>>> +/* first half contains valid data */
>>> +#define EXT4_EXT_DATA_ENTIRE_VALID1	0x8   /* has partially valid data */
>>> +#define EXT4_EXT_DATA_PARTIAL_VALID1	0x10  /* has entirely valid data */
> 
> Hey, sorry I forgot to mention this minor typo in my last email. The
> comment for partial and entirely valid flags are mismatched :)

Ha, right, I missed that, will fix in next iteration.

Thanks,
Yi.

> 
> Regards,
> ojaswin
> 
>>> +#define EXT4_EXT_DATA_VALID1		(EXT4_EXT_DATA_ENTIRE_VALID1 | \
>>> +					 EXT4_EXT_DATA_PARTIAL_VALID1)
>>> +
>>> +#define EXT4_EXT_DATA_VALID2	0x20 /* second half contains valid data */
>>>  
>>>  static __le32 ext4_extent_block_csum(struct inode *inode,
>>>  				     struct ext4_extent_header *eh)
>>> @@ -3190,8 +3195,9 @@ static struct ext4_ext_path *ext4_split_extent_at(handle_t *handle,
>>>  	unsigned int ee_len, depth;
>>>  	int err = 0;
>>>  
>>> -	BUG_ON((split_flag & (EXT4_EXT_DATA_VALID1 | EXT4_EXT_DATA_VALID2)) ==
>>> -	       (EXT4_EXT_DATA_VALID1 | EXT4_EXT_DATA_VALID2));
>>> +	BUG_ON((split_flag & EXT4_EXT_DATA_VALID1) == EXT4_EXT_DATA_VALID1);
>>> +	BUG_ON((split_flag & EXT4_EXT_DATA_VALID1) &&
>>> +	       (split_flag & EXT4_EXT_DATA_VALID2));
>>>  
>>>  	ext_debug(inode, "logical block %llu\n", (unsigned long long)split);
>>>  
>>> @@ -3358,7 +3364,7 @@ static struct ext4_ext_path *ext4_split_extent(handle_t *handle,
>>>  			split_flag1 |= EXT4_EXT_MARK_UNWRIT1 |
>>>  				       EXT4_EXT_MARK_UNWRIT2;
>>>  		if (split_flag & EXT4_EXT_DATA_VALID2)
>>> -			split_flag1 |= EXT4_EXT_DATA_VALID1;
>>> +			split_flag1 |= EXT4_EXT_DATA_ENTIRE_VALID1;
>>>  		path = ext4_split_extent_at(handle, inode, path,
>>>  				map->m_lblk + map->m_len, split_flag1, flags1);
>>>  		if (IS_ERR(path))
>>> @@ -3717,7 +3723,7 @@ static struct ext4_ext_path *ext4_split_convert_extents(handle_t *handle,
>>>  
>>>  	/* Convert to unwritten */
>>>  	if (flags & EXT4_GET_BLOCKS_CONVERT_UNWRITTEN) {
>>> -		split_flag |= EXT4_EXT_DATA_VALID1;
>>> +		split_flag |= EXT4_EXT_DATA_ENTIRE_VALID1;
>>>  	/* Convert to initialized */
>>>  	} else if (flags & EXT4_GET_BLOCKS_CONVERT) {
>>>  		split_flag |= ee_block + ee_len <= eof_block ?
>>> -- 
>>> 2.46.1
>>>