From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Joanne Koong <joannelkoong@gmail.com>,
brauner@kernel.org, djwong@kernel.org,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v1] iomap: simplify iomap_iter_advance()
Date: Thu, 18 Sep 2025 16:18:29 -0400 [thread overview]
Message-ID: <aMxpFWnIDOpEWR1U@bfoster> (raw)
In-Reply-To: <aMwW0Zp2hdXfTGos@infradead.org>
On Thu, Sep 18, 2025 at 07:27:29AM -0700, Christoph Hellwig wrote:
> On Thu, Sep 18, 2025 at 07:31:33AM -0400, Brian Foster wrote:
> > IME the __iomap_iter_advance() would be the most low level and flexible
> > version, whereas the wrappers simplify things. There's also the point
> > that the wrapper seems the more common case, so maybe that makes things
> > cleaner if that one is used more often.
> >
> > But TBH I'm not sure there is strong precedent. I'm content if we can
> > retain the current variant for the callers that take advantage of it.
> > Another idea is you could rename the current function to
> > iomap_iter_advance_and_update_length_for_loopy_callers() and see what
> > alternative suggestions come up. ;)
>
> Yeah, __ names are a bit nasty. I prefer to mostly limit them to
> local helpers, or to things with an obvious inline wrapper for the
> fast path. So I your latest suggestions actually aims in the right
> directly, but maybe we can shorten the name a little and do something
> like:
>
> iomap_iter_advance_and_update_len
>
> although even that would probably lead a few lines to spill.
> iomap_iter_advance_len would be a shorter, but a little more confusing,
> but still better than __-naming, so maybe it should be fine with a good
> kerneldoc comment?
>
Ack, anything like that is fine with me, even something like
iomap_iter_advance_and_length() with a comment that just points out it
also calls iomap_length().
Another thought was to have one helper that returns the remaining length
or error and then a wrapper that translates the return (i.e. return ret
>= 0 ? 0 : ret). But when I thought more about it seemed like it just
created confusion.
Brian
next prev parent reply other threads:[~2025-09-18 20:14 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-17 0:40 [PATCH v1] iomap: simplify iomap_iter_advance() Joanne Koong
2025-09-17 13:12 ` Brian Foster
2025-09-17 21:54 ` Joanne Koong
2025-09-18 11:31 ` Brian Foster
2025-09-18 14:27 ` Christoph Hellwig
2025-09-18 20:18 ` Brian Foster [this message]
2025-09-18 22:10 ` Joanne Koong
2025-09-19 11:35 ` Brian Foster
2025-09-19 18:03 ` Joanne Koong
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=aMxpFWnIDOpEWR1U@bfoster \
--to=bfoster@redhat.com \
--cc=brauner@kernel.org \
--cc=djwong@kernel.org \
--cc=hch@infradead.org \
--cc=joannelkoong@gmail.com \
--cc=linux-fsdevel@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.