On 2024/10/23 21:59, Theodore Ts'o wrote:
> The original implementation ext4's FS_IOC_GETFSMAP handling only
> worked when the range of queried blocks included at least one free
> (unallocated) block range. This is because how the metadata blocks
> were emitted was as a side effect of ext4_mballoc_query_range()
> calling ext4_getfsmap_datadev_helper(), and that function was only
> called when a free block range was identified. As a result, this
> caused generic/365 to fail.
>
> Fix this by creating a new function ext4_getfsmap_meta_helper() which
> gets called so that blocks before the first free block range in a
> block group can get properly reported.
>
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> ---
> fs/ext4/fsmap.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++-
> fs/ext4/mballoc.c | 18 ++++++++++++----
> fs/ext4/mballoc.h | 1 +
> 3 files changed, 68 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/fsmap.c b/fs/ext4/fsmap.c
> index df853c4d3a8c..383c6edea6dd 100644
> --- a/fs/ext4/fsmap.c
> +++ b/fs/ext4/fsmap.c
> @@ -185,6 +185,56 @@ static inline ext4_fsblk_t ext4_fsmap_next_pblk(struct ext4_fsmap *fmr)
> return fmr->fmr_physical + fmr->fmr_length;
> }
>
> +static int ext4_getfsmap_meta_helper(struct super_block *sb,
> + ext4_group_t agno, ext4_grpblk_t start,
> + ext4_grpblk_t len, void *priv)
> +{
> + struct ext4_getfsmap_info *info = priv;
> + struct ext4_fsmap *p;
> + struct ext4_fsmap *tmp;
> + struct ext4_sb_info *sbi = EXT4_SB(sb);
> + ext4_fsblk_t fsb, fs_start, fs_end;
> + int error;
> +
> + fs_start = fsb = (EXT4_C2B(sbi, start) +
> + ext4_group_first_block_no(sb, agno));
> + fs_end = fs_start + EXT4_C2B(sbi, len);
> +
> + /* Return relevant extents from the meta_list */
> + list_for_each_entry_safe(p, tmp, &info->gfi_meta_list, fmr_list) {
> + if (p->fmr_physical < info->gfi_next_fsblk) {
> + list_del(&p->fmr_list);
> + kfree(p);
> + continue;
> + }
> + if (p->fmr_physical <= fs_start ||
> + p->fmr_physical + p->fmr_length <= fs_end) {
> + /* Emit the retained free extent record if present */
> + if (info->gfi_lastfree.fmr_owner) {
> + error = ext4_getfsmap_helper(sb, info,
> + &info->gfi_lastfree);
> + if (error)
> + return error;
> + info->gfi_lastfree.fmr_owner = 0;
> + }
> + error = ext4_getfsmap_helper(sb, info, p);
> + if (error)
> + return error;
> + fsb = p->fmr_physical + p->fmr_length;
> + if (info->gfi_next_fsblk < fsb)
> + info->gfi_next_fsblk = fsb;
> + list_del(&p->fmr_list);
> + kfree(p);
> + continue;
> + }
> + }
> + if (info->gfi_next_fsblk < fsb)
> + info->gfi_next_fsblk = fsb;
> +
> + return 0;
> +}
> +
> +
> /* Transform a blockgroup's free record into a fsmap */
> static int ext4_getfsmap_datadev_helper(struct super_block *sb,
> ext4_group_t agno, ext4_grpblk_t start,
> @@ -539,6 +589,7 @@ static int ext4_getfsmap_datadev(struct super_block *sb,
> error = ext4_mballoc_query_range(sb, info->gfi_agno,
> EXT4_B2C(sbi, info->gfi_low.fmr_physical),
> EXT4_B2C(sbi, info->gfi_high.fmr_physical),
> + ext4_getfsmap_meta_helper,
> ext4_getfsmap_datadev_helper, info);
> if (error)
> goto err;
> @@ -560,7 +611,8 @@ static int ext4_getfsmap_datadev(struct super_block *sb,
>
> /* Report any gaps at the end of the bg */
> info->gfi_last = true;
> - error = ext4_getfsmap_datadev_helper(sb, end_ag, last_cluster, 0, info);
> + error = ext4_getfsmap_datadev_helper(sb, end_ag, last_cluster + 1,
> + 0, info);
> if (error)
> goto err;
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index d73e38323879..92f49d7eb3c0 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -6999,13 +6999,14 @@ int
> ext4_mballoc_query_range(
> struct super_block *sb,
> ext4_group_t group,
> - ext4_grpblk_t start,
> + ext4_grpblk_t first,
> ext4_grpblk_t end,
> + ext4_mballoc_query_range_fn meta_formatter,
> ext4_mballoc_query_range_fn formatter,
> void *priv)
> {
> void *bitmap;
> - ext4_grpblk_t next;
> + ext4_grpblk_t start, next;
> struct ext4_buddy e4b;
> int error;
>
> @@ -7016,10 +7017,19 @@ ext4_mballoc_query_range(
>
> ext4_lock_group(sb, group);
>
> - start = max(e4b.bd_info->bb_first_free, start);
> + start = max(e4b.bd_info->bb_first_free, first);
> if (end >= EXT4_CLUSTERS_PER_GROUP(sb))
> end = EXT4_CLUSTERS_PER_GROUP(sb) - 1;
> -
> + if (meta_formatter && start != first) {
> + if (start > end)
> + start = end;
> + ext4_unlock_group(sb, group);
> + error = meta_formatter(sb, group, first, start - first,
> + priv);
> + if (error)
> + goto out_unload;
> + ext4_lock_group(sb, group);
> + }
Hi, Ted!
Now it seems to be able to handle all the necessary metadata in the
query range here, can we remove the processing of info->gfi_meta_list
in ext4_getfsmap_datadev_helper() as well?
Thanks,
Yi.
> while (start <= end) {
> start = mb_find_next_zero_bit(bitmap, end + 1, start);
> if (start > end)
> diff --git a/fs/ext4/mballoc.h b/fs/ext4/mballoc.h
> index d8553f1498d3..f8280de3e882 100644
> --- a/fs/ext4/mballoc.h
> +++ b/fs/ext4/mballoc.h
> @@ -259,6 +259,7 @@ ext4_mballoc_query_range(
> ext4_group_t agno,
> ext4_grpblk_t start,
> ext4_grpblk_t end,
> + ext4_mballoc_query_range_fn meta_formatter,
> ext4_mballoc_query_range_fn formatter,
> void *priv);
>
