On Tue 02-01-24 20:38:56, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>
> 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 <yi.zhang@xxxxxxxxxx>
> Suggested-by: Jan Kara <jack@xxxxxxx>
Some small comments below.
> @@ -4062,6 +4038,66 @@ 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.
> + */
> +static void ext4_ext_determine_hole(struct inode *inode,
> + struct ext4_ext_path *path,
> + struct ext4_map_blocks *map)
> +{
I'd prefer to keep setting of 'map' inside ext4_ext_map_blocks() to make it
more obvious there and just pass map->m_lblk into
ext4_ext_determine_hole(). ext4_ext_determine_hole() will just return the
length of the extent from lblk that was mapped (i.e. 'len').
Also I'd probably call this function like ext4_ext_determine_insert_hole()
to make it more obvious the function modifies the status tree.
Otherwise the patch looks good to me.
Honza
> + ext4_lblk_t hole_start, len;
> + struct extent_status es;
> +
> + hole_start = map->m_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;
> +
> + /*
> + * Found a delayed range in the hole, handle it if the delayed
> + * range is before, behind and straddle the queried range.
> + */
> + if (map->m_lblk >= es.es_lblk + es.es_len) {
> + /*
> + * Before the queried range, find again from the queried
> + * start block.
> + */
> + len -= map->m_lblk - hole_start;
> + hole_start = map->m_lblk;
> + goto again;
> + } else if (in_range(map->m_lblk, es.es_lblk, es.es_len)) {
> + /*
> + * Straddle the beginning of the queried range, it's no
> + * longer a hole, adjust the length to the delayed extent's
> + * after map->m_lblk.
> + */
> + len = es.es_lblk + es.es_len - map->m_lblk;
> + goto out;
> + } else {
> + /*
> + * Partially or completely behind the queried range, update
> + * hole length until the beginning of the delayed 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 map->m_lblk */
> + if (hole_start != map->m_lblk)
> + len -= map->m_lblk - hole_start;
> +out:
> + map->m_pblk = 0;
> + map->m_len = min_t(unsigned int, map->m_len, len);
> +}
>
> /*
> * Block allocation/map/preallocation routine for extents based files
> @@ -4179,22 +4215,7 @@ 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;
> -
> - 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);
> -
> - /* 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);
> -
> + ext4_ext_determine_hole(inode, path, map);
> goto out;
> }
>
> --
> 2.39.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
