Message ID | 20240102123918.799062-4-yi.zhang@huaweicloud.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | ext4: use iomap for regular file's buffered IO path and enable large foilo | expand |
On Tue 02-01-24 20:38:56, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@huawei.com> > > 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@huawei.com> > Suggested-by: Jan Kara <jack@suse.cz> 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 >
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index d5efe076d3d3..0892d0568013 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,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) +{ + 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; }