From: Christoph Hellwig <hch@lst.de>
To: Brian Foster <bfoster@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>,
Christoph Hellwig <hch@lst.de>,
linux-xfs@vger.kernel.org
Subject: Re: [PATCH] xfs: pass a commit_mode to xfs_trans_commit
Date: Fri, 1 May 2020 12:42:45 +0200 [thread overview]
Message-ID: <20200501104245.GA28237@lst.de> (raw)
In-Reply-To: <20200501102403.GA37819@bfoster>
On Fri, May 01, 2020 at 06:24:03AM -0400, Brian Foster wrote:
> I recall looking at this when it was first posted and my first reaction
> was that I didn't really like the interface. I decided to think about it
> to see if it grew on me and then just lost track (sorry). It's not so
> much passing a flag to commit as opposed to the flags not directly
> controlling behavior (i.e., one flag means sync if <something> is true,
> another flag means sync if <something else> is true, etc.) tends to
> confuse me. I don't feel terribly strongly about it if others prefer
> this pattern, but I still find the existing code more readable.
>
> I vaguely recall thinking it might be nice if we could dump this into
> transaction state to avoid the aforementioned logic warts, but IIRC that
> might not have been possible for all users of this functionality..
Moving the flag out of the transaction structure was the main motivation
for this series - the fact that we need different arguments to
xfs_trans_commit is just a fallout from that. The rationale is that
I found it highly confusing to figure out how and where we set the sync
flag vs having it obvious in the one place where we commit the
transaction.
next prev parent reply other threads:[~2020-05-01 10:42 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-09 7:36 [PATCH] xfs: pass a commit_mode to xfs_trans_commit Christoph Hellwig
2020-05-01 8:07 ` Christoph Hellwig
2020-05-01 10:24 ` Brian Foster
2020-05-01 10:42 ` Christoph Hellwig [this message]
2020-05-01 11:51 ` Brian Foster
2020-05-01 15:53 ` Darrick J. Wong
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=20200501104245.GA28237@lst.de \
--to=hch@lst.de \
--cc=bfoster@redhat.com \
--cc=hch@infradead.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox