| Message ID | 163408160882.4151249.14701173486144926020.stgit@magnolia (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: support dynamic btree cursor height | expand |
On Tue, Oct 12, 2021 at 04:33:28PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Compute the actual maximum btree height when deciding if per-AG block > reservation is critically low. This only affects the sanity check > condition, since we /generally/ will trigger on the 10% threshold. > This is a long-winded way of saying that we're removing one more > usage of XFS_BTREE_MAXLEVELS. And replacing it with a branchy dynamic calculation that has a static, unchanging result. :( > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/libxfs/xfs_ag_resv.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > > diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c > index 2aa2b3484c28..d34d4614f175 100644 > --- a/fs/xfs/libxfs/xfs_ag_resv.c > +++ b/fs/xfs/libxfs/xfs_ag_resv.c > @@ -60,6 +60,20 @@ > * to use the reservation system should update ask/used in xfs_ag_resv_init. > */ > > +/* Compute maximum possible height for per-AG btree types for this fs. */ > +static unsigned int > +xfs_ag_btree_maxlevels( > + struct xfs_mount *mp) > +{ > + unsigned int ret = mp->m_ag_maxlevels; > + > + ret = max(ret, mp->m_bm_maxlevels[XFS_DATA_FORK]); > + ret = max(ret, mp->m_bm_maxlevels[XFS_ATTR_FORK]); > + ret = max(ret, M_IGEO(mp)->inobt_maxlevels); > + ret = max(ret, mp->m_rmap_maxlevels); > + return max(ret, mp->m_refc_maxlevels); > +} Hmmmm. perhaps mp->m_ag_maxlevels should be renamed to mp->m_agbno_maxlevels and we pre-calculate mp->m_ag_maxlevels from the above function and just use the variable in the xfs_ag_resv_critical() check? Cheers, Dave.
On Wed, Oct 13, 2021 at 04:49:39PM +1100, Dave Chinner wrote: > On Tue, Oct 12, 2021 at 04:33:28PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Compute the actual maximum btree height when deciding if per-AG block > > reservation is critically low. This only affects the sanity check > > condition, since we /generally/ will trigger on the 10% threshold. > > This is a long-winded way of saying that we're removing one more > > usage of XFS_BTREE_MAXLEVELS. > > And replacing it with a branchy dynamic calculation that has a > static, unchanging result. :( > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > fs/xfs/libxfs/xfs_ag_resv.c | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c > > index 2aa2b3484c28..d34d4614f175 100644 > > --- a/fs/xfs/libxfs/xfs_ag_resv.c > > +++ b/fs/xfs/libxfs/xfs_ag_resv.c > > @@ -60,6 +60,20 @@ > > * to use the reservation system should update ask/used in xfs_ag_resv_init. > > */ > > > > +/* Compute maximum possible height for per-AG btree types for this fs. */ > > +static unsigned int > > +xfs_ag_btree_maxlevels( > > + struct xfs_mount *mp) > > +{ > > + unsigned int ret = mp->m_ag_maxlevels; > > + > > + ret = max(ret, mp->m_bm_maxlevels[XFS_DATA_FORK]); > > + ret = max(ret, mp->m_bm_maxlevels[XFS_ATTR_FORK]); > > + ret = max(ret, M_IGEO(mp)->inobt_maxlevels); > > + ret = max(ret, mp->m_rmap_maxlevels); > > + return max(ret, mp->m_refc_maxlevels); > > +} > > Hmmmm. perhaps mp->m_ag_maxlevels should be renamed to > mp->m_agbno_maxlevels and we pre-calculate mp->m_ag_maxlevels from I prefer m_alloc_maxlevels for the first one, since "agbno" means "AG block number" in my head. As for the second, how about "m_agbtree_maxlevels" since we already use 'agbtree' to refer to per-AG btrees elsewhere? Other than the naming, I agree with your suggestion. --D > the above function and just use the variable in the > xfs_ag_resv_critical() check? > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com
On Wed, Oct 13, 2021 at 10:07:47AM -0700, Darrick J. Wong wrote: > On Wed, Oct 13, 2021 at 04:49:39PM +1100, Dave Chinner wrote: > > On Tue, Oct 12, 2021 at 04:33:28PM -0700, Darrick J. Wong wrote: > > > From: Darrick J. Wong <djwong@kernel.org> > > > > > > Compute the actual maximum btree height when deciding if per-AG block > > > reservation is critically low. This only affects the sanity check > > > condition, since we /generally/ will trigger on the 10% threshold. > > > This is a long-winded way of saying that we're removing one more > > > usage of XFS_BTREE_MAXLEVELS. > > > > And replacing it with a branchy dynamic calculation that has a > > static, unchanging result. :( > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > > --- > > > fs/xfs/libxfs/xfs_ag_resv.c | 18 +++++++++++++++++- > > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > > > > > > diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c > > > index 2aa2b3484c28..d34d4614f175 100644 > > > --- a/fs/xfs/libxfs/xfs_ag_resv.c > > > +++ b/fs/xfs/libxfs/xfs_ag_resv.c > > > @@ -60,6 +60,20 @@ > > > * to use the reservation system should update ask/used in xfs_ag_resv_init. > > > */ > > > > > > +/* Compute maximum possible height for per-AG btree types for this fs. */ > > > +static unsigned int > > > +xfs_ag_btree_maxlevels( > > > + struct xfs_mount *mp) > > > +{ > > > + unsigned int ret = mp->m_ag_maxlevels; > > > + > > > + ret = max(ret, mp->m_bm_maxlevels[XFS_DATA_FORK]); > > > + ret = max(ret, mp->m_bm_maxlevels[XFS_ATTR_FORK]); > > > + ret = max(ret, M_IGEO(mp)->inobt_maxlevels); > > > + ret = max(ret, mp->m_rmap_maxlevels); > > > + return max(ret, mp->m_refc_maxlevels); > > > +} > > > > Hmmmm. perhaps mp->m_ag_maxlevels should be renamed to > > mp->m_agbno_maxlevels and we pre-calculate mp->m_ag_maxlevels from > > I prefer m_alloc_maxlevels for the first one, since "agbno" means "AG > block number" in my head. > > As for the second, how about "m_agbtree_maxlevels" since we already use > 'agbtree' to refer to per-AG btrees elsewhere? Much better than my suggestions :) Cheers, Dave.
diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c index 2aa2b3484c28..d34d4614f175 100644 --- a/fs/xfs/libxfs/xfs_ag_resv.c +++ b/fs/xfs/libxfs/xfs_ag_resv.c @@ -60,6 +60,20 @@ * to use the reservation system should update ask/used in xfs_ag_resv_init. */ +/* Compute maximum possible height for per-AG btree types for this fs. */ +static unsigned int +xfs_ag_btree_maxlevels( + struct xfs_mount *mp) +{ + unsigned int ret = mp->m_ag_maxlevels; + + ret = max(ret, mp->m_bm_maxlevels[XFS_DATA_FORK]); + ret = max(ret, mp->m_bm_maxlevels[XFS_ATTR_FORK]); + ret = max(ret, M_IGEO(mp)->inobt_maxlevels); + ret = max(ret, mp->m_rmap_maxlevels); + return max(ret, mp->m_refc_maxlevels); +} + /* * Are we critically low on blocks? For now we'll define that as the number * of blocks we can get our hands on being less than 10% of what we reserved @@ -72,6 +86,7 @@ xfs_ag_resv_critical( { xfs_extlen_t avail; xfs_extlen_t orig; + xfs_extlen_t btree_maxlevels; switch (type) { case XFS_AG_RESV_METADATA: @@ -91,7 +106,8 @@ xfs_ag_resv_critical( trace_xfs_ag_resv_critical(pag, type, avail); /* Critically low if less than 10% or max btree height remains. */ - return XFS_TEST_ERROR(avail < orig / 10 || avail < XFS_BTREE_MAXLEVELS, + btree_maxlevels = xfs_ag_btree_maxlevels(pag->pag_mount); + return XFS_TEST_ERROR(avail < orig / 10 || avail < btree_maxlevels, pag->pag_mount, XFS_ERRTAG_AG_RESV_CRITICAL); }