> +static xfs_extlen_t > +xfs_bunmapi_align( > + struct xfs_inode *ip, > + xfs_fsblock_t bno) > +{ > + struct xfs_mount *mp = ip->i_mount; > + xfs_agblock_t agbno; > + > + if (xfs_inode_has_forcealign(ip)) { > + if (is_power_of_2(ip->i_extsize)) > + return bno & (ip->i_extsize - 1); > + > + agbno = XFS_FSB_TO_AGBNO(mp, bno); > + return agbno % ip->i_extsize; > + } > + ASSERT(XFS_IS_REALTIME_INODE(ip)); > + return xfs_rtb_to_rtxoff(ip->i_mount, bno); This helper isn't really bunmapi sepcific, is it? > @@ -5425,6 +5444,7 @@ __xfs_bunmapi( > struct xfs_bmbt_irec got; /* current extent record */ > struct xfs_ifork *ifp; /* inode fork pointer */ > int isrt; /* freeing in rt area */ > + int isforcealign; /* freeing for inode with forcealign */ This is really a bool. And while it matches the code around it the code feels a bit too verbose.. > > + if ((!isrt && !isforcealign) || (flags & XFS_BMAPI_REMAP)) > goto delete; > > - mod = xfs_rtb_to_rtxoff(mp, > - del.br_startblock + del.br_blockcount); > + mod = xfs_bunmapi_align(ip, del.br_startblock + del.br_blockcount); Overly long line. We've been long wanting to split the whole align / convert unwritten / etc code into a helper outside the main bumapi flow. And when adding new logic to it this might indeed be a good time. > + if (isforcealign) { > + off = ip->i_extsize - mod; > + } else { > + ASSERT(isrt); > + off = mp->m_sb.sb_rextsize - mod; > + } And we'll really need proper helpers so that we don't have to open code the i_extsize vs sb_rextsize logic all over.