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
