> + ASSERT(new_agino == NULLAGINO || > + xfs_verify_agino(mp, XFS_INO_TO_AGNO(mp, ino), new_agino)); We have quite a few uses of this pattern. Shouldn't we just add a a xfs_verify_agino_or_null helper? Also given that the caller has the agno and we use it a few times maybe pass it as an argument? Also shouldn't new_agino be called something like next_agino or at least have a little comment explaining what it stands for? > + xfs_trans_log_buf(tp, ibp, offset, (offset + sizeof(xfs_agino_t) - 1)); No need for the innter braces. Otherwise this looks conceptually fine to me.