linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Andreas Gruenbacher <agruenba@redhat.com>
Cc: Matthew Wilcox <willy@infradead.org>,
	Ritesh Harjani <ritesh.list@gmail.com>,
	Christoph Hellwig <hch@infradead.org>,
	linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Dave Chinner <david@fromorbit.com>,
	Brian Foster <bfoster@redhat.com>,
	Ojaswin Mujoo <ojaswin@linux.ibm.com>,
	Disha Goel <disgoel@linux.ibm.com>
Subject: Re: [PATCHv9 3/6] iomap: Add some uptodate state handling helpers for ifs state bitmap
Date: Mon, 12 Jun 2023 09:10:34 -0700	[thread overview]
Message-ID: <20230612161034.GD11441@frogsfrogsfrogs> (raw)
In-Reply-To: <CAHc6FU7GnVeKmUC4GkySqE1bV3WgbA_WTuQ3D0dcMyn193M4VA@mail.gmail.com>

On Mon, Jun 12, 2023 at 05:57:48PM +0200, Andreas Gruenbacher wrote:
> On Mon, Jun 12, 2023 at 5:24 PM Matthew Wilcox <willy@infradead.org> wrote:
> > On Mon, Jun 12, 2023 at 08:48:16PM +0530, Ritesh Harjani wrote:
> > > > Since we're at the nitpicking, I don't find those names very useful,
> > > > either. How about the following instead?
> > > >
> > > > iomap_ifs_alloc -> iomap_folio_state_alloc
> > > > iomap_ifs_free -> iomap_folio_state_free
> > > > iomap_ifs_calc_range -> iomap_folio_state_calc_range
> > >
> > > First of all I think we need to get used to the name "ifs" like how we
> > > were using "iop" earlier. ifs == iomap_folio_state...
> > >
> > > >
> > > > iomap_ifs_is_fully_uptodate -> iomap_folio_is_fully_uptodate
> > > > iomap_ifs_is_block_uptodate -> iomap_block_is_uptodate
> > > > iomap_ifs_is_block_dirty -> iomap_block_is_dirty
> > > >
> > >
> > > ...if you then look above functions with _ifs_ == _iomap_folio_state_
> > > naming. It will make more sense.
> >
> > Well, it doesn't because it's iomap_iomap_folio_state_is_fully_uptodate.
> 
> Exactly.
> 
> > I don't think there's any need to namespace this so fully.
> > ifs_is_fully_uptodate() is just fine for a static function, IMO.
> 
> I'd be perfectly happy with that kind of naming scheme as well.

Ugh, /another/ round of renaming.

to_folio_state (or just folio->private)

ifs_alloc
ifs_free
ifs_calc_range

ifs_set_range_uptodate
ifs_is_fully_uptodate
ifs_block_is_uptodate

ifs_block_is_dirty
ifs_clear_range_dirty
ifs_set_range_dirty

No more renaming suggestions.  I've reached the point where my eyes and
brain have both glazed over from repeated re-reads of this series such
that I don't see the *bugs* anymore.

Anyone else wanting new naming gets to *send in their own patch*.
Please focus only on finding code defects or friction between iomap and
some other subsystem.

Flame away about my aggressive tone,

~Your burned out and pissed off maintainer~

> Thanks,
> Andreas
> 

  reply	other threads:[~2023-06-12 16:10 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-10 11:39 [PATCHv9 0/6] iomap: Add support for per-block dirty state to improve write performance Ritesh Harjani (IBM)
2023-06-10 11:39 ` [PATCHv9 1/6] iomap: Rename iomap_page to iomap_folio_state and others Ritesh Harjani (IBM)
2023-06-12  6:21   ` Christoph Hellwig
2023-06-12  6:23     ` Christoph Hellwig
2023-06-12  9:19       ` Ritesh Harjani
2023-06-12 15:05         ` Darrick J. Wong
2023-06-12 15:08           ` Matthew Wilcox
2023-06-12 15:59             ` Darrick J. Wong
2023-06-12 17:43               ` Ritesh Harjani
2023-06-12 17:54                 ` Matthew Wilcox
2023-06-13  5:05             ` Christoph Hellwig
2023-06-10 11:39 ` [PATCHv9 2/6] iomap: Drop ifs argument from iomap_set_range_uptodate() Ritesh Harjani (IBM)
2023-06-12  6:24   ` Christoph Hellwig
2023-06-10 11:39 ` [PATCHv9 3/6] iomap: Add some uptodate state handling helpers for ifs state bitmap Ritesh Harjani (IBM)
2023-06-12  6:25   ` Christoph Hellwig
2023-06-12  9:14     ` Ritesh Harjani
2023-06-12 12:54     ` Andreas Gruenbacher
2023-06-12 15:18       ` Ritesh Harjani
2023-06-12 15:24         ` Matthew Wilcox
2023-06-12 15:33           ` Ritesh Harjani
2023-06-12 15:57           ` Andreas Gruenbacher
2023-06-12 16:10             ` Darrick J. Wong [this message]
2023-06-12 17:54               ` Ritesh Harjani
2023-06-12 12:40   ` Andreas Gruenbacher
2023-06-12 15:30     ` Ritesh Harjani
2023-06-12 16:14       ` Andreas Grünbacher
2023-06-12 16:16       ` Darrick J. Wong
2023-06-12 16:19         ` Andreas Gruenbacher
2023-06-12 17:57         ` Ritesh Harjani
2023-06-10 11:39 ` [PATCHv9 4/6] iomap: Refactor iomap_write_delalloc_punch() function out Ritesh Harjani (IBM)
2023-06-12  6:25   ` Christoph Hellwig
2023-06-12  9:01     ` Ritesh Harjani
2023-06-12 13:22   ` Matthew Wilcox
2023-06-12 14:03     ` Ritesh Harjani
2023-06-12 14:19       ` Matthew Wilcox
     [not found]   ` <CGME20230612135700eucas1p2269a4e8cc8f5f47186ea3e7e575430df@eucas1p2.samsung.com>
2023-06-12 13:56     ` Pankaj Raghav
2023-06-12 14:55       ` Ritesh Harjani
2023-06-10 11:39 ` [PATCHv9 5/6] iomap: Allocate ifs in ->write_begin() early Ritesh Harjani (IBM)
2023-06-10 11:39 ` [PATCHv9 6/6] iomap: Add per-block dirty state tracking to improve performance Ritesh Harjani (IBM)
2023-06-12  6:30   ` Christoph Hellwig
2023-06-12  9:00     ` Ritesh Harjani
2023-06-12 16:27   ` Matthew Wilcox
2023-06-15 15:03 ` [PATCHv9 0/6] iomap: Add support for per-block dirty state to improve write performance Ritesh Harjani
2023-06-15 16:12   ` Ritesh Harjani

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=20230612161034.GD11441@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=agruenba@redhat.com \
    --cc=bfoster@redhat.com \
    --cc=david@fromorbit.com \
    --cc=disgoel@linux.ibm.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=ojaswin@linux.ibm.com \
    --cc=ritesh.list@gmail.com \
    --cc=willy@infradead.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;
as well as URLs for NNTP newsgroup(s).