[<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