| Message ID | 169704721736.1773834.4052037252966105617.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: refactor rtbitmap/summary macros | expand |
On Wed, Oct 11, 2023 at 11:08:03AM -0700, Darrick J. Wong wrote: > +/* Compute the number of rtsummary blocks needed to track the given rt space. */ > +xfs_filblks_t > +xfs_rtsummary_blockcount( > + struct xfs_mount *mp, > + unsigned int rsumlevels, > + xfs_extlen_t rbmblocks) > +{ > + unsigned long long rsumwords; > + > + rsumwords = (unsigned long long)rsumlevels * rbmblocks; > + return XFS_B_TO_FSB(mp, rsumwords << XFS_WORDLOG); > +} This helper and its users make complete sense to me and looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> > +/* > + * Compute the number of rtsummary info words needed to populate every block of > + * a summary file that is large enough to track the given rt space. > + */ > +unsigned long long > +xfs_rtsummary_wordcount( > + struct xfs_mount *mp, > + unsigned int rsumlevels, > + xfs_extlen_t rbmblocks) > +{ > + xfs_filblks_t blocks; > + > + blocks = xfs_rtsummary_blockcount(mp, rsumlevels, rbmblocks); > + return XFS_FSB_TO_B(mp, blocks) >> XFS_WORDLOG; > +} > @@ -54,8 +55,10 @@ xchk_setup_rtsummary( > * Create an xfile to construct a new rtsummary file. The xfile allows > * us to avoid pinning kernel memory for this purpose. > */ > + wordcnt = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, > + mp->m_sb.sb_rbmblocks); > descr = xchk_xfile_descr(sc, "realtime summary file"); > - error = xfile_create(descr, mp->m_rsumsize, &sc->xfile); > + error = xfile_create(descr, wordcnt << XFS_WORDLOG, &sc->xfile); > kfree(descr); But this confuses me. What problem does it solve over just using m_rsumsize?
On Thu, Oct 12, 2023 at 08:25:51AM +0200, Christoph Hellwig wrote: > On Wed, Oct 11, 2023 at 11:08:03AM -0700, Darrick J. Wong wrote: > > +/* Compute the number of rtsummary blocks needed to track the given rt space. */ > > +xfs_filblks_t > > +xfs_rtsummary_blockcount( > > + struct xfs_mount *mp, > > + unsigned int rsumlevels, > > + xfs_extlen_t rbmblocks) > > +{ > > + unsigned long long rsumwords; > > + > > + rsumwords = (unsigned long long)rsumlevels * rbmblocks; > > + return XFS_B_TO_FSB(mp, rsumwords << XFS_WORDLOG); > > +} > > This helper and its users make complete sense to me and looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > > +/* > > + * Compute the number of rtsummary info words needed to populate every block of > > + * a summary file that is large enough to track the given rt space. > > + */ > > +unsigned long long > > +xfs_rtsummary_wordcount( > > + struct xfs_mount *mp, > > + unsigned int rsumlevels, > > + xfs_extlen_t rbmblocks) > > +{ > > + xfs_filblks_t blocks; > > + > > + blocks = xfs_rtsummary_blockcount(mp, rsumlevels, rbmblocks); > > + return XFS_FSB_TO_B(mp, blocks) >> XFS_WORDLOG; > > +} > > > @@ -54,8 +55,10 @@ xchk_setup_rtsummary( > > * Create an xfile to construct a new rtsummary file. The xfile allows > > * us to avoid pinning kernel memory for this purpose. > > */ > > + wordcnt = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, > > + mp->m_sb.sb_rbmblocks); > > descr = xchk_xfile_descr(sc, "realtime summary file"); > > - error = xfile_create(descr, mp->m_rsumsize, &sc->xfile); > > + error = xfile_create(descr, wordcnt << XFS_WORDLOG, &sc->xfile); > > kfree(descr); > > But this confuses me. What problem does it solve over just using > m_rsumsize? The rtbitmap and rtsummary repair code should be computing rbmblocks and rsumsize from sb_rextents. rbmblocks = xfs_rtbitmap_wordcount(mp, mp->m_sb.sb_rextents); rsumsize = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, rbmblocks); From that, it should be checking isize and the data fork mappings of the file and the superblock values. Repair ought to map (or unmap) blocks as necessary, update isize if needed, and update the superblock if the values there are incorrect. --D
On Thu, Oct 12, 2023 at 03:18:36PM -0700, Darrick J. Wong wrote: > > > @@ -54,8 +55,10 @@ xchk_setup_rtsummary( > > > * Create an xfile to construct a new rtsummary file. The xfile allows > > > * us to avoid pinning kernel memory for this purpose. > > > */ > > > + wordcnt = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, > > > + mp->m_sb.sb_rbmblocks); > > > descr = xchk_xfile_descr(sc, "realtime summary file"); > > > - error = xfile_create(descr, mp->m_rsumsize, &sc->xfile); > > > + error = xfile_create(descr, wordcnt << XFS_WORDLOG, &sc->xfile); > > > kfree(descr); > > > > But this confuses me. What problem does it solve over just using > > m_rsumsize? > > The rtbitmap and rtsummary repair code should be computing rbmblocks and > rsumsize from sb_rextents. > > rbmblocks = xfs_rtbitmap_wordcount(mp, mp->m_sb.sb_rextents); > rsumsize = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, rbmblocks); > > >From that, it should be checking isize and the data fork mappings of > the file and the superblock values. Repair ought to map (or unmap) > blocks as necessary, update isize if needed, and update the superblock > if the values there are incorrect. So this is really a feature path that should be documented as such and not just be about adding a helper?
On Fri, Oct 13, 2023 at 06:29:47AM +0200, Christoph Hellwig wrote: > On Thu, Oct 12, 2023 at 03:18:36PM -0700, Darrick J. Wong wrote: > > > > @@ -54,8 +55,10 @@ xchk_setup_rtsummary( > > > > * Create an xfile to construct a new rtsummary file. The xfile allows > > > > * us to avoid pinning kernel memory for this purpose. > > > > */ > > > > + wordcnt = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, > > > > + mp->m_sb.sb_rbmblocks); > > > > descr = xchk_xfile_descr(sc, "realtime summary file"); > > > > - error = xfile_create(descr, mp->m_rsumsize, &sc->xfile); > > > > + error = xfile_create(descr, wordcnt << XFS_WORDLOG, &sc->xfile); > > > > kfree(descr); > > > > > > But this confuses me. What problem does it solve over just using > > > m_rsumsize? > > > > The rtbitmap and rtsummary repair code should be computing rbmblocks and > > rsumsize from sb_rextents. > > > > rbmblocks = xfs_rtbitmap_wordcount(mp, mp->m_sb.sb_rextents); > > rsumsize = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, rbmblocks); > > > > >From that, it should be checking isize and the data fork mappings of > > the file and the superblock values. Repair ought to map (or unmap) > > blocks as necessary, update isize if needed, and update the superblock > > if the values there are incorrect. > > So this is really a feature path that should be documented as such > and not just be about adding a helper? Yeah. If these rt cleanups land before online repair, then that whole hunk won't be in the patch that we merge anyway. In that case, I'll want the helpers simply to reduce the number of things I have to keep track of. (meanwhile I think the pre-rtgroups rtbitmap and rtsummary code need some fixing...) --D
diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c index b2b1a1aec3422..be5c793da46c9 100644 --- a/fs/xfs/libxfs/xfs_rtbitmap.c +++ b/fs/xfs/libxfs/xfs_rtbitmap.c @@ -1205,3 +1205,32 @@ xfs_rtbitmap_wordcount( blocks = xfs_rtbitmap_blockcount(mp, rtextents); return XFS_FSB_TO_B(mp, blocks) >> XFS_WORDLOG; } + +/* Compute the number of rtsummary blocks needed to track the given rt space. */ +xfs_filblks_t +xfs_rtsummary_blockcount( + struct xfs_mount *mp, + unsigned int rsumlevels, + xfs_extlen_t rbmblocks) +{ + unsigned long long rsumwords; + + rsumwords = (unsigned long long)rsumlevels * rbmblocks; + return XFS_B_TO_FSB(mp, rsumwords << XFS_WORDLOG); +} + +/* + * Compute the number of rtsummary info words needed to populate every block of + * a summary file that is large enough to track the given rt space. + */ +unsigned long long +xfs_rtsummary_wordcount( + struct xfs_mount *mp, + unsigned int rsumlevels, + xfs_extlen_t rbmblocks) +{ + xfs_filblks_t blocks; + + blocks = xfs_rtsummary_blockcount(mp, rsumlevels, rbmblocks); + 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 0a3c6299af8e2..a66357cf002be 100644 --- a/fs/xfs/libxfs/xfs_rtbitmap.h +++ b/fs/xfs/libxfs/xfs_rtbitmap.h @@ -270,6 +270,11 @@ xfs_rtword_t xfs_rtbitmap_getword(struct xfs_mount *mp, union xfs_rtword_ondisk *wordptr); void xfs_rtbitmap_setword(struct xfs_mount *mp, union xfs_rtword_ondisk *wordptr, xfs_rtword_t incore); + +xfs_filblks_t xfs_rtsummary_blockcount(struct xfs_mount *mp, + unsigned int rsumlevels, xfs_extlen_t rbmblocks); +unsigned long long xfs_rtsummary_wordcount(struct xfs_mount *mp, + unsigned int rsumlevels, xfs_extlen_t rbmblocks); #else /* CONFIG_XFS_RT */ # define xfs_rtfree_extent(t,b,l) (-ENOSYS) # define xfs_rtfree_blocks(t,rb,rl) (-ENOSYS) @@ -284,6 +289,8 @@ xfs_rtbitmap_blockcount(struct xfs_mount *mp, xfs_rtbxlen_t rtextents) return 0; } # define xfs_rtbitmap_wordcount(mp, r) (0) +# define xfs_rtsummary_blockcount(mp, l, b) (0) +# define xfs_rtsummary_wordcount(mp, l, b) (0) #endif /* CONFIG_XFS_RT */ #endif /* __XFS_RTBITMAP_H__ */ diff --git a/fs/xfs/scrub/rtsummary.c b/fs/xfs/scrub/rtsummary.c index 29cd26b7779d1..3fcf7de9f685f 100644 --- a/fs/xfs/scrub/rtsummary.c +++ b/fs/xfs/scrub/rtsummary.c @@ -41,6 +41,7 @@ xchk_setup_rtsummary( struct xfs_mount *mp = sc->mp; char *descr; size_t bufsize = mp->m_sb.sb_blocksize; + unsigned int wordcnt; unsigned int resblks = 0; int error; @@ -54,8 +55,10 @@ xchk_setup_rtsummary( * Create an xfile to construct a new rtsummary file. The xfile allows * us to avoid pinning kernel memory for this purpose. */ + wordcnt = xfs_rtsummary_wordcount(mp, mp->m_rsumlevels, + mp->m_sb.sb_rbmblocks); descr = xchk_xfile_descr(sc, "realtime summary file"); - error = xfile_create(descr, mp->m_rsumsize, &sc->xfile); + error = xfile_create(descr, wordcnt << XFS_WORDLOG, &sc->xfile); kfree(descr); if (error) return error; diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c index 32e3ac40e28f7..2e7857a528408 100644 --- a/fs/xfs/xfs_rtalloc.c +++ b/fs/xfs/xfs_rtalloc.c @@ -999,8 +999,7 @@ xfs_growfs_rt( nrbmblocks = xfs_rtbitmap_blockcount(mp, nrextents); nrextslog = xfs_highbit32(nrextents); nrsumlevels = nrextslog + 1; - nrsumsize = (uint)sizeof(xfs_suminfo_t) * nrsumlevels * nrbmblocks; - nrsumblocks = XFS_B_TO_FSB(mp, nrsumsize); + nrsumblocks = xfs_rtsummary_blockcount(mp, nrsumlevels, nrbmblocks); nrsumsize = XFS_FSB_TO_B(mp, nrsumblocks); /* * New summary size can't be more than half the size of @@ -1061,10 +1060,8 @@ xfs_growfs_rt( ASSERT(nsbp->sb_rextents != 0); nsbp->sb_rextslog = xfs_highbit32(nsbp->sb_rextents); nrsumlevels = nmp->m_rsumlevels = nsbp->sb_rextslog + 1; - nrsumsize = - (uint)sizeof(xfs_suminfo_t) * nrsumlevels * - nsbp->sb_rbmblocks; - nrsumblocks = XFS_B_TO_FSB(mp, nrsumsize); + nrsumblocks = xfs_rtsummary_blockcount(mp, nrsumlevels, + nsbp->sb_rbmblocks); nmp->m_rsumsize = nrsumsize = XFS_FSB_TO_B(mp, nrsumblocks); /* * Start a transaction, get the log reservation. @@ -1270,6 +1267,7 @@ xfs_rtmount_init( struct xfs_buf *bp; /* buffer for last block of subvolume */ struct xfs_sb *sbp; /* filesystem superblock copy in mount */ xfs_daddr_t d; /* address of last block of subvolume */ + unsigned int rsumblocks; int error; sbp = &mp->m_sb; @@ -1281,10 +1279,9 @@ xfs_rtmount_init( return -ENODEV; } mp->m_rsumlevels = sbp->sb_rextslog + 1; - mp->m_rsumsize = - (uint)sizeof(xfs_suminfo_t) * mp->m_rsumlevels * - sbp->sb_rbmblocks; - mp->m_rsumsize = roundup(mp->m_rsumsize, sbp->sb_blocksize); + rsumblocks = xfs_rtsummary_blockcount(mp, mp->m_rsumlevels, + mp->m_sb.sb_rbmblocks); + mp->m_rsumsize = XFS_FSB_TO_B(mp, rsumblocks); mp->m_rbmip = mp->m_rsumip = NULL; /* * Check that the realtime section is an ok size.