All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 0/4] xfs: properly invalidate cached writeback mapping
Date: Fri, 11 Jan 2019 07:30:28 -0500	[thread overview]
Message-ID: <20190111123032.31538-1-bfoster@redhat.com> (raw)

Hi all,

This series attempts to fix the stale writepage mapping problem in XFS.
The problem is essentially that ->writepages() caches the current extent
across multiple writepage instances and in certain circumstances the
cached mapping can be made invalid by concurrent filesystem operations.
For example, even with the current EOF trim band-aid for dealing with
post-eof speculative preallocation, a truncate+append sequence that
happens to race with background writeback can lead to a writepage to an
incorrect location.

Since we already have an xfs_ifork change/sequence number mechanism in
place, we reuse that to invalidate cached writeback mappings any time
the associated data fork has changed. Note that while certain workloads
might lead to a high frequency of spurious invalidations (i.e.,
with allocsize=4k mounts, files with a predetermined size such as vdisk
images, etc.), I've not been able to reproduce any noticeable effects at
a user level. See the patch 3 commit log description for further
discussion.

If we do run into use cases and workloads for which this is a problem, I
think there are options to further restrict seqno changing events (or
use multiple counters for subsets of change events) for less frequent
invalidations. For example, a sequence count that only tracks block
removals may still be sufficient to preserve coherency of cached
writeback mappings. Since this is all handwavy and theoretical, I opted
to keep the code simple and only deal with this should the need arise.

Patch 1 is a stable fix for the initial EOF trim patch. Patches 2-4
tweak the fork seqno mechanism to work for data forks, use it to
invalidate the cached writeback map and remove the EOF trim mechanism.
This has been tested via xfstests on multiple FSB sizes and fsx without
any explosions.

Thoughts, reviews, flames appreciated.

Brian

Brian Foster (4):
  xfs: eof trim writeback mapping as soon as it is cached
  xfs: update fork seq counter on data fork changes
  xfs: validate writeback mapping using data fork seq counter
  xfs: remove superfluous writeback mapping eof trimming

 fs/xfs/libxfs/xfs_bmap.c       | 11 -----------
 fs/xfs/libxfs/xfs_bmap.h       |  1 -
 fs/xfs/libxfs/xfs_iext_tree.c  | 13 ++++++-------
 fs/xfs/libxfs/xfs_inode_fork.h |  2 +-
 fs/xfs/xfs_aops.c              | 21 ++++++---------------
 fs/xfs/xfs_iomap.c             |  4 ++--
 6 files changed, 15 insertions(+), 37 deletions(-)

-- 
2.17.2

             reply	other threads:[~2019-01-11 12:30 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-11 12:30 Brian Foster [this message]
2019-01-11 12:30 ` [PATCH 1/4] xfs: eof trim writeback mapping as soon as it is cached Brian Foster
2019-01-16 13:35   ` Sasha Levin
2019-01-16 13:35     ` Sasha Levin
2019-01-16 14:10     ` Brian Foster
2019-01-11 12:30 ` [PATCH 2/4] xfs: update fork seq counter on data fork changes Brian Foster
2019-01-17 14:41   ` Christoph Hellwig
2019-01-11 12:30 ` [PATCH 3/4] xfs: validate writeback mapping using data fork seq counter Brian Foster
2019-01-13 21:49   ` Dave Chinner
2019-01-14 15:34     ` Brian Foster
2019-01-14 20:57       ` Dave Chinner
2019-01-15 11:26         ` Brian Foster
2019-01-17 14:47       ` Christoph Hellwig
2019-01-17 16:35         ` Brian Foster
2019-01-17 16:41           ` Christoph Hellwig
2019-01-17 17:53             ` Brian Foster
2019-01-11 12:30 ` [PATCH 4/4] xfs: remove superfluous writeback mapping eof trimming Brian Foster
2019-01-11 13:31 ` [PATCH] tests/generic: test writepage cached mapping validity Brian Foster
2019-01-14  9:30   ` Eryu Guan
2019-01-14 15:34     ` Brian Foster
2019-01-15  3:52     ` Dave Chinner

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=20190111123032.31538-1-bfoster@redhat.com \
    --to=bfoster@redhat.com \
    --cc=linux-xfs@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.