From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Chandan Babu R <chandan.babu@oracle.com>,
"Darrick J. Wong" <djwong@kernel.org>,
linux-xfs@vger.kernel.org
Subject: Re: [PATCH 7/7] xfs: split xfs_trans_mod_sb
Date: Fri, 11 Oct 2024 10:05:33 -0400 [thread overview]
Message-ID: <ZwkwrTajIqYz2Ykw@bfoster> (raw)
In-Reply-To: <20241011075408.GB2749@lst.de>
On Fri, Oct 11, 2024 at 09:54:08AM +0200, Christoph Hellwig wrote:
> On Thu, Oct 10, 2024 at 10:06:15AM -0400, Brian Foster wrote:
> > Seems Ok, but not sure I see the point personally. Rather than a single
> > helper with flags, we have multiple helpers, some of which still mix
> > deltas via an incrementally harder to read boolean param. This seems
> > sort of arbitrary to me. Is this to support some future work?
>
> I just find these multiplexers that have no common logic very confusing.
>
> And yes, I also have some changes to share more logic between the
> delalloc vs non-delalloc block accounting.
>
I'm not sure what you mean by no common logic. The original
trans_mod_sb() is basically a big switch statement for modifying the
appropriate transaction delta associated with a superblock field. That
seems logical to me.
Just to be clear, I don't really feel strongly about this one way or the
other. I don't object and I don't think it makes anything worse, and
it's less of a change if half this stuff goes away anyways by changing
how the sb is logged. But I also think sometimes code seems more clear
moreso because we go through the process of refactoring it (i.e.
familiarity bias) over what the code ultimately looks like.
*shrug* This is all subjective, I'm sure there are other opinions.
Brian
next prev parent reply other threads:[~2024-10-11 14:04 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-30 16:41 fix recovery of allocator ops after a growfs Christoph Hellwig
2024-09-30 16:41 ` [PATCH 1/7] xfs: pass the exact range to initialize to xfs_initialize_perag Christoph Hellwig
2024-10-10 14:02 ` Brian Foster
2024-10-11 7:53 ` Christoph Hellwig
2024-10-11 14:01 ` Brian Foster
2024-09-30 16:41 ` [PATCH 2/7] xfs: merge the perag freeing helpers Christoph Hellwig
2024-10-10 14:02 ` Brian Foster
2024-09-30 16:41 ` [PATCH 3/7] xfs: update the file system geometry after recoverying superblock buffers Christoph Hellwig
2024-09-30 16:50 ` Darrick J. Wong
2024-10-01 8:49 ` Christoph Hellwig
2024-10-10 16:02 ` Darrick J. Wong
2024-10-10 14:03 ` Brian Foster
2024-09-30 16:41 ` [PATCH 4/7] xfs: error out when a superblock buffer updates reduces the agcount Christoph Hellwig
2024-09-30 16:51 ` Darrick J. Wong
2024-10-01 8:47 ` Christoph Hellwig
2024-10-10 14:04 ` Brian Foster
2024-09-30 16:41 ` [PATCH 5/7] xfs: don't use __GFP_RETRY_MAYFAIL in xfs_initialize_perag Christoph Hellwig
2024-10-10 14:04 ` Brian Foster
2024-09-30 16:41 ` [PATCH 6/7] xfs: don't update file system geometry through transaction deltas Christoph Hellwig
2024-10-10 14:05 ` Brian Foster
2024-10-11 7:57 ` Christoph Hellwig
2024-10-11 14:02 ` Brian Foster
2024-10-11 17:13 ` Darrick J. Wong
2024-10-11 18:41 ` Brian Foster
2024-10-11 23:12 ` Darrick J. Wong
2024-10-11 23:29 ` Darrick J. Wong
2024-10-14 5:58 ` Christoph Hellwig
2024-10-14 15:30 ` Darrick J. Wong
2024-10-14 18:50 ` Brian Foster
2024-10-15 16:42 ` Darrick J. Wong
2024-10-18 12:27 ` Brian Foster
2024-10-21 16:59 ` Darrick J. Wong
2024-10-23 14:45 ` Brian Foster
2024-10-24 18:02 ` Darrick J. Wong
2024-10-21 13:38 ` Dave Chinner
2024-10-23 15:06 ` Brian Foster
2024-10-10 19:01 ` Darrick J. Wong
2024-10-11 7:59 ` Christoph Hellwig
2024-10-11 16:44 ` Darrick J. Wong
2024-09-30 16:41 ` [PATCH 7/7] xfs: split xfs_trans_mod_sb Christoph Hellwig
2024-10-10 14:06 ` Brian Foster
2024-10-11 7:54 ` Christoph Hellwig
2024-10-11 14:05 ` Brian Foster [this message]
2024-10-11 16:50 ` 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=ZwkwrTajIqYz2Ykw@bfoster \
--to=bfoster@redhat.com \
--cc=chandan.babu@oracle.com \
--cc=djwong@kernel.org \
--cc=hch@lst.de \
--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