From: Zhang Yi <yi.zhang@huawei.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@huawei.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 */
+#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.1On Fri, Nov 21, 2025 at 02:08:00PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.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@huawei.com>
Looks good, feel free to add:
Reviewed-by: Ojaswin Mujoo <ojaswin@linux.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 */
> +#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
>
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@huawei.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@huawei.com>
>
> Looks good, feel free to add:
> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.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 :)
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
> >
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@huawei.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@huawei.com>
>>
>> Looks good, feel free to add:
>> Reviewed-by: Ojaswin Mujoo <ojaswin@linux.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
>>>
© 2016 - 2025 Red Hat, Inc.