From: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
To: linux-xfs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org,
Christoph Hellwig <hch@infradead.org>,
"Darrick J. Wong" <djwong@kernel.org>,
Matthew Wilcox <willy@infradead.org>,
Dave Chinner <david@fromorbit.com>,
Brian Foster <bfoster@redhat.com>,
Andreas Gruenbacher <agruenba@redhat.com>,
Ojaswin Mujoo <ojaswin@linux.ibm.com>,
Disha Goel <disgoel@linux.ibm.com>
Subject: Re: [PATCHv9 0/6] iomap: Add support for per-block dirty state to improve write performance
Date: Thu, 15 Jun 2023 20:33:13 +0530 [thread overview]
Message-ID: <87h6r8wyxa.fsf@doe.com> (raw)
In-Reply-To: <cover.1686395560.git.ritesh.list@gmail.com>
"Ritesh Harjani (IBM)" <ritesh.list@gmail.com> writes:
Hello All,
So I gave some thoughts about function naming and I guess the reason we
are ping ponging between the different namings is that I am not able to
properly articulate the reasoning behind, why we chose iomap_ifs_**.
Here is my attempt to convince everyone....
In one of the previous versions of the patchsets, Christoph opposed the
idea of naming these functions with iop_** because he wanted iomap_ as a
prefix in all of these function names. Now that I gave more thought to it,
I too agree that we should have iomap_ as prefix in these APIs. Because
- fs/iomap/buffered-io.c follows that style for all other functions.
- It then also becomes easy in finding function names using ctags and
in doing grep or fuzzy searches.
Now why "ifs" in the naming because we are abbrevating iomap_folio_state
as "ifs". And since we are passing ifs as an argument in these functions
and operating upon it, hence the naming of all of these functions should
go as iomap_ifs_**.
Now if I am reading all of the emails correctly, none of the reviewers have
any strong objections with iomap_ifs_** naming style. Some of us just
started with nitpicking, but there are no strong objections, I feel.
Also I do think iomap_ifs_** naming is completely apt for these
functional changes.
So if no one has any strong objections, could we please continue with
iomap_ifs_** naming itself.
In case if someone does oppose strongly, I would humbly request to please
also convince the rest of the reviewers on why your function naming
should be chosen by giving proper reasoning (like above).
I can definitely help with making the required changes and testing them.
Does this look good and sound fair for the function naming part?
If everyone is convinced with iomap_ifs_** naming, then I will go ahead
and work on the rest of the review comments.
Thanks a lot for all the great review!
-ritesh
next prev parent reply other threads:[~2023-06-15 15:03 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
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
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 ` Ritesh Harjani [this message]
2023-06-15 16:12 ` [PATCHv9 0/6] iomap: Add support for per-block dirty state to improve write performance 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=87h6r8wyxa.fsf@doe.com \
--to=ritesh.list@gmail.com \
--cc=agruenba@redhat.com \
--cc=bfoster@redhat.com \
--cc=david@fromorbit.com \
--cc=disgoel@linux.ibm.com \
--cc=djwong@kernel.org \
--cc=hch@infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=ojaswin@linux.ibm.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