On Tue, Nov 28, 2023 at 06:04:26AM -0800, Christoph Hellwig wrote:
> > + /*
> > + * Now that we've locked the rtbitmap, we can't race with growfsrt
> > + * trying to expand the bitmap or change the size of the rt volume.
> > + * Hence it is safe to compute and check the geometry values.
> > + */
> > + rtb->rextents = xfs_rtb_to_rtx(mp, mp->m_sb.sb_rblocks);
> > + rtb->rextslog = rtb->rextents ? xfs_highbit32(rtb->rextents) : 0;
> > + rtb->rbmblocks = xfs_rtbitmap_blockcount(mp, rtb->rextents);
>
> All these will be 0 if mp->m_sb.sb_rblocks, and rtb is zeroed allocation
> right above, so calculating the values seems a bit odd. Why not simply:
>
> if (mp->m_sb.sb_rblocks) {
> rtb->rextents = xfs_rtb_to_rtx(mp, mp->m_sb.sb_rblocks);
> rtb->rextslog = xfs_highbit32(rtb->rextents);
Well... xfs_highbit32 returns -1 if its argument is zero, which is
possible for the nasty edge case of (say) a 64k block device and a
realtime extent size of 1MB, which results in rblocks > 0 and
rextents == 0.
So I'll still have to do:
if (rtb->rextents)
rtb->rextslog = xfs_highbit32()
but otherwise this is fine.
> rtb->rbmblocks = xfs_rtbitmap_blockcount(mp, rtb->rextents);
> }
>
> ?
>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
Thank you!
--D
