Re: [PATCH v3 1/3] xfs: don't unlock invalidated buf on aborted tx commit

[Dave Chinner <david@fromorbit.com>]

On Sat, Sep 01, 2018 at 04:05:02AM -0700, Christoph Hellwig wrote:
> On Fri, Aug 31, 2018 at 09:21:19AM -0400, Brian Foster wrote:
> > > > +	} else if (stale) {
> > > > +		/*
> > > > +		 * Stale buffers remain locked until final unpin unless the bli
> > > > +		 * was freed in the branch above. A freed stale bli implies an
> > > > +		 * abort because buffer invalidation dirties the bli and
> > > > +		 * transaction.
> > > > +		 */
> > > > +		ASSERT(!freed);
> > > 
> > > This assert doesn't make sense as we're already in the else statement
> > > of the 'if (freed) check.
> > > 
> > 
> > It was intended to be defensive. I actually considered 'else if (freed
> > && stale)' so the code was more clear, but settled on this (which is
> > eventually replaced).
> 
> I see you've resend it, but I still object to an ASSERT(!freed) in
> an
> 
> 	if (freed) {
> 		..
> 	} else if (stale) {
> 		ASSEET(!freed);
> 	}
> 
> statement.  This isn't defensive but just bogus logic.

I'll drop it on commit.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com