All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Brian Foster <bfoster@redhat.com>
Cc: linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v3 2/4] iomap: lift zeroed mapping handling into iomap_zero_range()
Date: Sun, 10 Nov 2024 22:03:44 -0800	[thread overview]
Message-ID: <ZzGeQGl9zvQLkRfZ@infradead.org> (raw)
In-Reply-To: <20241108124246.198489-3-bfoster@redhat.com>

On Fri, Nov 08, 2024 at 07:42:44AM -0500, Brian Foster wrote:
> In preparation for special handling of subranges, lift the zeroed
> mapping logic from the iterator into the caller.

What's that special code?  I don't really see anything added to this
in the new code?  In general I would prefer if all code for the
iteration would be kept in a single function in preparation for
unrolling these loops.  If you want to keep this code separate
from the write zeroes logic (which seems like a good idea) please
just just move the actual real zeroing out of iomap_zero_iter into
a separate helper similar to how we e.g. have multiple different
implementations in the dio iterator.

> +	while ((ret = iomap_iter(&iter, ops)) > 0) {
> +		const struct iomap *s = iomap_iter_srcmap(&iter);
> +
> +		if (s->type == IOMAP_HOLE || s->type == IOMAP_UNWRITTEN) {
> +			loff_t p = iomap_length(&iter);

Also please stick to variable names that are readable and preferably
the same as in the surrounding code, e.g. s -> srcmap p -> pos.


  parent reply	other threads:[~2024-11-11  6:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-08 12:42 [PATCH v3 0/4] iomap: zero range flush fixes Brian Foster
2024-11-08 12:42 ` [PATCH v3 1/4] iomap: reset per-iter state on non-error iter advances Brian Foster
2024-11-09  3:00   ` Darrick J. Wong
2024-11-11  5:53   ` Christoph Hellwig
2024-11-12 13:59     ` Brian Foster
2024-11-08 12:42 ` [PATCH v3 2/4] iomap: lift zeroed mapping handling into iomap_zero_range() Brian Foster
2024-11-09  3:01   ` Darrick J. Wong
2024-11-12 13:59     ` Brian Foster
2024-11-11  6:03   ` Christoph Hellwig [this message]
2024-11-12 14:00     ` Brian Foster
2024-11-15 14:53       ` Brian Foster
2024-11-15 17:02         ` Darrick J. Wong
2024-11-15 19:31           ` Brian Foster
2024-11-08 12:42 ` [PATCH v3 3/4] iomap: elide flush from partial eof zero range Brian Foster
2024-11-09  3:03   ` Darrick J. Wong
2024-11-11  6:06   ` Christoph Hellwig
2024-11-08 12:42 ` [PATCH v3 4/4] iomap: warn on zero range of a post-eof folio Brian Foster
2024-11-09  3:06   ` Darrick J. Wong
2024-11-12 14:01     ` Brian Foster
2024-11-11  6:06   ` Christoph Hellwig

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=ZzGeQGl9zvQLkRfZ@infradead.org \
    --to=hch@infradead.org \
    --cc=bfoster@redhat.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.