Re: [PATCH 2/2] iomap: make zero range flush conditional on unwritten mappings

Christoph Hellwig
2024-08-29T05:41:57Z

On Wed, Aug 28, 2024 at 08:35:47AM -0400, Brian Foster wrote:
> Yeah, it was buried in a separate review around potentially killing off
> iomap_truncate_page():
>
> https://lore.kernel.org/linux-fsdevel/ZlxUpYvb9dlOHFR3@bfoster/
>
> The idea is pretty simple.. use the same kind of check this patch does
> for doing a flush, but instead open code and isolate it to
> iomap_truncate_page() so we can just default to doing the buffered write
> instead.
>
> Note that I don't think this replaces the need for patch 1, but it might
> arguably make further optimization of the flush kind of pointless
> because I'm not sure zero range would ever be called from somewhere that
> doesn't flush already.
>
> The tradeoffs I can think of are this might introduce some false
> positives where an EOF folio might be dirty but a sub-folio size block
> backing EOF might be clean, and again that callers like truncate and
> write extension would need to both truncate the eof page and zero the
> broader post-eof range. Neither of those seem all that significant to
> me, but just my .02.

Looking at that patch and your current series I kinda like not having
to deal with the dirty caches in the loop, and in fact I'd also prefer
to not do any writeback from the low-level zero helpers if we can.
That is not doing your patch 1 but instead auditing the callers if
any of them needs them and documenting the expectation.

But please let Dave and Darrick chime in first before investing any
work into this.