Re: [PATCH 42/50] xfs_io: display rt group in verbose fsmap output

Darrick J. Wong
2024-12-11T22:12:31Z

On Mon, Dec 09, 2024 at 10:00:16PM -0800, Christoph Hellwig wrote:
> > + } else if (p->fmr_device == xfs_rt_dev && fsgeo->rgcount > 0) {
> > + agno = p->fmr_physical / bperrtg;
> > + agoff = p->fmr_physical - (agno * bperrtg);
>
> This second calculation seems awfully complicated vs the simple:
>
> agoff = p->fmr_physical % bperrtg;
>
> Any reason for that except for copy and pasting the AG version?
>
> > + } else if (p->fmr_device == xfs_rt_dev && fsgeo->rgcount > 0) {
> > + agno = p->fmr_physical / bperrtg;
> > + agoff = p->fmr_physical - (agno * bperrtg);
>
> Also a little annoying that all this is duplicated, but that also seems
> to be based off the AG version.

No particular reason outside of copy pasta.

> So while this could all look a little nicer, the changes themselves
> looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!

--D