All of lore.kernel.org
 help / color / mirror / Atom feed
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


  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 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.