All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Brian Foster <bfoster@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH v3 1/3] xfs: don't unlock invalidated buf on aborted tx commit
Date: Sat, 1 Sep 2018 04:05:02 -0700	[thread overview]
Message-ID: <20180901110502.GA29326@infradead.org> (raw)
In-Reply-To: <20180831132119.GB39825@bfoster>

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.

  reply	other threads:[~2018-09-01 15:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-29 17:26 [PATCH v3 0/3] xfs: bli refcount fixups Brian Foster
2018-08-29 17:26 ` [PATCH v3 1/3] xfs: don't unlock invalidated buf on aborted tx commit Brian Foster
2018-08-31  7:24   ` Christoph Hellwig
2018-08-31 13:21     ` Brian Foster
2018-09-01 11:05       ` Christoph Hellwig [this message]
2018-09-03  0:18         ` Dave Chinner
2018-08-29 17:26 ` [PATCH v3 2/3] xfs: clean up xfs_trans_brelse() Brian Foster
2018-08-29 17:26 ` [PATCH v3 3/3] xfs: refactor xfs_buf_log_item reference count handling Brian Foster
2018-08-31  7:30   ` Christoph Hellwig
2018-08-31 13:22     ` Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180901110502.GA29326@infradead.org \
    --to=hch@infradead.org \
    --cc=bfoster@redhat.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.