Re: [PATCH 5/7] iomap: write iomap validity checks

Christoph Hellwig
2022-11-02T08:36:45Z

On Tue, Nov 01, 2022 at 11:34:10AM +1100, Dave Chinner wrote:
> + /*
> + * Now we have a locked folio, before we do anything with it we need to
> + * check that the iomap we have cached is not stale. The inode extent
> + * mapping can change due to concurrent IO in flight (e.g.
> + * IOMAP_UNWRITTEN state can change and memory reclaim could have
> + * reclaimed a previously partially written page at this index after IO
> + * completion before this write reaches this file offset) and hence we
> + * could do the wrong thing here (zero a page range incorrectly or fail
> + * to zero) and corrupt data.
> + */
> + if (ops->iomap_valid) {
> + bool iomap_valid = ops->iomap_valid(iter->inode, &iter->iomap);
> +
> + if (!iomap_valid) {
> + iter->iomap.flags |= IOMAP_F_STALE;
> + status = 0;
> + goto out_unlock;
> + }
> + }

So the design so far has been that everything that applies at a page (or
now folio) level goes into iomap_page_ops, not iomap_ops which is just
the generic iteration, and I think we should probably do it that way.

I'm a little disappointed that we need two callout almost next to each
other, but given that we need to validate with the folio locked, and
gfs2 wants the callback with the folio unlocked I think we have to do
it that.