On 06/07/2024 08:58, Christoph Hellwig wrote:
+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?
Right, it is not really. Apart from the ASSERT to ensure that we are not
calling from a stray context.
@@ -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..
I can change both to a bool - would that be better?
Using isfa (instead of isforcealign) might be interpreted as something
else :)
+ 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.
noted
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.
ok, I'll see if can come up with something
+ 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.
sure
