On Mon, Jun 28, 2021 at 07:27:25PM +0200, Andreas Gruenbacher wrote:

(1) In iomap_readpage_actor, an iomap_page is attached to the page even
for inline inodes.  This is unnecessary because inline inodes don't need
iomap_page objects.  That alone wouldn't cause any real issues, but when
iomap_read_inline_data copies the inline data into the page, it sets the
PageUptodate flag without setting iop->uptodate, causing an
inconsistency between the two.  This will trigger a WARN_ON in
iomap_page_release.  The fix should be not to allocate iomap_page
objects when reading from inline inodes (patch 1).

I don't have a problem with this patch.

(2) When un-inlining an inode, we must allocate a page with an attached
iomap_page object (iomap_page_create) and initialize the iop->uptodate
bitmap (iomap_set_range_uptodate).  We can't currently do that because
iomap_page_create and iomap_set_range_uptodate are not exported.  That
could be fixed by exporting those functions, or by implementing an
additional helper as in patch 2.  Which of the two would you prefer?

Not hugely happy with either of these options, tbh.  I'd rather we apply
a patch akin to this one (plucked from the folio tree), so won't apply:

@@ -1305,7 +1311,7 @@ iomap_writepage_map(struct iomap_writepage_ctx *wpc,
                struct writeback_control *wbc, struct inode *inode,
                struct folio *folio, loff_t end_pos)
 {
-       struct iomap_page *iop = to_iomap_page(folio);
+       struct iomap_page *iop = iomap_page_create(inode, folio);
        struct iomap_ioend *ioend, *next;
        unsigned len = i_blocksize(inode);
        unsigned nblocks = i_blocks_per_folio(inode, folio);

@@ -1313,7 +1319,6 @@ iomap_writepage_map(struct iomap_writepage_ctx *wpc,
        int error = 0, count = 0, i;
        LIST_HEAD(submit_list);

-       WARN_ON_ONCE(nblocks > 1 && !iop);
        WARN_ON_ONCE(iop && atomic_read(&iop->write_bytes_pending) != 0);

        /*

so permit pages without an iop to enter writeback and create an iop
*then*.  Would that solve your problem?

(3) We're not yet using iomap_page_mkwrite, so iomap_page objects don't
get created on .page_mkwrite, either.  Part of the reason is that
iomap_page_mkwrite locks the page and then calls into the filesystem for
uninlining and for allocating backing blocks.  This conflicts with the
gfs2 locking order: on gfs2, transactions must be started before locking
any pages.  We can fix that by calling iomap_page_create from
gfs2_page_mkwrite, or by doing the uninlining and allocations before
calling iomap_page_mkwrite.  I've implemented option 2 for now; see
here:

I think this might also solve this problem?