Re: [PATCH 1/2 V2] xfs: gut error handling in xfs_trans_unreserve_and_mod_sb()

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, May 21, 2020 at 07:55:30AM +1000, Dave Chinner wrote:
> On Wed, May 20, 2020 at 01:27:02PM -0700, Darrick J. Wong wrote:
> > On Wed, May 20, 2020 at 12:48:05AM -0700, Christoph Hellwig wrote:
> > > On Wed, May 20, 2020 at 05:33:58PM +1000, Dave Chinner wrote:
> > > > +	/*
> > > > +	 * Debug checks outside of the spinlock so they don't lock up the
> > > > +	 * machine if they fail.
> > > > +	 */
> > > > +	ASSERT(mp->m_sb.sb_frextents >= 0);
> > > > +	ASSERT(mp->m_sb.sb_dblocks >= 0);
> > > > +	ASSERT(mp->m_sb.sb_agcount >= 0);
> > > > +	ASSERT(mp->m_sb.sb_imax_pct >= 0);
> > > > +	ASSERT(mp->m_sb.sb_rextsize >= 0);
> > > > +	ASSERT(mp->m_sb.sb_rbmblocks >= 0);
> > > > +	ASSERT(mp->m_sb.sb_rblocks >= 0);
> > > > +	ASSERT(mp->m_sb.sb_rextents >= 0);
> > > > +	ASSERT(mp->m_sb.sb_rextslog >= 0);
> > 
> > Except for imax_pct and rextslog, all of these are unsigned quantities,
> > right?  So the asserts will /never/ trigger?
> 
> In truth, I didn't look that far. I just assumed that because all
> the xfs_sb_mod*() functions used signed math that they could all
> underflow/overflow.  IOWs, the checking for overflow/underflow was
> completely wrong in the first place.
> 
> Should I just remove the ASSERT()s entirely?

It causes a bunch of gcc 9.3 warnings, so yes please. :)

(Granted, I ripped out all the asserts except for the two I mentioned
above, so if nobody else have complaints then no need to resend.)

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com