v2->v3:
- Rename ext4_ext_determine_hole() to ext4_ext_determine_insert_hole()
and keep setting of 'map' inside ext4_ext_map_blocks().
- Don't set EXT4_MAP_DELAYED in ext4_ext_determine_insert_hole()
because it's unreliable, and revise the comments.
v1->v2:
- Fix a long standing race issue between determine hole and inserting
new delalloc extent analyzed by Jan Kara.
- Change method of adjusting hole length, instead of skip holes in
ext4_map_blocks(), now we find delalloc and correct length and type
in ext4_ext_determine_hole().
v2: https://lore.kernel.org/linux-ext4/[email protected]/
v1: https://lore.kernel.org/linux-ext4/[email protected]/
Hello, all!
I'm working on switching ext4 buffer IO from buffer_head to iomap
and enable large folio on regular file recently [1], this patch set
is one of a preparation of this work. It first fix a long standing race
issue between bmap querying and adding new delalloc extents, then
correct the hole length returned by ext4_map_blocks() when user querying
map type and blocks range, after that, make this function and
ext4_set_iomap() are able to distinguish delayed allocated only mapping
from hole, finally BTW cleanup the ext4_iomap_begin_report().
This preparation patch set changes the ext4 map -> iomap converting logic
in ext4_set_iomap(), so that the later buffer IO conversion can use this
helper to connect iomap frame. This patch set is already passed
'kvm-xfstests -g auto' tests.
Thanks,
Yi.
[1] https://lore.kernel.org/linux-ext4/[email protected]/
Zhang Yi (6):
ext4: refactor ext4_da_map_blocks()
ext4: convert to exclusive lock while inserting delalloc extents
ext4: correct the hole length returned by ext4_map_blocks()
ext4: add a hole extent entry in cache after punch
ext4: make ext4_map_blocks() distinguish delalloc only extent
ext4: make ext4_set_iomap() recognize IOMAP_DELALLOC map type
fs/ext4/ext4.h | 4 +-
fs/ext4/extents.c | 114 +++++++++++++++++++++++++++++-----------------
fs/ext4/inode.c | 84 +++++++++++-----------------------
3 files changed, 103 insertions(+), 99 deletions(-)
--
2.39.2
From: Zhang Yi <[email protected]>
Since ext4_map_blocks() can recognize a delayed allocated only extent,
make ext4_set_iomap() can also recognize it, and remove the useless
separate check in ext4_iomap_begin_report().
Signed-off-by: Zhang Yi <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
---
fs/ext4/inode.c | 32 +++-----------------------------
1 file changed, 3 insertions(+), 29 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c141bf6d8db2..0458d7f0c059 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3261,6 +3261,9 @@ static void ext4_set_iomap(struct inode *inode, struct iomap *iomap,
iomap->addr = (u64) map->m_pblk << blkbits;
if (flags & IOMAP_DAX)
iomap->addr += EXT4_SB(inode->i_sb)->s_dax_part_off;
+ } else if (map->m_flags & EXT4_MAP_DELAYED) {
+ iomap->type = IOMAP_DELALLOC;
+ iomap->addr = IOMAP_NULL_ADDR;
} else {
iomap->type = IOMAP_HOLE;
iomap->addr = IOMAP_NULL_ADDR;
@@ -3423,35 +3426,11 @@ const struct iomap_ops ext4_iomap_overwrite_ops = {
.iomap_end = ext4_iomap_end,
};
-static bool ext4_iomap_is_delalloc(struct inode *inode,
- struct ext4_map_blocks *map)
-{
- struct extent_status es;
- ext4_lblk_t offset = 0, end = map->m_lblk + map->m_len - 1;
-
- ext4_es_find_extent_range(inode, &ext4_es_is_delayed,
- map->m_lblk, end, &es);
-
- if (!es.es_len || es.es_lblk > end)
- return false;
-
- if (es.es_lblk > map->m_lblk) {
- map->m_len = es.es_lblk - map->m_lblk;
- return false;
- }
-
- offset = map->m_lblk - es.es_lblk;
- map->m_len = es.es_len - offset;
-
- return true;
-}
-
static int ext4_iomap_begin_report(struct inode *inode, loff_t offset,
loff_t length, unsigned int flags,
struct iomap *iomap, struct iomap *srcmap)
{
int ret;
- bool delalloc = false;
struct ext4_map_blocks map;
u8 blkbits = inode->i_blkbits;
@@ -3492,13 +3471,8 @@ static int ext4_iomap_begin_report(struct inode *inode, loff_t offset,
ret = ext4_map_blocks(NULL, inode, &map, 0);
if (ret < 0)
return ret;
- if (ret == 0)
- delalloc = ext4_iomap_is_delalloc(inode, &map);
-
set_iomap:
ext4_set_iomap(inode, iomap, &map, offset, length, flags);
- if (delalloc && iomap->type == IOMAP_HOLE)
- iomap->type = IOMAP_DELALLOC;
return 0;
}
--
2.39.2
From: Zhang Yi <[email protected]>
In ext4_map_blocks(), if we can't find a range of mapping in the
extents cache, we are calling ext4_ext_map_blocks() to search the real
path and ext4_ext_determine_hole() to determine the hole range. But if
the querying range was partially or completely overlaped by a delalloc
extent, we can't find it in the real extent path, so the returned hole
length could be incorrect.
Fortunately, ext4_ext_put_gap_in_cache() have already handle delalloc
extent, but it searches start from the expanded hole_start, doesn't
start from the querying range, so the delalloc extent found could not be
the one that overlaped the querying range, plus, it also didn't adjust
the hole length. Let's just remove ext4_ext_put_gap_in_cache(), handle
delalloc and insert adjusted hole extent in ext4_ext_determine_hole().
Signed-off-by: Zhang Yi <[email protected]>
Suggested-by: Jan Kara <[email protected]>
---
fs/ext4/extents.c | 111 +++++++++++++++++++++++++++++-----------------
1 file changed, 70 insertions(+), 41 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index d5efe076d3d3..e0b7e48c4c67 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2229,7 +2229,7 @@ static int ext4_fill_es_cache_info(struct inode *inode,
/*
- * ext4_ext_determine_hole - determine hole around given block
+ * ext4_ext_find_hole - find hole around given block according to the given path
* @inode: inode we lookup in
* @path: path in extent tree to @lblk
* @lblk: pointer to logical block around which we want to determine hole
@@ -2241,9 +2241,9 @@ static int ext4_fill_es_cache_info(struct inode *inode,
* The function returns the length of a hole starting at @lblk. We update @lblk
* to the beginning of the hole if we managed to find it.
*/
-static ext4_lblk_t ext4_ext_determine_hole(struct inode *inode,
- struct ext4_ext_path *path,
- ext4_lblk_t *lblk)
+static ext4_lblk_t ext4_ext_find_hole(struct inode *inode,
+ struct ext4_ext_path *path,
+ ext4_lblk_t *lblk)
{
int depth = ext_depth(inode);
struct ext4_extent *ex;
@@ -2270,30 +2270,6 @@ static ext4_lblk_t ext4_ext_determine_hole(struct inode *inode,
return len;
}
-/*
- * ext4_ext_put_gap_in_cache:
- * calculate boundaries of the gap that the requested block fits into
- * and cache this gap
- */
-static void
-ext4_ext_put_gap_in_cache(struct inode *inode, ext4_lblk_t hole_start,
- ext4_lblk_t hole_len)
-{
- struct extent_status es;
-
- ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start,
- hole_start + hole_len - 1, &es);
- if (es.es_len) {
- /* There's delayed extent containing lblock? */
- if (es.es_lblk <= hole_start)
- return;
- hole_len = min(es.es_lblk - hole_start, hole_len);
- }
- ext_debug(inode, " -> %u:%u\n", hole_start, hole_len);
- ext4_es_insert_extent(inode, hole_start, hole_len, ~0,
- EXTENT_STATUS_HOLE);
-}
-
/*
* ext4_ext_rm_idx:
* removes index from the index block.
@@ -4062,6 +4038,69 @@ static int get_implied_cluster_alloc(struct super_block *sb,
return 0;
}
+/*
+ * Determine hole length around the given logical block, first try to
+ * locate and expand the hole from the given @path, and then adjust it
+ * if it's partially or completely converted to delayed extents, insert
+ * it into the extent cache tree if it's indeed a hole, finally return
+ * the length of the determined extent.
+ */
+static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
+ struct ext4_ext_path *path,
+ ext4_lblk_t lblk)
+{
+ ext4_lblk_t hole_start, len;
+ struct extent_status es;
+
+ hole_start = lblk;
+ len = ext4_ext_find_hole(inode, path, &hole_start);
+again:
+ ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start,
+ hole_start + len - 1, &es);
+ if (!es.es_len)
+ goto insert_hole;
+
+ /*
+ * There's a delalloc extent in the hole, handle it if the delalloc
+ * extent is in front of, behind and straddle the queried range.
+ */
+ if (lblk >= es.es_lblk + es.es_len) {
+ /*
+ * The delalloc extent is in front of the queried range,
+ * find again from the queried start block.
+ */
+ len -= lblk - hole_start;
+ hole_start = lblk;
+ goto again;
+ } else if (in_range(lblk, es.es_lblk, es.es_len)) {
+ /*
+ * The delalloc extent containing lblk, it must have been
+ * added after ext4_map_blocks() checked the extent status
+ * tree, adjust the length to the delalloc extent's after
+ * lblk.
+ */
+ len = es.es_lblk + es.es_len - lblk;
+ return len;
+ } else {
+ /*
+ * The delalloc extent is partially or completely behind
+ * the queried range, update hole length until the
+ * beginning of the delalloc extent.
+ */
+ len = min(es.es_lblk - hole_start, len);
+ }
+
+insert_hole:
+ /* Put just found gap into cache to speed up subsequent requests */
+ ext_debug(inode, " -> %u:%u\n", hole_start, len);
+ ext4_es_insert_extent(inode, hole_start, len, ~0, EXTENT_STATUS_HOLE);
+
+ /* Update hole_len to reflect hole size after lblk */
+ if (hole_start != lblk)
+ len -= lblk - hole_start;
+
+ return len;
+}
/*
* Block allocation/map/preallocation routine for extents based files
@@ -4179,22 +4218,12 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
* we couldn't try to create block if create flag is zero
*/
if ((flags & EXT4_GET_BLOCKS_CREATE) == 0) {
- ext4_lblk_t hole_start, hole_len;
+ ext4_lblk_t len;
- hole_start = map->m_lblk;
- hole_len = ext4_ext_determine_hole(inode, path, &hole_start);
- /*
- * put just found gap into cache to speed up
- * subsequent requests
- */
- ext4_ext_put_gap_in_cache(inode, hole_start, hole_len);
+ len = ext4_ext_determine_insert_hole(inode, path, map->m_lblk);
- /* Update hole_len to reflect hole size after map->m_lblk */
- if (hole_start != map->m_lblk)
- hole_len -= map->m_lblk - hole_start;
map->m_pblk = 0;
- map->m_len = min_t(unsigned int, map->m_len, hole_len);
-
+ map->m_len = min_t(unsigned int, map->m_len, len);
goto out;
}
--
2.39.2
From: Zhang Yi <[email protected]>
Add a new map flag EXT4_MAP_DELAYED to indicate the mapping range is a
delayed allocated only (not unwritten) one, and making
ext4_map_blocks() can distinguish it, no longer mixing it with holes.
Signed-off-by: Zhang Yi <[email protected]>
---
fs/ext4/ext4.h | 4 +++-
fs/ext4/extents.c | 7 +++++--
fs/ext4/inode.c | 2 ++
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index a5d784872303..55195909d32f 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -252,8 +252,10 @@ struct ext4_allocation_request {
#define EXT4_MAP_MAPPED BIT(BH_Mapped)
#define EXT4_MAP_UNWRITTEN BIT(BH_Unwritten)
#define EXT4_MAP_BOUNDARY BIT(BH_Boundary)
+#define EXT4_MAP_DELAYED BIT(BH_Delay)
#define EXT4_MAP_FLAGS (EXT4_MAP_NEW | EXT4_MAP_MAPPED |\
- EXT4_MAP_UNWRITTEN | EXT4_MAP_BOUNDARY)
+ EXT4_MAP_UNWRITTEN | EXT4_MAP_BOUNDARY |\
+ EXT4_MAP_DELAYED)
struct ext4_map_blocks {
ext4_fsblk_t m_pblk;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index e0b7e48c4c67..6b64319a7df8 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -4076,8 +4076,11 @@ static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
/*
* The delalloc extent containing lblk, it must have been
* added after ext4_map_blocks() checked the extent status
- * tree, adjust the length to the delalloc extent's after
- * lblk.
+ * tree so we are not holding i_rwsem and delalloc info is
+ * only stabilized by i_data_sem we are going to release
+ * soon. Don't modify the extent status tree and report
+ * extent as a hole, just adjust the length to the delalloc
+ * extent's after lblk.
*/
len = es.es_lblk + es.es_len - lblk;
return len;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1b5e6409f958..c141bf6d8db2 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -515,6 +515,8 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
map->m_len = retval;
} else if (ext4_es_is_delayed(&es) || ext4_es_is_hole(&es)) {
map->m_pblk = 0;
+ map->m_flags |= ext4_es_is_delayed(&es) ?
+ EXT4_MAP_DELAYED : 0;
retval = es.es_len - (map->m_lblk - es.es_lblk);
if (retval > map->m_len)
retval = map->m_len;
--
2.39.2
On Fri 05-01-24 11:30:15, Zhang Yi wrote:
> From: Zhang Yi <[email protected]>
>
> In ext4_map_blocks(), if we can't find a range of mapping in the
> extents cache, we are calling ext4_ext_map_blocks() to search the real
> path and ext4_ext_determine_hole() to determine the hole range. But if
> the querying range was partially or completely overlaped by a delalloc
> extent, we can't find it in the real extent path, so the returned hole
> length could be incorrect.
>
> Fortunately, ext4_ext_put_gap_in_cache() have already handle delalloc
> extent, but it searches start from the expanded hole_start, doesn't
> start from the querying range, so the delalloc extent found could not be
> the one that overlaped the querying range, plus, it also didn't adjust
> the hole length. Let's just remove ext4_ext_put_gap_in_cache(), handle
> delalloc and insert adjusted hole extent in ext4_ext_determine_hole().
>
> Signed-off-by: Zhang Yi <[email protected]>
> Suggested-by: Jan Kara <[email protected]>
Thanks! Looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/ext4/extents.c | 111 +++++++++++++++++++++++++++++-----------------
> 1 file changed, 70 insertions(+), 41 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index d5efe076d3d3..e0b7e48c4c67 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2229,7 +2229,7 @@ static int ext4_fill_es_cache_info(struct inode *inode,
>
>
> /*
> - * ext4_ext_determine_hole - determine hole around given block
> + * ext4_ext_find_hole - find hole around given block according to the given path
> * @inode: inode we lookup in
> * @path: path in extent tree to @lblk
> * @lblk: pointer to logical block around which we want to determine hole
> @@ -2241,9 +2241,9 @@ static int ext4_fill_es_cache_info(struct inode *inode,
> * The function returns the length of a hole starting at @lblk. We update @lblk
> * to the beginning of the hole if we managed to find it.
> */
> -static ext4_lblk_t ext4_ext_determine_hole(struct inode *inode,
> - struct ext4_ext_path *path,
> - ext4_lblk_t *lblk)
> +static ext4_lblk_t ext4_ext_find_hole(struct inode *inode,
> + struct ext4_ext_path *path,
> + ext4_lblk_t *lblk)
> {
> int depth = ext_depth(inode);
> struct ext4_extent *ex;
> @@ -2270,30 +2270,6 @@ static ext4_lblk_t ext4_ext_determine_hole(struct inode *inode,
> return len;
> }
>
> -/*
> - * ext4_ext_put_gap_in_cache:
> - * calculate boundaries of the gap that the requested block fits into
> - * and cache this gap
> - */
> -static void
> -ext4_ext_put_gap_in_cache(struct inode *inode, ext4_lblk_t hole_start,
> - ext4_lblk_t hole_len)
> -{
> - struct extent_status es;
> -
> - ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start,
> - hole_start + hole_len - 1, &es);
> - if (es.es_len) {
> - /* There's delayed extent containing lblock? */
> - if (es.es_lblk <= hole_start)
> - return;
> - hole_len = min(es.es_lblk - hole_start, hole_len);
> - }
> - ext_debug(inode, " -> %u:%u\n", hole_start, hole_len);
> - ext4_es_insert_extent(inode, hole_start, hole_len, ~0,
> - EXTENT_STATUS_HOLE);
> -}
> -
> /*
> * ext4_ext_rm_idx:
> * removes index from the index block.
> @@ -4062,6 +4038,69 @@ static int get_implied_cluster_alloc(struct super_block *sb,
> return 0;
> }
>
> +/*
> + * Determine hole length around the given logical block, first try to
> + * locate and expand the hole from the given @path, and then adjust it
> + * if it's partially or completely converted to delayed extents, insert
> + * it into the extent cache tree if it's indeed a hole, finally return
> + * the length of the determined extent.
> + */
> +static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
> + struct ext4_ext_path *path,
> + ext4_lblk_t lblk)
> +{
> + ext4_lblk_t hole_start, len;
> + struct extent_status es;
> +
> + hole_start = lblk;
> + len = ext4_ext_find_hole(inode, path, &hole_start);
> +again:
> + ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start,
> + hole_start + len - 1, &es);
> + if (!es.es_len)
> + goto insert_hole;
> +
> + /*
> + * There's a delalloc extent in the hole, handle it if the delalloc
> + * extent is in front of, behind and straddle the queried range.
> + */
> + if (lblk >= es.es_lblk + es.es_len) {
> + /*
> + * The delalloc extent is in front of the queried range,
> + * find again from the queried start block.
> + */
> + len -= lblk - hole_start;
> + hole_start = lblk;
> + goto again;
> + } else if (in_range(lblk, es.es_lblk, es.es_len)) {
> + /*
> + * The delalloc extent containing lblk, it must have been
> + * added after ext4_map_blocks() checked the extent status
> + * tree, adjust the length to the delalloc extent's after
> + * lblk.
> + */
> + len = es.es_lblk + es.es_len - lblk;
> + return len;
> + } else {
> + /*
> + * The delalloc extent is partially or completely behind
> + * the queried range, update hole length until the
> + * beginning of the delalloc extent.
> + */
> + len = min(es.es_lblk - hole_start, len);
> + }
> +
> +insert_hole:
> + /* Put just found gap into cache to speed up subsequent requests */
> + ext_debug(inode, " -> %u:%u\n", hole_start, len);
> + ext4_es_insert_extent(inode, hole_start, len, ~0, EXTENT_STATUS_HOLE);
> +
> + /* Update hole_len to reflect hole size after lblk */
> + if (hole_start != lblk)
> + len -= lblk - hole_start;
> +
> + return len;
> +}
>
> /*
> * Block allocation/map/preallocation routine for extents based files
> @@ -4179,22 +4218,12 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
> * we couldn't try to create block if create flag is zero
> */
> if ((flags & EXT4_GET_BLOCKS_CREATE) == 0) {
> - ext4_lblk_t hole_start, hole_len;
> + ext4_lblk_t len;
>
> - hole_start = map->m_lblk;
> - hole_len = ext4_ext_determine_hole(inode, path, &hole_start);
> - /*
> - * put just found gap into cache to speed up
> - * subsequent requests
> - */
> - ext4_ext_put_gap_in_cache(inode, hole_start, hole_len);
> + len = ext4_ext_determine_insert_hole(inode, path, map->m_lblk);
>
> - /* Update hole_len to reflect hole size after map->m_lblk */
> - if (hole_start != map->m_lblk)
> - hole_len -= map->m_lblk - hole_start;
> map->m_pblk = 0;
> - map->m_len = min_t(unsigned int, map->m_len, hole_len);
> -
> + map->m_len = min_t(unsigned int, map->m_len, len);
> goto out;
> }
>
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Fri 05-01-24 11:30:17, Zhang Yi wrote:
> From: Zhang Yi <[email protected]>
>
> Add a new map flag EXT4_MAP_DELAYED to indicate the mapping range is a
> delayed allocated only (not unwritten) one, and making
> ext4_map_blocks() can distinguish it, no longer mixing it with holes.
>
> Signed-off-by: Zhang Yi <[email protected]>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> ---
> fs/ext4/ext4.h | 4 +++-
> fs/ext4/extents.c | 7 +++++--
> fs/ext4/inode.c | 2 ++
> 3 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index a5d784872303..55195909d32f 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -252,8 +252,10 @@ struct ext4_allocation_request {
> #define EXT4_MAP_MAPPED BIT(BH_Mapped)
> #define EXT4_MAP_UNWRITTEN BIT(BH_Unwritten)
> #define EXT4_MAP_BOUNDARY BIT(BH_Boundary)
> +#define EXT4_MAP_DELAYED BIT(BH_Delay)
> #define EXT4_MAP_FLAGS (EXT4_MAP_NEW | EXT4_MAP_MAPPED |\
> - EXT4_MAP_UNWRITTEN | EXT4_MAP_BOUNDARY)
> + EXT4_MAP_UNWRITTEN | EXT4_MAP_BOUNDARY |\
> + EXT4_MAP_DELAYED)
>
> struct ext4_map_blocks {
> ext4_fsblk_t m_pblk;
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index e0b7e48c4c67..6b64319a7df8 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -4076,8 +4076,11 @@ static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
> /*
> * The delalloc extent containing lblk, it must have been
> * added after ext4_map_blocks() checked the extent status
> - * tree, adjust the length to the delalloc extent's after
> - * lblk.
> + * tree so we are not holding i_rwsem and delalloc info is
> + * only stabilized by i_data_sem we are going to release
> + * soon. Don't modify the extent status tree and report
> + * extent as a hole, just adjust the length to the delalloc
> + * extent's after lblk.
> */
> len = es.es_lblk + es.es_len - lblk;
> return len;
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 1b5e6409f958..c141bf6d8db2 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -515,6 +515,8 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
> map->m_len = retval;
> } else if (ext4_es_is_delayed(&es) || ext4_es_is_hole(&es)) {
> map->m_pblk = 0;
> + map->m_flags |= ext4_es_is_delayed(&es) ?
> + EXT4_MAP_DELAYED : 0;
> retval = es.es_len - (map->m_lblk - es.es_lblk);
> if (retval > map->m_len)
> retval = map->m_len;
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On 2024/1/5 18:17, Jan Kara wrote:
> On Fri 05-01-24 11:30:15, Zhang Yi wrote:
>> From: Zhang Yi <[email protected]>
>>
>> In ext4_map_blocks(), if we can't find a range of mapping in the
>> extents cache, we are calling ext4_ext_map_blocks() to search the real
>> path and ext4_ext_determine_hole() to determine the hole range. But if
>> the querying range was partially or completely overlaped by a delalloc
>> extent, we can't find it in the real extent path, so the returned hole
>> length could be incorrect.
>>
>> Fortunately, ext4_ext_put_gap_in_cache() have already handle delalloc
>> extent, but it searches start from the expanded hole_start, doesn't
>> start from the querying range, so the delalloc extent found could not be
>> the one that overlaped the querying range, plus, it also didn't adjust
>> the hole length. Let's just remove ext4_ext_put_gap_in_cache(), handle
>> delalloc and insert adjusted hole extent in ext4_ext_determine_hole().
>>
>> Signed-off-by: Zhang Yi <[email protected]>
>> Suggested-by: Jan Kara <[email protected]>
>
> Thanks! Looks good. Feel free to add:
>
> Reviewed-by: Jan Kara <[email protected]>
>
Thanks a lot for your review!
Yi.
>
>> ---
>> fs/ext4/extents.c | 111 +++++++++++++++++++++++++++++-----------------
>> 1 file changed, 70 insertions(+), 41 deletions(-)
>>
>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>> index d5efe076d3d3..e0b7e48c4c67 100644
>> --- a/fs/ext4/extents.c
>> +++ b/fs/ext4/extents.c
>> @@ -2229,7 +2229,7 @@ static int ext4_fill_es_cache_info(struct inode *inode,
>>
>>
>> /*
>> - * ext4_ext_determine_hole - determine hole around given block
>> + * ext4_ext_find_hole - find hole around given block according to the given path
>> * @inode: inode we lookup in
>> * @path: path in extent tree to @lblk
>> * @lblk: pointer to logical block around which we want to determine hole
>> @@ -2241,9 +2241,9 @@ static int ext4_fill_es_cache_info(struct inode *inode,
>> * The function returns the length of a hole starting at @lblk. We update @lblk
>> * to the beginning of the hole if we managed to find it.
>> */
>> -static ext4_lblk_t ext4_ext_determine_hole(struct inode *inode,
>> - struct ext4_ext_path *path,
>> - ext4_lblk_t *lblk)
>> +static ext4_lblk_t ext4_ext_find_hole(struct inode *inode,
>> + struct ext4_ext_path *path,
>> + ext4_lblk_t *lblk)
>> {
>> int depth = ext_depth(inode);
>> struct ext4_extent *ex;
>> @@ -2270,30 +2270,6 @@ static ext4_lblk_t ext4_ext_determine_hole(struct inode *inode,
>> return len;
>> }
>>
>> -/*
>> - * ext4_ext_put_gap_in_cache:
>> - * calculate boundaries of the gap that the requested block fits into
>> - * and cache this gap
>> - */
>> -static void
>> -ext4_ext_put_gap_in_cache(struct inode *inode, ext4_lblk_t hole_start,
>> - ext4_lblk_t hole_len)
>> -{
>> - struct extent_status es;
>> -
>> - ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start,
>> - hole_start + hole_len - 1, &es);
>> - if (es.es_len) {
>> - /* There's delayed extent containing lblock? */
>> - if (es.es_lblk <= hole_start)
>> - return;
>> - hole_len = min(es.es_lblk - hole_start, hole_len);
>> - }
>> - ext_debug(inode, " -> %u:%u\n", hole_start, hole_len);
>> - ext4_es_insert_extent(inode, hole_start, hole_len, ~0,
>> - EXTENT_STATUS_HOLE);
>> -}
>> -
>> /*
>> * ext4_ext_rm_idx:
>> * removes index from the index block.
>> @@ -4062,6 +4038,69 @@ static int get_implied_cluster_alloc(struct super_block *sb,
>> return 0;
>> }
>>
>> +/*
>> + * Determine hole length around the given logical block, first try to
>> + * locate and expand the hole from the given @path, and then adjust it
>> + * if it's partially or completely converted to delayed extents, insert
>> + * it into the extent cache tree if it's indeed a hole, finally return
>> + * the length of the determined extent.
>> + */
>> +static ext4_lblk_t ext4_ext_determine_insert_hole(struct inode *inode,
>> + struct ext4_ext_path *path,
>> + ext4_lblk_t lblk)
>> +{
>> + ext4_lblk_t hole_start, len;
>> + struct extent_status es;
>> +
>> + hole_start = lblk;
>> + len = ext4_ext_find_hole(inode, path, &hole_start);
>> +again:
>> + ext4_es_find_extent_range(inode, &ext4_es_is_delayed, hole_start,
>> + hole_start + len - 1, &es);
>> + if (!es.es_len)
>> + goto insert_hole;
>> +
>> + /*
>> + * There's a delalloc extent in the hole, handle it if the delalloc
>> + * extent is in front of, behind and straddle the queried range.
>> + */
>> + if (lblk >= es.es_lblk + es.es_len) {
>> + /*
>> + * The delalloc extent is in front of the queried range,
>> + * find again from the queried start block.
>> + */
>> + len -= lblk - hole_start;
>> + hole_start = lblk;
>> + goto again;
>> + } else if (in_range(lblk, es.es_lblk, es.es_len)) {
>> + /*
>> + * The delalloc extent containing lblk, it must have been
>> + * added after ext4_map_blocks() checked the extent status
>> + * tree, adjust the length to the delalloc extent's after
>> + * lblk.
>> + */
>> + len = es.es_lblk + es.es_len - lblk;
>> + return len;
>> + } else {
>> + /*
>> + * The delalloc extent is partially or completely behind
>> + * the queried range, update hole length until the
>> + * beginning of the delalloc extent.
>> + */
>> + len = min(es.es_lblk - hole_start, len);
>> + }
>> +
>> +insert_hole:
>> + /* Put just found gap into cache to speed up subsequent requests */
>> + ext_debug(inode, " -> %u:%u\n", hole_start, len);
>> + ext4_es_insert_extent(inode, hole_start, len, ~0, EXTENT_STATUS_HOLE);
>> +
>> + /* Update hole_len to reflect hole size after lblk */
>> + if (hole_start != lblk)
>> + len -= lblk - hole_start;
>> +
>> + return len;
>> +}
>>
>> /*
>> * Block allocation/map/preallocation routine for extents based files
>> @@ -4179,22 +4218,12 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode,
>> * we couldn't try to create block if create flag is zero
>> */
>> if ((flags & EXT4_GET_BLOCKS_CREATE) == 0) {
>> - ext4_lblk_t hole_start, hole_len;
>> + ext4_lblk_t len;
>>
>> - hole_start = map->m_lblk;
>> - hole_len = ext4_ext_determine_hole(inode, path, &hole_start);
>> - /*
>> - * put just found gap into cache to speed up
>> - * subsequent requests
>> - */
>> - ext4_ext_put_gap_in_cache(inode, hole_start, hole_len);
>> + len = ext4_ext_determine_insert_hole(inode, path, map->m_lblk);
>>
>> - /* Update hole_len to reflect hole size after map->m_lblk */
>> - if (hole_start != map->m_lblk)
>> - hole_len -= map->m_lblk - hole_start;
>> map->m_pblk = 0;
>> - map->m_len = min_t(unsigned int, map->m_len, hole_len);
>> -
>> + map->m_len = min_t(unsigned int, map->m_len, len);
>> goto out;
>> }
>>
>> --
>> 2.39.2
>>