On Thu, Apr 07, 2022 at 11:05:44AM +1000, Dave Chinner wrote: > On Wed, Apr 06, 2022 at 11:48:56AM +0530, Chandan Babu R wrote: > > This commit defines new macros to represent maximum extent counts allowed by > > filesystems which have support for large per-inode extent counters. > > > > Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx> > > Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx> > > --- > > fs/xfs/libxfs/xfs_bmap.c | 9 ++++----- > > fs/xfs/libxfs/xfs_bmap_btree.c | 3 ++- > > fs/xfs/libxfs/xfs_format.h | 24 ++++++++++++++++++++++-- > > fs/xfs/libxfs/xfs_inode_buf.c | 4 +++- > > fs/xfs/libxfs/xfs_inode_fork.c | 3 ++- > > fs/xfs/libxfs/xfs_inode_fork.h | 21 +++++++++++++++++---- > > 6 files changed, 50 insertions(+), 14 deletions(-) > > > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > > index b317226fb4ba..1254d4d4821e 100644 > > --- a/fs/xfs/libxfs/xfs_bmap.c > > +++ b/fs/xfs/libxfs/xfs_bmap.c > > @@ -61,10 +61,8 @@ xfs_bmap_compute_maxlevels( > > int sz; /* root block size */ > > > > /* > > - * The maximum number of extents in a file, hence the maximum number of > > - * leaf entries, is controlled by the size of the on-disk extent count, > > - * either a signed 32-bit number for the data fork, or a signed 16-bit > > - * number for the attr fork. > > + * The maximum number of extents in a fork, hence the maximum number of > > + * leaf entries, is controlled by the size of the on-disk extent count. > > * > > * Note that we can no longer assume that if we are in ATTR1 that the > > * fork offset of all the inodes will be > > @@ -74,7 +72,8 @@ xfs_bmap_compute_maxlevels( > > * ATTR2 we have to assume the worst case scenario of a minimum size > > * available. > > */ > > - maxleafents = xfs_iext_max_nextents(whichfork); > > + maxleafents = xfs_iext_max_nextents(xfs_has_large_extent_counts(mp), > > + whichfork); > > if (whichfork == XFS_DATA_FORK) > > sz = XFS_BMDR_SPACE_CALC(MINDBTPTRS); > > else > > Just to confirm, the large extent count feature bit can only be > added when the filesystem is unmounted? Yes, because we (currently) don't support /any/ online feature upgrades. IIRC Chandan said that you'd have to be careful about validating the min log size requirements are still met because the tx reservation sizes can change with the taller bmbts. > > diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c > > index 453309fc85f2..7aabeccea9ab 100644 > > --- a/fs/xfs/libxfs/xfs_bmap_btree.c > > +++ b/fs/xfs/libxfs/xfs_bmap_btree.c > > @@ -611,7 +611,8 @@ xfs_bmbt_maxlevels_ondisk(void) > > minrecs[1] = xfs_bmbt_block_maxrecs(blocklen, false) / 2; > > > > /* One extra level for the inode root. */ > > - return xfs_btree_compute_maxlevels(minrecs, MAXEXTNUM) + 1; > > + return xfs_btree_compute_maxlevels(minrecs, > > + XFS_MAX_EXTCNT_DATA_FORK_LARGE) + 1; > > } > > Why is this set to XFS_MAX_EXTCNT_DATA_FORK_LARGE rather than being > conditional xfs_has_large_extent_counts(mp)? i.e. if the feature bit > is not set, the maximum on-disk levels in the bmbt is determined by > XFS_MAX_EXTCNT_DATA_FORK_SMALL, not XFS_MAX_EXTCNT_DATA_FORK_LARGE. This function (and all the other _maxlevels_ondisk functions) compute the maximum possible btree height for any filesystem that we'd care to mount. This value is then passed to the functions that create the btree cursor caches, which is why this is independent of any xfs_mount. That said ... depending on how much this inflates the size of the bmbt cursor cache, I think we could create multiple slabs. > The "_ondisk" suffix implies that it has something to do with the > on-disk format of the filesystem, but AFAICT what we are calculating > here is a constant used for in-memory structure allocation? There > needs to be something explained/changed here, because this is > confusing... You suggested it. ;) https://lore.kernel.org/linux-xfs/20211013075743.GG2361455@xxxxxxxxxxxxxxxxxxx/ --D > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx
