From: Dave Chinner <firstname.lastname@example.org> To: Brian Foster <email@example.com> Cc: firstname.lastname@example.org, email@example.com, Christoph Hellwig <firstname.lastname@example.org> Subject: Re: [PATCH] iomap: set page dirty after partial delalloc on mkwrite Date: Mon, 1 Oct 2018 10:31:31 +1000 [thread overview] Message-ID: <20181001003131.GN31060@dastard> (raw) In-Reply-To: <email@example.com> On Fri, Sep 28, 2018 at 01:39:56PM -0400, Brian Foster wrote: > The iomap page fault mechanism currently dirties the associated page > after the full block range of the page has been allocated. This > leaves the page susceptible to delayed allocations without ever > being set dirty on sub-page block sized filesystems. > > For example, consider a page fault on a page with one preexisting > real (non-delalloc) block allocated in the middle of the page. The > first iomap_apply() iteration performs delayed allocation on the > range up to the preexisting block, the next iteration finds the > preexisting block, and the last iteration attempts to perform > delayed allocation on the range after the prexisting block to the > end of the page. If the first allocation succeeds and the final > allocation fails with -ENOSPC, iomap_apply() returns the error and > iomap_page_mkwrite() fails to dirty the page having already > performed partial delayed allocation. This eventually results in the > page being invalidated without ever converting the delayed > allocation to real blocks. > > This problem is reliably reproduced by generic/083 on XFS on ppc64 > systems (64k page size, 4k block size). It results in leaked > delalloc blocks on inode reclaim, which triggers an assert failure > in xfs_fs_destroy_inode() and filesystem accounting inconsistency. > > Move the set_page_dirty() call from iomap_page_mkwrite() to the > actor callback, similar to how the buffer head implementation works. > The actor callback is called iff ->iomap_begin() returns success, so > ensures the page is dirtied as soon as possible after an allocation. > > Signed-off-by: Brian Foster <firstname.lastname@example.org> Looks sensible to me. Reviewed-by: Dave Chinner <email@example.com> > --- > > This patch addresses the problem with generic/083, but I'm still in the > process of running broader testing. I wanted to get it on the list for > review in the meantime... It's been fine on my weekend test runs in the my candidate fixes for 4.19 branch. There have been no regressions for v4 512b and v5 1k block size test runs from it - I got 14 complete fstests runs of sub page block size configs across multiple test machines over the weekend, so I've pushed it with that branch. -Dave. -- Dave Chinner firstname.lastname@example.org
prev parent reply other threads:[~2018-10-01 7:06 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-09-28 17:39 Brian Foster 2018-09-30 22:44 ` Christoph Hellwig 2018-10-01 10:54 ` Brian Foster 2018-10-01 0:31 ` Dave Chinner [this message]
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=20181001003131.GN31060@dastard \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH] iomap: set page dirty after partial delalloc on mkwrite' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).