[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b3ee16de-e04b-4e95-a839-f1563b292bfa@huaweicloud.com>
Date: Mon, 3 Nov 2025 13:01:34 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Yang Erkun <yangerkun@...wei.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
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,
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