diff mbox series

[5/8] xfs: create helpers for rtbitmap block/wordcount computations

Message ID 169704721706.1773834.7063943000548807823.stgit@frogsfrogsfrogs (mailing list archive)
State Superseded
Headers show
Series xfs: refactor rtbitmap/summary macros | expand

Commit Message

Darrick J. Wong Oct. 11, 2023, 6:07 p.m. UTC
From: Darrick J. Wong <djwong@kernel.org>

Create helper functions that compute the number of blocks or words
necessary to store the rt bitmap.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_rtbitmap.c   |   27 +++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_rtbitmap.h   |   12 ++++++++++++
 fs/xfs/libxfs/xfs_trans_resv.c |    9 +++++----
 fs/xfs/scrub/rtsummary.c       |    7 +++----
 fs/xfs/xfs_rtalloc.c           |    2 +-
 5 files changed, 48 insertions(+), 9 deletions(-)

Comments

Christoph Hellwig Oct. 12, 2023, 5:44 a.m. UTC | #1
On Wed, Oct 11, 2023 at 11:07:32AM -0700, Darrick J. Wong wrote:
> +/*
> + * Compute the number of rtbitmap blocks needed to track the given number of rt
> + * extents.
> + */
> +xfs_filblks_t
> +xfs_rtbitmap_blockcount(
> +	struct xfs_mount	*mp,
> +	xfs_rtbxlen_t		rtextents)
> +{
> +	return howmany_64(rtextents, NBBY * mp->m_sb.sb_blocksize);
> +}

Given that this only has a few users, the !RT stub is a pain, and
having a different result from before in the transaction reservation
is somewhat unexpected change (even if harmless), maybe just mark
this inline?

> +/*
> + * Compute the number of rtbitmap words needed to populate every block of a
> + * bitmap that is large enough to track the given number of rt extents.
> + */
> +unsigned long long
> +xfs_rtbitmap_wordcount(
> +	struct xfs_mount	*mp,
> +	xfs_rtbxlen_t		rtextents)
> +{
> +	xfs_filblks_t		blocks;
> +
> +	blocks = xfs_rtbitmap_blockcount(mp, rtextents);
> +	return XFS_FSB_TO_B(mp, blocks) >> XFS_WORDLOG;
> +}

This one isn't used in this patch or the rest of the series.  Maybe
move it to the patch (-series) that adds the caller in the repair
code?
Darrick J. Wong Oct. 12, 2023, 9:55 p.m. UTC | #2
On Thu, Oct 12, 2023 at 07:44:33AM +0200, Christoph Hellwig wrote:
> On Wed, Oct 11, 2023 at 11:07:32AM -0700, Darrick J. Wong wrote:
> > +/*
> > + * Compute the number of rtbitmap blocks needed to track the given number of rt
> > + * extents.
> > + */
> > +xfs_filblks_t
> > +xfs_rtbitmap_blockcount(
> > +	struct xfs_mount	*mp,
> > +	xfs_rtbxlen_t		rtextents)
> > +{
> > +	return howmany_64(rtextents, NBBY * mp->m_sb.sb_blocksize);
> > +}
> 
> Given that this only has a few users, the !RT stub is a pain, and
> having a different result from before in the transaction reservation
> is somewhat unexpected change (even if harmless),

Ohh, right, I didn't even notice that the result changes slightly when
the we go from dividing by NBBY before howmany'ing with blocksize to
howmany'ing with (NBBY * blocksize).

> maybe just mark this inline?

I could make these inline functions at the bottom of xfs_rtbitmap.h, and
even put them outside of the #ifdef RT bits.  That'll get rid of two
stubs for now, but later the rtgroups patchset wants to add a header
to rtbitmap blocks.  Then we'll need a function to compute the number of
rtextents covered by a single bitmap block:

/* Compute the number of rt extents tracked by a single bitmap block. */
xfs_rtxnum_t
xfs_rtbitmap_rtx_per_rbmblock(
	struct xfs_mount	*mp)
{
	unsigned int		rbmblock_bytes = mp->m_sb.sb_blocksize;

	if (xfs_has_rtgroups(mp))
		rbmblock_bytes -= sizeof(struct xfs_rtbuf_blkinfo);

	return rbmblock_bytes * NBBY;
}

So then either this function will need to have a stub that returns a
garbage value.  Alternately, it could move out of xfs_rtbitmap.c.

Right now, xfs_rtbitmap_rtx_per_rbmblock isn't even a defined symbol for
!RT, and the stub version of _wordcount and _blockcount return 0, which
at least makes sense.

> > +/*
> > + * Compute the number of rtbitmap words needed to populate every block of a
> > + * bitmap that is large enough to track the given number of rt extents.
> > + */
> > +unsigned long long
> > +xfs_rtbitmap_wordcount(
> > +	struct xfs_mount	*mp,
> > +	xfs_rtbxlen_t		rtextents)
> > +{
> > +	xfs_filblks_t		blocks;
> > +
> > +	blocks = xfs_rtbitmap_blockcount(mp, rtextents);
> > +	return XFS_FSB_TO_B(mp, blocks) >> XFS_WORDLOG;
> > +}
> 
> This one isn't used in this patch or the rest of the series.  Maybe
> move it to the patch (-series) that adds the caller in the repair
> code?

<shrug> The xfsprogs version of this patch uses this helper to decrapify
the incore rtbitmap computation in xfs_repair:

diff --git a/repair/rt.c b/repair/rt.c
index 8f3b9082a9b..244b59f04ce 100644
--- a/repair/rt.c
+++ b/repair/rt.c
@@ -19,6 +19,8 @@
 void
 rtinit(xfs_mount_t *mp)
 {
+       unsigned long long      wordcnt;
+
        if (mp->m_sb.sb_rblocks == 0)
                return;
 
@@ -26,11 +28,9 @@ rtinit(xfs_mount_t *mp)
         * realtime init -- blockmap initialization is
         * handled by incore_init()
         */
-       /*
-       sumfile = calloc(mp->m_rsumsize, 1);
-       */
-       if ((btmcompute = calloc(mp->m_sb.sb_rbmblocks *
-                       mp->m_sb.sb_blocksize, 1)) == NULL)
+       wordcnt = libxfs_rtbitmap_wordcount(mp, mp->m_sb.sb_rextents);
+       btmcompute = calloc(wordcnt, sizeof(xfs_rtword_t));
+       if (!btmcompute)
                do_error(
        _("couldn't allocate memory for incore realtime bitmap.\n"));

So I'd rather leave these two helpers defined as they are.

--D
Christoph Hellwig Oct. 13, 2023, 4:26 a.m. UTC | #3
On Thu, Oct 12, 2023 at 02:55:46PM -0700, Darrick J. Wong wrote:
> <shrug> The xfsprogs version of this patch uses this helper to decrapify
> the incore rtbitmap computation in xfs_repair:

Ok, let's keep them as-is.
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index b6a1d240c5545..2a453f0215ee0 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -1142,3 +1142,30 @@  xfs_rtalloc_extent_is_free(
 	*is_free = matches;
 	return 0;
 }
+
+/*
+ * Compute the number of rtbitmap blocks needed to track the given number of rt
+ * extents.
+ */
+xfs_filblks_t
+xfs_rtbitmap_blockcount(
+	struct xfs_mount	*mp,
+	xfs_rtbxlen_t		rtextents)
+{
+	return howmany_64(rtextents, NBBY * mp->m_sb.sb_blocksize);
+}
+
+/*
+ * Compute the number of rtbitmap words needed to populate every block of a
+ * bitmap that is large enough to track the given number of rt extents.
+ */
+unsigned long long
+xfs_rtbitmap_wordcount(
+	struct xfs_mount	*mp,
+	xfs_rtbxlen_t		rtextents)
+{
+	xfs_filblks_t		blocks;
+
+	blocks = xfs_rtbitmap_blockcount(mp, rtextents);
+	return XFS_FSB_TO_B(mp, blocks) >> XFS_WORDLOG;
+}
diff --git a/fs/xfs/libxfs/xfs_rtbitmap.h b/fs/xfs/libxfs/xfs_rtbitmap.h
index f616956b28911..308ce814a908a 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.h
+++ b/fs/xfs/libxfs/xfs_rtbitmap.h
@@ -261,6 +261,11 @@  xfs_rtfree_extent(
 /* Same as above, but in units of rt blocks. */
 int xfs_rtfree_blocks(struct xfs_trans *tp, xfs_fsblock_t rtbno,
 		xfs_filblks_t rtlen);
+
+xfs_filblks_t xfs_rtbitmap_blockcount(struct xfs_mount *mp, xfs_rtbxlen_t
+		rtextents);
+unsigned long long xfs_rtbitmap_wordcount(struct xfs_mount *mp,
+		xfs_rtbxlen_t rtextents);
 #else /* CONFIG_XFS_RT */
 # define xfs_rtfree_extent(t,b,l)			(-ENOSYS)
 # define xfs_rtfree_blocks(t,rb,rl)			(-ENOSYS)
@@ -268,6 +273,13 @@  int xfs_rtfree_blocks(struct xfs_trans *tp, xfs_fsblock_t rtbno,
 # define xfs_rtalloc_query_all(m,t,f,p)			(-ENOSYS)
 # define xfs_rtbuf_get(m,t,b,i,p)			(-ENOSYS)
 # define xfs_rtalloc_extent_is_free(m,t,s,l,i)		(-ENOSYS)
+static inline xfs_filblks_t
+xfs_rtbitmap_blockcount(struct xfs_mount *mp, xfs_rtbxlen_t rtextents)
+{
+	/* shut up gcc */
+	return 0;
+}
+# define xfs_rtbitmap_wordcount(mp, r)			(0)
 #endif /* CONFIG_XFS_RT */
 
 #endif /* __XFS_RTBITMAP_H__ */
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index 9c3dbcbe05358..01fccc4ade8a5 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -221,11 +221,12 @@  xfs_rtalloc_block_count(
 	struct xfs_mount	*mp,
 	unsigned int		num_ops)
 {
-	unsigned int		blksz = XFS_FSB_TO_B(mp, 1);
-	unsigned int		rtbmp_bytes;
+	unsigned int		rtbmp_blocks;
+	xfs_rtxlen_t		rtxlen;
 
-	rtbmp_bytes = xfs_extlen_to_rtxlen(mp, XFS_MAX_BMBT_EXTLEN) / NBBY;
-	return (howmany(rtbmp_bytes, blksz) + 1) * num_ops;
+	rtxlen = xfs_extlen_to_rtxlen(mp, XFS_MAX_BMBT_EXTLEN);
+	rtbmp_blocks = xfs_rtbitmap_blockcount(mp, rtxlen);
+	return (rtbmp_blocks + 1) * num_ops;
 }
 
 /*
diff --git a/fs/xfs/scrub/rtsummary.c b/fs/xfs/scrub/rtsummary.c
index 901696c593dc9..29cd26b7779d1 100644
--- a/fs/xfs/scrub/rtsummary.c
+++ b/fs/xfs/scrub/rtsummary.c
@@ -172,12 +172,11 @@  xchk_rtsum_compute(
 	struct xfs_scrub	*sc)
 {
 	struct xfs_mount	*mp = sc->mp;
-	unsigned long long	rtbmp_bytes;
+	unsigned long long	rtbmp_blocks;
 
 	/* If the bitmap size doesn't match the computed size, bail. */
-	rtbmp_bytes = howmany_64(mp->m_sb.sb_rextents, NBBY);
-	if (roundup_64(rtbmp_bytes, mp->m_sb.sb_blocksize) !=
-			mp->m_rbmip->i_disk_size)
+	rtbmp_blocks = xfs_rtbitmap_blockcount(mp, mp->m_sb.sb_rextents);
+	if (XFS_FSB_TO_B(mp, rtbmp_blocks) != mp->m_rbmip->i_disk_size)
 		return -EFSCORRUPTED;
 
 	return xfs_rtalloc_query_all(sc->mp, sc->tp, xchk_rtsum_record_free,
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 14d01b97b892d..32e3ac40e28f7 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -996,7 +996,7 @@  xfs_growfs_rt(
 	 */
 	nrextents = nrblocks;
 	do_div(nrextents, in->extsize);
-	nrbmblocks = howmany_64(nrextents, NBBY * sbp->sb_blocksize);
+	nrbmblocks = xfs_rtbitmap_blockcount(mp, nrextents);
 	nrextslog = xfs_highbit32(nrextents);
 	nrsumlevels = nrextslog + 1;
 	nrsumsize = (uint)sizeof(xfs_suminfo_t) * nrsumlevels * nrbmblocks;