Re: [PATCH 46/50] xfs_scrub: call GETFSMAP for each rt group in parallel

Darrick J. Wong
2024-12-11T22:18:32Z

On Mon, Dec 09, 2024 at 10:04:54PM -0800, Christoph Hellwig wrote:
> > + struct fsmap keys[2];
> > + off_t bperrg = bytes_per_rtgroup(&ctx->mnt.fsgeom);
> > + int ret;
> > +
> > +
> > + memset(keys, 0, sizeof(struct fsmap) * 2);
>
> This could be simplified to
>
> struct fsmap keys[2] = { };
>
> instead of the manual initialization.
>
> > + keys->fmr_device = ctx->fsinfo.fs_rtdev;
> > + keys->fmr_physical = (xfs_rtblock_t)rgno * bperrg;
> > + (keys + 1)->fmr_device = ctx->fsinfo.fs_rtdev;
> > + (keys + 1)->fmr_physical = ((rgno + 1) * bperrg) - 1;
> > + (keys + 1)->fmr_owner = ULLONG_MAX;
> > + (keys + 1)->fmr_offset = ULLONG_MAX;
> > + (keys + 1)->fmr_flags = UINT_MAX;
>
> The usage of keys here and various other places looks really odd.
> It's an array, so doing pointer math instad of the simple
>
>
> keys[0].fmr_device = ctx->fsinfo.fs_rtdev;
>
> keys[1].fmr_device = ctx->fsinfo.fs_rtdev;
>
> ..
>
> is rather confusing. I've actually cleaned this up but forgot to
> send it to you. Feel free to grab the patch here:
>
> http://git.infradead.org/?p=users/hch/xfsprogs.git;a=commitdiff;h=5699e03cf03e6b1189a89f903631046d16980ff6
>
> and fold it in.

Folded, thanks for the cleanup.

> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks! I've folded the scan_rtg_rmaps cleanup into this patch but left
the other two as a separate patch.

--D