From: Brian Foster <bfoster@redhat.com>
To: xfs@oss.sgi.com
Subject: [PATCH 3/4] xfs: writeback and inval. file range to be shifted by collapse
Date: Sun, 7 Sep 2014 08:25:59 -0400 [thread overview]
Message-ID: <1410092760-3451-4-git-send-email-bfoster@redhat.com> (raw)
In-Reply-To: <1410092760-3451-1-git-send-email-bfoster@redhat.com>
The collapse range operation currently writes the entire file before
starting the collapse to avoid changes in the in-core extent list due to
writeback causing the extent count to change. Now that collapse range is
fsb based rather than extent index based it can sustain changes in the
extent list during the shift sequence without disruption.
Modify xfs_collapse_file_space() to writeback and invalidate pages
associated with the range of the file to be shifted.
xfs_free_file_space() currently has similar behavior, but the space free
need only affect the region of the file that is freed and this could
change in the future.
Also update the comments to reflect the current implementation. We
retain the eofblocks trim permanently as a best option for dealing with
delalloc extents. We don't shift delalloc extents because this scenario
only occurs with post-eof preallocation (since data must be flushed such
that the cache can be invalidated and data can be shifted). That means
said space must also be initialized before being shifted into the
accessible region of the file only to be immediately truncated off as
the last part of the collapse. In other words, the eofblocks trim will
happen anyways, we just run it first to ensure the file remains in a
consistent state throughout the collapse.
Finally, BUG() in the event of a delalloc extent during the extent shift
such that a failure is obvious. The implementation explicitly does not
support delalloc extents and the caller is expected to prevent this
scenario in advance as is done by collapse.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_bmap.c | 2 ++
fs/xfs/xfs_bmap_util.c | 32 +++++++++++++++++++-------------
2 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 449a016..1dd04c2 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5617,6 +5617,8 @@ xfs_bmap_shift_extents(
*/
total_extents = ifp->if_bytes / sizeof(xfs_bmbt_rec_t);
while (nexts++ < num_exts && current_ext < total_extents) {
+ /* can't handle delalloc extents */
+ BUG_ON(isnullstartblock(got.br_startblock));
startoff = got.br_startoff - offset_shift_fsb;
/* grab the left extent and check for a potential merge */
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 1e96d77..eae763f 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1470,27 +1470,33 @@ xfs_collapse_file_space(
next_fsb = XFS_B_TO_FSB(mp, offset + len);
shift_fsb = XFS_B_TO_FSB(mp, len);
- /*
- * Writeback the entire file and force remove any post-eof blocks. The
- * writeback prevents changes to the extent list via concurrent
- * writeback and the eofblocks trim prevents the extent shift algorithm
- * from running into a post-eof delalloc extent.
- *
- * XXX: This is a temporary fix until the extent shift loop below is
- * converted to use offsets and lookups within the ILOCK rather than
- * carrying around the index into the extent list for the next
- * iteration.
- */
- error = filemap_write_and_wait(VFS_I(ip)->i_mapping);
+ error = xfs_free_file_space(ip, offset, len);
if (error)
return error;
+
+ /*
+ * Trim eofblocks to avoid shifting uninitialized post-eof preallocation
+ * into the accessible region of the file.
+ */
if (xfs_can_free_eofblocks(ip, true)) {
error = xfs_free_eofblocks(mp, ip, false);
if (error)
return error;
}
- error = xfs_free_file_space(ip, offset, len);
+ /*
+ * Writeback and invalidate cache for the remainder of the file as we're
+ * about to shift down every extent from the collapse range to EOF. The
+ * free of the collapse range above might have already done some of
+ * this, but we shouldn't rely on it to do anything outside of the range
+ * that was freed.
+ */
+ error = filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
+ offset + len, -1);
+ if (error)
+ return error;
+ error = invalidate_inode_pages2_range(VFS_I(ip)->i_mapping,
+ (offset + len) >> PAGE_CACHE_SHIFT, -1);
if (error)
return error;
--
1.8.3.1
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2014-09-07 12:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-07 12:25 [PATCH 0/4] clean up collapse range and handle post-eof delalloc Brian Foster
2014-09-07 12:25 ` [PATCH 1/4] xfs: track collapse via file offset rather than extent index Brian Foster
2014-09-09 4:03 ` Dave Chinner
2014-09-07 12:25 ` [PATCH 2/4] xfs: refactor xfs_bmap_shift_extents() into multiple functions Brian Foster
2014-09-09 5:08 ` Dave Chinner
2014-09-09 15:04 ` Brian Foster
2014-09-07 12:25 ` Brian Foster [this message]
2014-09-09 5:13 ` [PATCH 3/4] xfs: writeback and inval. file range to be shifted by collapse Dave Chinner
2014-09-09 15:16 ` Brian Foster
2014-09-07 12:26 ` [PATCH 4/4] xfs: only writeback and truncate pages for the freed range Brian Foster
2014-09-09 5:14 ` 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=1410092760-3451-4-git-send-email-bfoster@redhat.com \
--to=bfoster@redhat.com \
--cc=xfs@oss.sgi.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox