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@xxxxxxxxxxxxx
