On Wed, Nov 20, 2019 at 08:50:03PM -0800, Darrick J. Wong wrote: > On Thu, Nov 21, 2019 at 03:00:23PM +1100, Dave Chinner wrote: > > On Wed, Nov 20, 2019 at 06:38:36PM -0800, Darrick J. Wong wrote: > > > On Thu, Nov 21, 2019 at 11:44:37AM +1100, Dave Chinner wrote: > > > > -out_undo_frextents: > > > > - if (rtxdelta) > > > > - xfs_sb_mod64(&mp->m_sb.sb_frextents, -rtxdelta); > > > > -out_undo_ifree: > > > > + xfs_sb_mod64(&mp->m_sb.sb_frextents, rtxdelta); > > > > > > As for these bits... why even bother with a three line helper? I think > > > this is clearer about what's going on: > > > > > > mp->m_sb.sb_frextents += rtxdelta; > > > mp->m_sb.sb_dblocks += tp->t_dblocks_delta; > > > ... > > > ASSERT(!rtxdelta || mp->m_sb.sb_frextents >= 0); > > > ASSERT(!tp->t_dblocks_delta || mp->m_sb.sb.dblocks >= 0); > > > > That required writing more code and adding more logic I'd have to > > think about to write, and then think about again every time I read > > it. > > OTOH it's an opportunity to make the asserts more useful, because right > now they just say: > > XFS (sda): Assertion failed: counter >= 0, file: xfs_trans.c, line XXX > > *Which* counter just tripped the assert? At least it could say: > > XFS (sda): Assertion failed: mp->m_sb.sb_dblocks >= 0, file: xfs_trans.c, line XXX Ok, that's a decent reason to make the code a bit more complex. I'll see what I can do.... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
