All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: [PATCH] btrfs: clear file extent mapping for punch past i_size
Date: Thu, 20 Feb 2020 09:29:47 -0500	[thread overview]
Message-ID: <20200220142947.3880392-1-josef@toxicpanda.com> (raw)

In my stress testing we were still seeing some hole's with my patches to
fix missing hole extents.  Turns out we do not fill in holes during hole
punch if the punch is past i_size.  I incorrectly assumed this was fine,
because anybody extending would use btrfs_cont_expand, however there is
a corner that still can give us trouble.  Start with an empty file and

fallocate KEEP_SIZE 1m-2m

We now have a 0 length file, and a hole file extent from 0-1m, and a
prealloc extent from 1m-2m.  Now

punch 1m-1.5m

Because this is past i_size we have

[HOLE EXTENT][ NOTHING ][PREALLOC]
[0        1m][1m   1.5m][1.5m  2m]

with an i_size of 0.  Now if we pwrite 0-1.5m we'll increas our i_size
to 1.5m, but our disk_i_size is still 0 until the ordered extent
completes.

However if we now immediately truncate 2m on the file we'll just call
btrfs_cont_expand(inode, 1.5m, 2m), since our old i_size is 1.5m.  If we
commit the transaction here and crash we'll expose the gap.

To fix this we need to clear the file extent mapping for the range that
we punched but didn't insert a corresponding file extent for.  This will
mean the truncate will only get an disk_i_size set to 1m if we crash
before the finish ordered io happens.

I've written an xfstest to reproduce the problem and validate this fix.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
- Dave, this needs to be folded into "btrfs: use the file extent tree
  infrastructure" and the changelog needs to be adjusted since I incorrectly
  point out that we don't need to clear for hole punch.  We definitely need to
  clear for the case that we're punching past i_size as we aren't inserting hole
  file extents.

 fs/btrfs/file.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 5cdcdbdd908b..6f6f1805e6fd 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -2597,6 +2597,24 @@ int btrfs_punch_hole_range(struct inode *inode, struct btrfs_path *path,
 				btrfs_abort_transaction(trans, ret);
 				break;
 			}
+		} else if (!clone_info && cur_offset < drop_end) {
+			/*
+			 * We are past the i_size here, but since we didn't
+			 * insert holes we need to clear the mapped area so we
+			 * know to not set disk_i_size in this area until a new
+			 * file extent is inserted here.
+			 */
+			ret = btrfs_inode_clear_file_extent_range(BTRFS_I(inode),
+					cur_offset, drop_end - cur_offset);
+			if (ret) {
+				/*
+				 * We couldn't clear our area, so we could
+				 * presumably adjust up and corrupt the fs, so
+				 * we need to abort.
+				 */
+				btrfs_abort_transaction(trans, ret);
+				break;
+			}
 		}
 
 		if (clone_info && drop_end > clone_info->file_offset) {
@@ -2687,6 +2705,15 @@ int btrfs_punch_hole_range(struct inode *inode, struct btrfs_path *path,
 			btrfs_abort_transaction(trans, ret);
 			goto out_trans;
 		}
+	} else if (!clone_info && cur_offset < drop_end) {
+		/* See the comment in the loop above for the reasoning here. */
+		ret = btrfs_inode_clear_file_extent_range(BTRFS_I(inode),
+					cur_offset, drop_end - cur_offset);
+		if (ret) {
+			btrfs_abort_transaction(trans, ret);
+			goto out_trans;
+		}
+
 	}
 	if (clone_info) {
 		ret = btrfs_insert_clone_extent(trans, inode, path, clone_info,
-- 
2.24.1


             reply	other threads:[~2020-02-20 14:29 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-20 14:29 Josef Bacik [this message]
2020-02-20 16:18 ` [PATCH] btrfs: clear file extent mapping for punch past i_size Filipe Manana
2020-02-21 16:58 ` David Sterba

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=20200220142947.3880392-1-josef@toxicpanda.com \
    --to=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@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.