Re: [PATCH 16/24] xfs: move RT bitmap and summary information to the rtgroup

Dave Chinner
2024-08-26T01:58:55Z

On Thu, Aug 22, 2024 at 05:18:49PM -0700, Darrick J. Wong wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> Move the pointers to the RT bitmap and summary inodes as well as the
> summary cache to the rtgroups structure to prepare for having a
> separate bitmap and summary inodes for each rtgroup.
>
> Code using the inodes now needs to operate on a rtgroup. Where easily
> possible such code is converted to iterate over all rtgroups, else
> rtgroup 0 (the only one that can currently exist) is hardcoded.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 40 +++-
> fs/xfs/libxfs/xfs_rtbitmap.c | 174 ++++++++--------
> fs/xfs/libxfs/xfs_rtbitmap.h | 68 +++---
> fs/xfs/libxfs/xfs_rtgroup.c | 90 +++++++-
> fs/xfs/libxfs/xfs_rtgroup.h | 14 +
> fs/xfs/scrub/bmap.c | 13 +
> fs/xfs/scrub/fscounters.c | 26 +-
> fs/xfs/scrub/repair.c | 24 ++
> fs/xfs/scrub/repair.h | 7 +
> fs/xfs/scrub/rtbitmap.c | 45 ++--
> fs/xfs/scrub/rtsummary.c | 93 +++++----
> fs/xfs/scrub/rtsummary_repair.c | 7 -
> fs/xfs/scrub/scrub.c | 4
> fs/xfs/xfs_discard.c | 100 ++++++---
> fs/xfs/xfs_fsmap.c | 143 ++++++++-----
> fs/xfs/xfs_mount.h | 10 -
> fs/xfs/xfs_qm.c | 27 ++-
> fs/xfs/xfs_rtalloc.c | 415 ++++++++++++++++++++++-----------------
> 18 files changed, 763 insertions(+), 537 deletions(-)

I'm finding this patch does far too many things to be reviewable.
There's code factoring, abstraction by local variables, changes to
locking APIs, etc that are needed to simplify the conversion, but
could all be done separately before the actual changeover to using
rtgroups.

There's also multiple functional changes in the code - like support
for growfs using rtgroups and moving to per-rtg summary caches - so
it's really difficult to separate and review the individual changes
in this.

Can you please split this up into a couple of separate steps? One
for all the local variable conversions and "no change" factoring,
one to move the summary cache code, one to add the grwofs support
and, finally, one to actually convert everything over to use
rtgroups directly?

-Dave.
--
Dave Chinner
david@fromorbit.com