All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/3] btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation
@ 2020-06-28  5:18 Qu Wenruo
  2020-06-28  5:18 ` [PATCH v6 1/3] " Qu Wenruo
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Qu Wenruo @ 2020-06-28  5:18 UTC (permalink / raw)
  To: linux-btrfs

Before this patch, btrfs_truncate_block() never checks the NODATACOW
bit, thus when we run out of data space, we can return ENOSPC for
truncate for NODATACOW inode.

This patchset will address this problem by doing the same behavior as
buffered write to address it.

Changelog:
v2:
- Rebased to misc-next
  Only one minor conflict in ctree.h

v3:
- Added two new patches
- Refactor check_can_nocow()
  Since the introduction of nowait, check_can_nocow() are in fact split
  into two usage patterns: check_can_nocow(nowait = false) with
  btrfs_drew_write_unlock(), and single check_can_nocow(nowait = true).
  Refactor them into two functions: start_nocow_check() paired with
  end_nocow_check(), and single try_nocow_check(). With comment added.

- Rebased to latest misc-next

- Added btrfs_assert_drew_write_locked() for btrfs_end_nocow_check()
  This is a little concerning one, as it's in the hot path of buffered
  write.
  It has percpu_counter_sum() called in that hot path, causing
  obvious performance drop for CONFIG_BTRFS_DEBUG build.
  Not sure if the assert is worthy since there aren't any other users.

v4:
- Rebased to latest misc-next

- Comment update to follow the relaxed kernel-doc format

- Re-order the patches
  So the fix comes first, then refactors.

- Naming updates for the refactor patch
  Now the exported pair is btrfs_check_nocow_lock() and
  btrfs_check_nocow_unlock().

- Remove the btrfs_assert_drew_write_lock()
  It's extremely slow for btrfs/187, and we only have two call sites so
  far, thus it's not really worthy.

v5:
- Fix a stupid bug that check_nocow_nolock() passes wrong bool

- Update the comment for btrfs_check_nocow_lock() as there is no
  "nowait" parameter anymore

v6:
- Replace one btrfs_drew_write_unlock() with the wrapper

Qu Wenruo (3):
  btrfs: allow btrfs_truncate_block() to fallback to nocow for data
    space reservation
  btrfs: add comments for btrfs_check_can_nocow() and can_nocow_extent()
  btrfs: refactor btrfs_check_can_nocow() into two variants

 fs/btrfs/ctree.h |  3 +++
 fs/btrfs/file.c  | 61 +++++++++++++++++++++++++++++++++++-----------
 fs/btrfs/inode.c | 63 +++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 104 insertions(+), 23 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v6 1/3] btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation
  2020-06-28  5:18 [PATCH v6 0/3] btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation Qu Wenruo
@ 2020-06-28  5:18 ` Qu Wenruo
  2020-06-28  5:18 ` [PATCH v6 2/3] btrfs: add comments for btrfs_check_can_nocow() and can_nocow_extent() Qu Wenruo
  2020-06-28  5:18 ` [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants Qu Wenruo
  2 siblings, 0 replies; 9+ messages in thread
From: Qu Wenruo @ 2020-06-28  5:18 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Martin Doucha, Filipe Manana, Anand Jain

[BUG]
When the data space is exhausted, even the inode has NOCOW attribute,
btrfs will still refuse to truncate unaligned range due to ENOSPC.

The following script can reproduce it pretty easily:
  #!/bin/bash

  dev=/dev/test/test
  mnt=/mnt/btrfs

  umount $dev &> /dev/null
  umount $mnt&> /dev/null

  mkfs.btrfs -f $dev -b 1G
  mount -o nospace_cache $dev $mnt
  touch $mnt/foobar
  chattr +C $mnt/foobar

  xfs_io -f -c "pwrite -b 4k 0 4k" $mnt/foobar > /dev/null
  xfs_io -f -c "pwrite -b 4k 0 1G" $mnt/padding &> /dev/null
  sync

  xfs_io -c "fpunch 0 2k" $mnt/foobar
  umount $mnt

Current btrfs will fail at the fpunch part.

[CAUSE]
Because btrfs_truncate_block() always reserve space without checking the
NOCOW attribute.

Since the writeback path follows NOCOW bit, we only need to bother the
space reservation code in btrfs_truncate_block().

[FIX]
Make btrfs_truncate_block() to follow btrfs_buffered_write() to try to
reserve data space first, and falls back to NOCOW check only when we
don't have enough space.

Such always-try-reserve is an optimization introduced in
btrfs_buffered_write(), to avoid expensive btrfs_check_can_nocow() call.

This patch will export check_can_nocow() as btrfs_check_can_nocow(), and
use it in btrfs_truncate_block() to fix the problem.

Reported-by: Martin Doucha <martin.doucha@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ctree.h |  2 ++
 fs/btrfs/file.c  | 12 ++++++------
 fs/btrfs/inode.c | 44 +++++++++++++++++++++++++++++++++++++-------
 3 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d8301bf240e0..b7e14189da32 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3035,6 +3035,8 @@ int btrfs_dirty_pages(struct inode *inode, struct page **pages,
 		      size_t num_pages, loff_t pos, size_t write_bytes,
 		      struct extent_state **cached);
 int btrfs_fdatawrite_range(struct inode *inode, loff_t start, loff_t end);
+int btrfs_check_can_nocow(struct btrfs_inode *inode, loff_t pos,
+			  size_t *write_bytes, bool nowait);
 
 /* tree-defrag.c */
 int btrfs_defrag_leaves(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index fccf5862cd3e..0115ef7f7943 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1533,8 +1533,8 @@ lock_and_cleanup_extent_if_need(struct btrfs_inode *inode, struct page **pages,
 	return ret;
 }
 
-static noinline int check_can_nocow(struct btrfs_inode *inode, loff_t pos,
-				    size_t *write_bytes, bool nowait)
+int btrfs_check_can_nocow(struct btrfs_inode *inode, loff_t pos,
+			  size_t *write_bytes, bool nowait)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct btrfs_root *root = inode->root;
@@ -1649,8 +1649,8 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 		if (ret < 0) {
 			if ((BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
 						      BTRFS_INODE_PREALLOC)) &&
-			    check_can_nocow(BTRFS_I(inode), pos,
-					    &write_bytes, false) > 0) {
+			    btrfs_check_can_nocow(BTRFS_I(inode), pos,
+						  &write_bytes, false) > 0) {
 				/*
 				 * For nodata cow case, no need to reserve
 				 * data space.
@@ -1927,8 +1927,8 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
 		 */
 		if (!(BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
 					      BTRFS_INODE_PREALLOC)) ||
-		    check_can_nocow(BTRFS_I(inode), pos, &nocow_bytes,
-				    true) <= 0) {
+		    btrfs_check_can_nocow(BTRFS_I(inode), pos, &nocow_bytes,
+					  true) <= 0) {
 			inode_unlock(inode);
 			return -EAGAIN;
 		}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 86f7aa377da9..9d9ec7838f13 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4519,11 +4519,13 @@ int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
 	struct extent_state *cached_state = NULL;
 	struct extent_changeset *data_reserved = NULL;
 	char *kaddr;
+	bool only_release_metadata = false;
 	u32 blocksize = fs_info->sectorsize;
 	pgoff_t index = from >> PAGE_SHIFT;
 	unsigned offset = from & (blocksize - 1);
 	struct page *page;
 	gfp_t mask = btrfs_alloc_write_mask(mapping);
+	size_t write_bytes = blocksize;
 	int ret = 0;
 	u64 block_start;
 	u64 block_end;
@@ -4535,11 +4537,27 @@ int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
 	block_start = round_down(from, blocksize);
 	block_end = block_start + blocksize - 1;
 
-	ret = btrfs_delalloc_reserve_space(inode, &data_reserved,
-					   block_start, blocksize);
-	if (ret)
-		goto out;
 
+	ret = btrfs_check_data_free_space(inode, &data_reserved, block_start,
+					  blocksize);
+	if (ret < 0) {
+		if ((BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
+					      BTRFS_INODE_PREALLOC)) &&
+		    btrfs_check_can_nocow(BTRFS_I(inode), block_start,
+					  &write_bytes, false) > 0) {
+			/* For nocow case, no need to reserve data space. */
+			only_release_metadata = true;
+		} else {
+			goto out;
+		}
+	}
+	ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), blocksize);
+	if (ret < 0) {
+		if (!only_release_metadata)
+			btrfs_free_reserved_data_space(inode, data_reserved,
+					block_start, blocksize);
+		goto out;
+	}
 again:
 	page = find_or_create_page(mapping, index, mask);
 	if (!page) {
@@ -4608,14 +4626,26 @@ int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
 	set_page_dirty(page);
 	unlock_extent_cached(io_tree, block_start, block_end, &cached_state);
 
+	if (only_release_metadata)
+		set_extent_bit(&BTRFS_I(inode)->io_tree, block_start,
+				block_end, EXTENT_NORESERVE, NULL, NULL,
+				GFP_NOFS);
+
 out_unlock:
-	if (ret)
-		btrfs_delalloc_release_space(inode, data_reserved, block_start,
-					     blocksize, true);
+	if (ret) {
+		if (!only_release_metadata)
+			btrfs_delalloc_release_space(inode, data_reserved,
+					block_start, blocksize, true);
+		else
+			btrfs_delalloc_release_metadata(BTRFS_I(inode),
+					blocksize, true);
+	}
 	btrfs_delalloc_release_extents(BTRFS_I(inode), blocksize);
 	unlock_page(page);
 	put_page(page);
 out:
+	if (only_release_metadata)
+		btrfs_drew_write_unlock(&BTRFS_I(inode)->root->snapshot_lock);
 	extent_changeset_free(data_reserved);
 	return ret;
 }
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 2/3] btrfs: add comments for btrfs_check_can_nocow() and can_nocow_extent()
  2020-06-28  5:18 [PATCH v6 0/3] btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation Qu Wenruo
  2020-06-28  5:18 ` [PATCH v6 1/3] " Qu Wenruo
@ 2020-06-28  5:18 ` Qu Wenruo
  2020-06-28  5:18 ` [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants Qu Wenruo
  2 siblings, 0 replies; 9+ messages in thread
From: Qu Wenruo @ 2020-06-28  5:18 UTC (permalink / raw)
  To: linux-btrfs

These two functions have extra conditions that their callers need to
meet, and some not-that-common parameters used for return value.

So adding some comments may save reviewers some time.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/file.c  | 21 +++++++++++++++++++++
 fs/btrfs/inode.c | 21 +++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 0115ef7f7943..2bee888cb929 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1533,6 +1533,27 @@ lock_and_cleanup_extent_if_need(struct btrfs_inode *inode, struct page **pages,
 	return ret;
 }
 
+/*
+ * Check if we can do nocow write into the range [@pos, @pos + @write_bytes)
+ *
+ * @pos:	 File offset
+ * @write_bytes: The length to write, will be updated to the nocow writeable
+ *		 range
+ * @nowait:	 Whether this function could sleep.
+ *
+ * This function will flush ordered extents in the range to ensure proper
+ * nocow checks for (nowait == false) case.
+ *
+ * Return:
+ * >0 and update @write_bytes if we can do nocow write.
+ * 0 if we can't do nocow write.
+ * -EAGAIN if we can't get the needed lock or there are ordered extents for
+ * (nowait == true) case.
+ * <0 if other error happened.
+ *
+ * NOTE: For wait (nowait==false) calls, callers need to release the drew write
+ * 	 lock of inode->root->snapshot_lock when return value > 0.
+ */
 int btrfs_check_can_nocow(struct btrfs_inode *inode, loff_t pos,
 			  size_t *write_bytes, bool nowait)
 {
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 9d9ec7838f13..339d739b2d29 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6952,8 +6952,25 @@ static struct extent_map *btrfs_new_extent_direct(struct inode *inode,
 }
 
 /*
- * returns 1 when the nocow is safe, < 1 on error, 0 if the
- * block must be cow'd
+ * Check if we can do nocow write into the range [@offset, @offset+ @len)
+ *
+ * @offset:	File offset
+ * @len:	The length to write, will be updated to the nocow writeable
+ *		range
+ * @orig_start:	(Optional) Return the original file offset of the file extent
+ * @orig_len:	(Optional) Return the original on-disk length of the file extent
+ * @ram_bytes:	(Optional) Return the ram_bytes of the file extent
+ *
+ * This function will flush ordered extents in the range to ensure proper
+ * nocow checks for (nowait == false) case.
+ *
+ * Return:
+ * >0 and update @len if we can do nocow write.
+ * 0 if we can't do nocow write.
+ * <0 if error happened.
+ *
+ * NOTE: This only checks the file extents, caller is responsible to wait for
+ *	 any ordered extents.
  */
 noinline int can_nocow_extent(struct inode *inode, u64 offset, u64 *len,
 			      u64 *orig_start, u64 *orig_block_len,
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants
  2020-06-28  5:18 [PATCH v6 0/3] btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation Qu Wenruo
  2020-06-28  5:18 ` [PATCH v6 1/3] " Qu Wenruo
  2020-06-28  5:18 ` [PATCH v6 2/3] btrfs: add comments for btrfs_check_can_nocow() and can_nocow_extent() Qu Wenruo
@ 2020-06-28  5:18 ` Qu Wenruo
  2020-06-28  8:41   ` kernel test robot
  2020-06-29  7:13   ` Johannes Thumshirn
  2 siblings, 2 replies; 9+ messages in thread
From: Qu Wenruo @ 2020-06-28  5:18 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Anand Jain

The function btrfs_check_can_nocow() now has two completely different call
patterns.

For nowait variant, callers don't need to do any cleanup.
While for wait variant, callers need to release the lock if they can do
nocow write.

This is somehow confusing, and is already a problem for the exported
btrfs_check_can_nocow().

So this patch will separate the different patterns into different
functions.
For nowait variant, the function will be called check_nocow_nolock().
For wait variant, the function pair will be btrfs_check_nocow_lock()
btrfs_check_nocow_unlock().

Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/ctree.h |  5 +--
 fs/btrfs/file.c  | 82 +++++++++++++++++++++++++++---------------------
 fs/btrfs/inode.c |  8 ++---
 3 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index b7e14189da32..ce716c4e3b13 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3035,8 +3035,9 @@ int btrfs_dirty_pages(struct inode *inode, struct page **pages,
 		      size_t num_pages, loff_t pos, size_t write_bytes,
 		      struct extent_state **cached);
 int btrfs_fdatawrite_range(struct inode *inode, loff_t start, loff_t end);
-int btrfs_check_can_nocow(struct btrfs_inode *inode, loff_t pos,
-			  size_t *write_bytes, bool nowait);
+int btrfs_check_nocow_lock(struct btrfs_inode *inode, loff_t pos,
+			   size_t *write_bytes);
+void btrfs_check_nocow_unlock(struct btrfs_inode *inode);
 
 /* tree-defrag.c */
 int btrfs_defrag_leaves(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 2bee888cb929..3c84f0df1292 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1533,29 +1533,8 @@ lock_and_cleanup_extent_if_need(struct btrfs_inode *inode, struct page **pages,
 	return ret;
 }
 
-/*
- * Check if we can do nocow write into the range [@pos, @pos + @write_bytes)
- *
- * @pos:	 File offset
- * @write_bytes: The length to write, will be updated to the nocow writeable
- *		 range
- * @nowait:	 Whether this function could sleep.
- *
- * This function will flush ordered extents in the range to ensure proper
- * nocow checks for (nowait == false) case.
- *
- * Return:
- * >0 and update @write_bytes if we can do nocow write.
- * 0 if we can't do nocow write.
- * -EAGAIN if we can't get the needed lock or there are ordered extents for
- * (nowait == true) case.
- * <0 if other error happened.
- *
- * NOTE: For wait (nowait==false) calls, callers need to release the drew write
- * 	 lock of inode->root->snapshot_lock when return value > 0.
- */
-int btrfs_check_can_nocow(struct btrfs_inode *inode, loff_t pos,
-			  size_t *write_bytes, bool nowait)
+static int check_can_nocow(struct btrfs_inode *inode, loff_t pos,
+			   size_t *write_bytes, bool nowait)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct btrfs_root *root = inode->root;
@@ -1563,6 +1542,9 @@ int btrfs_check_can_nocow(struct btrfs_inode *inode, loff_t pos,
 	u64 num_bytes;
 	int ret;
 
+	if (!(inode->flags & (BTRFS_INODE_NODATACOW | BTRFS_INODE_PREALLOC)))
+		return 0;
+
 	if (!nowait && !btrfs_drew_try_write_lock(&root->snapshot_lock))
 		return -EAGAIN;
 
@@ -1605,6 +1587,41 @@ int btrfs_check_can_nocow(struct btrfs_inode *inode, loff_t pos,
 	return ret;
 }
 
+static int check_nocow_nolock(struct btrfs_inode *inode, loff_t pos,
+			      size_t *write_bytes)
+{
+	return check_can_nocow(inode, pos, write_bytes, true);
+}
+
+/*
+ * Check if we can do nocow write into the range [@pos, @pos + @write_bytes)
+ *
+ * @pos:	 File offset
+ * @write_bytes: The length to write, will be updated to the nocow writeable
+ *		 range
+ *
+ * This function will flush ordered extents in the range to ensure proper
+ * nocow checks.
+ *
+ * Return:
+ * >0 and update @write_bytes if we can do nocow write.
+ * 0 if we can't do nocow write.
+ * -EAGAIN if we can't get the needed lock.
+ * <0 if other error happened.
+ *
+ * NOTE: Callers need to release the lock by btrfs_check_nocow_unlock().
+ */
+int btrfs_check_nocow_lock(struct btrfs_inode *inode, loff_t pos,
+			   size_t *write_bytes)
+{
+	return check_can_nocow(inode, pos, write_bytes, false);
+}
+
+void btrfs_check_nocow_unlock(struct btrfs_inode *inode)
+{
+	btrfs_drew_write_unlock(&inode->root->snapshot_lock);
+}
+
 static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 					       struct iov_iter *i)
 {
@@ -1612,7 +1629,6 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 	loff_t pos = iocb->ki_pos;
 	struct inode *inode = file_inode(file);
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
-	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct page **pages = NULL;
 	struct extent_changeset *data_reserved = NULL;
 	u64 release_bytes = 0;
@@ -1668,10 +1684,8 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 		ret = btrfs_check_data_free_space(inode, &data_reserved, pos,
 						  write_bytes);
 		if (ret < 0) {
-			if ((BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
-						      BTRFS_INODE_PREALLOC)) &&
-			    btrfs_check_can_nocow(BTRFS_I(inode), pos,
-						  &write_bytes, false) > 0) {
+			if (btrfs_check_nocow_lock(BTRFS_I(inode), pos,
+						   &write_bytes) > 0) {
 				/*
 				 * For nodata cow case, no need to reserve
 				 * data space.
@@ -1700,7 +1714,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 						data_reserved, pos,
 						write_bytes);
 			else
-				btrfs_drew_write_unlock(&root->snapshot_lock);
+				btrfs_check_nocow_unlock(BTRFS_I(inode));
 			break;
 		}
 
@@ -1804,7 +1818,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 
 		release_bytes = 0;
 		if (only_release_metadata)
-			btrfs_drew_write_unlock(&root->snapshot_lock);
+			btrfs_check_nocow_unlock(BTRFS_I(inode));
 
 		if (only_release_metadata && copied > 0) {
 			lockstart = round_down(pos,
@@ -1831,7 +1845,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 
 	if (release_bytes) {
 		if (only_release_metadata) {
-			btrfs_drew_write_unlock(&root->snapshot_lock);
+			btrfs_check_nocow_unlock(BTRFS_I(inode));
 			btrfs_delalloc_release_metadata(BTRFS_I(inode),
 					release_bytes, true);
 		} else {
@@ -1946,10 +1960,8 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
 		 * We will allocate space in case nodatacow is not set,
 		 * so bail
 		 */
-		if (!(BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
-					      BTRFS_INODE_PREALLOC)) ||
-		    btrfs_check_can_nocow(BTRFS_I(inode), pos, &nocow_bytes,
-					  true) <= 0) {
+		if (check_nocow_nolock(BTRFS_I(inode), pos, &nocow_bytes)
+		    <= 0) {
 			inode_unlock(inode);
 			return -EAGAIN;
 		}
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 339d739b2d29..95d3b68eee4f 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4541,10 +4541,8 @@ int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
 	ret = btrfs_check_data_free_space(inode, &data_reserved, block_start,
 					  blocksize);
 	if (ret < 0) {
-		if ((BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW |
-					      BTRFS_INODE_PREALLOC)) &&
-		    btrfs_check_can_nocow(BTRFS_I(inode), block_start,
-					  &write_bytes, false) > 0) {
+		if (btrfs_check_nocow_lock(BTRFS_I(inode), block_start,
+					   &write_bytes) > 0) {
 			/* For nocow case, no need to reserve data space. */
 			only_release_metadata = true;
 		} else {
@@ -4645,7 +4643,7 @@ int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
 	put_page(page);
 out:
 	if (only_release_metadata)
-		btrfs_drew_write_unlock(&BTRFS_I(inode)->root->snapshot_lock);
+		btrfs_check_nocow_unlock(BTRF_I(inode));
 	extent_changeset_free(data_reserved);
 	return ret;
 }
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants
  2020-06-28  5:18 ` [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants Qu Wenruo
@ 2020-06-28  8:41   ` kernel test robot
  2020-06-29  7:13   ` Johannes Thumshirn
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2020-06-28  8:41 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 8091 bytes --]

Hi Qu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20200626]
[cannot apply to kdave/for-next btrfs/next v5.8-rc2 v5.8-rc1 v5.7 v5.8-rc2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Qu-Wenruo/btrfs-allow-btrfs_truncate_block-to-fallback-to-nocow-for-data-space-reservation/20200628-133225
base:    36e3135df4d426612fc77db26a312c2531108603
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 8cd117c24f48428e01f88cf18480e5af7eb20c0c)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   In file included from fs/btrfs/inode.c:36:
   fs/btrfs/ctree.h:2217:8: warning: 'const' type qualifier on return type has no effect [-Wignored-qualifiers]
   size_t __const btrfs_get_num_csums(void);
          ^~~~~~~~
>> fs/btrfs/inode.c:4646:28: error: implicit declaration of function 'BTRF_I' [-Werror,-Wimplicit-function-declaration]
                   btrfs_check_nocow_unlock(BTRF_I(inode));
                                            ^
   fs/btrfs/inode.c:4646:28: note: did you mean 'BTRFS_I'?
   fs/btrfs/btrfs_inode.h:204:35: note: 'BTRFS_I' declared here
   static inline struct btrfs_inode *BTRFS_I(const struct inode *inode)
                                     ^
>> fs/btrfs/inode.c:4646:28: warning: incompatible integer to pointer conversion passing 'int' to parameter of type 'struct btrfs_inode *' [-Wint-conversion]
                   btrfs_check_nocow_unlock(BTRF_I(inode));
                                            ^~~~~~~~~~~~~
   fs/btrfs/ctree.h:2987:51: note: passing argument to parameter 'inode' here
   void btrfs_check_nocow_unlock(struct btrfs_inode *inode);
                                                     ^
   2 warnings and 1 error generated.

vim +/BTRF_I +4646 fs/btrfs/inode.c

  4500	
  4501	/*
  4502	 * btrfs_truncate_block - read, zero a chunk and write a block
  4503	 * @inode - inode that we're zeroing
  4504	 * @from - the offset to start zeroing
  4505	 * @len - the length to zero, 0 to zero the entire range respective to the
  4506	 *	offset
  4507	 * @front - zero up to the offset instead of from the offset on
  4508	 *
  4509	 * This will find the block for the "from" offset and cow the block and zero the
  4510	 * part we want to zero.  This is used with truncate and hole punching.
  4511	 */
  4512	int btrfs_truncate_block(struct inode *inode, loff_t from, loff_t len,
  4513				int front)
  4514	{
  4515		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
  4516		struct address_space *mapping = inode->i_mapping;
  4517		struct extent_io_tree *io_tree = &BTRFS_I(inode)->io_tree;
  4518		struct btrfs_ordered_extent *ordered;
  4519		struct extent_state *cached_state = NULL;
  4520		struct extent_changeset *data_reserved = NULL;
  4521		char *kaddr;
  4522		bool only_release_metadata = false;
  4523		u32 blocksize = fs_info->sectorsize;
  4524		pgoff_t index = from >> PAGE_SHIFT;
  4525		unsigned offset = from & (blocksize - 1);
  4526		struct page *page;
  4527		gfp_t mask = btrfs_alloc_write_mask(mapping);
  4528		size_t write_bytes = blocksize;
  4529		int ret = 0;
  4530		u64 block_start;
  4531		u64 block_end;
  4532	
  4533		if (IS_ALIGNED(offset, blocksize) &&
  4534		    (!len || IS_ALIGNED(len, blocksize)))
  4535			goto out;
  4536	
  4537		block_start = round_down(from, blocksize);
  4538		block_end = block_start + blocksize - 1;
  4539	
  4540	
  4541		ret = btrfs_check_data_free_space(inode, &data_reserved, block_start,
  4542						  blocksize);
  4543		if (ret < 0) {
  4544			if (btrfs_check_nocow_lock(BTRFS_I(inode), block_start,
  4545						   &write_bytes) > 0) {
  4546				/* For nocow case, no need to reserve data space. */
  4547				only_release_metadata = true;
  4548			} else {
  4549				goto out;
  4550			}
  4551		}
  4552		ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), blocksize);
  4553		if (ret < 0) {
  4554			if (!only_release_metadata)
  4555				btrfs_free_reserved_data_space(inode, data_reserved,
  4556						block_start, blocksize);
  4557			goto out;
  4558		}
  4559	again:
  4560		page = find_or_create_page(mapping, index, mask);
  4561		if (!page) {
  4562			btrfs_delalloc_release_space(inode, data_reserved,
  4563						     block_start, blocksize, true);
  4564			btrfs_delalloc_release_extents(BTRFS_I(inode), blocksize);
  4565			ret = -ENOMEM;
  4566			goto out;
  4567		}
  4568	
  4569		if (!PageUptodate(page)) {
  4570			ret = btrfs_readpage(NULL, page);
  4571			lock_page(page);
  4572			if (page->mapping != mapping) {
  4573				unlock_page(page);
  4574				put_page(page);
  4575				goto again;
  4576			}
  4577			if (!PageUptodate(page)) {
  4578				ret = -EIO;
  4579				goto out_unlock;
  4580			}
  4581		}
  4582		wait_on_page_writeback(page);
  4583	
  4584		lock_extent_bits(io_tree, block_start, block_end, &cached_state);
  4585		set_page_extent_mapped(page);
  4586	
  4587		ordered = btrfs_lookup_ordered_extent(inode, block_start);
  4588		if (ordered) {
  4589			unlock_extent_cached(io_tree, block_start, block_end,
  4590					     &cached_state);
  4591			unlock_page(page);
  4592			put_page(page);
  4593			btrfs_start_ordered_extent(inode, ordered, 1);
  4594			btrfs_put_ordered_extent(ordered);
  4595			goto again;
  4596		}
  4597	
  4598		clear_extent_bit(&BTRFS_I(inode)->io_tree, block_start, block_end,
  4599				 EXTENT_DELALLOC | EXTENT_DO_ACCOUNTING | EXTENT_DEFRAG,
  4600				 0, 0, &cached_state);
  4601	
  4602		ret = btrfs_set_extent_delalloc(inode, block_start, block_end, 0,
  4603						&cached_state);
  4604		if (ret) {
  4605			unlock_extent_cached(io_tree, block_start, block_end,
  4606					     &cached_state);
  4607			goto out_unlock;
  4608		}
  4609	
  4610		if (offset != blocksize) {
  4611			if (!len)
  4612				len = blocksize - offset;
  4613			kaddr = kmap(page);
  4614			if (front)
  4615				memset(kaddr + (block_start - page_offset(page)),
  4616					0, offset);
  4617			else
  4618				memset(kaddr + (block_start - page_offset(page)) +  offset,
  4619					0, len);
  4620			flush_dcache_page(page);
  4621			kunmap(page);
  4622		}
  4623		ClearPageChecked(page);
  4624		set_page_dirty(page);
  4625		unlock_extent_cached(io_tree, block_start, block_end, &cached_state);
  4626	
  4627		if (only_release_metadata)
  4628			set_extent_bit(&BTRFS_I(inode)->io_tree, block_start,
  4629					block_end, EXTENT_NORESERVE, NULL, NULL,
  4630					GFP_NOFS);
  4631	
  4632	out_unlock:
  4633		if (ret) {
  4634			if (!only_release_metadata)
  4635				btrfs_delalloc_release_space(inode, data_reserved,
  4636						block_start, blocksize, true);
  4637			else
  4638				btrfs_delalloc_release_metadata(BTRFS_I(inode),
  4639						blocksize, true);
  4640		}
  4641		btrfs_delalloc_release_extents(BTRFS_I(inode), blocksize);
  4642		unlock_page(page);
  4643		put_page(page);
  4644	out:
  4645		if (only_release_metadata)
> 4646			btrfs_check_nocow_unlock(BTRF_I(inode));
  4647		extent_changeset_free(data_reserved);
  4648		return ret;
  4649	}
  4650	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 75504 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants
  2020-06-28  5:18 ` [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants Qu Wenruo
  2020-06-28  8:41   ` kernel test robot
@ 2020-06-29  7:13   ` Johannes Thumshirn
  2020-06-29  7:20     ` Qu Wenruo
  1 sibling, 1 reply; 9+ messages in thread
From: Johannes Thumshirn @ 2020-06-29  7:13 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs; +Cc: Anand Jain

On 28/06/2020 07:19, Qu Wenruo wrote:
> -		btrfs_drew_write_unlock(&BTRFS_I(inode)->root->snapshot_lock);
> +		btrfs_check_nocow_unlock(BTRF_I(inode));

Huhz?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants
  2020-06-29  7:13   ` Johannes Thumshirn
@ 2020-06-29  7:20     ` Qu Wenruo
  2020-06-29  7:21       ` Johannes Thumshirn
  2020-06-29 20:29       ` David Sterba
  0 siblings, 2 replies; 9+ messages in thread
From: Qu Wenruo @ 2020-06-29  7:20 UTC (permalink / raw)
  To: Johannes Thumshirn, linux-btrfs; +Cc: Anand Jain



On 2020/6/29 下午3:13, Johannes Thumshirn wrote:
> On 28/06/2020 07:19, Qu Wenruo wrote:
>> -		btrfs_drew_write_unlock(&BTRFS_I(inode)->root->snapshot_lock);
>> +		btrfs_check_nocow_unlock(BTRF_I(inode));
> 
> Huhz?
> 
That's why there is a v7 update...


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants
  2020-06-29  7:20     ` Qu Wenruo
@ 2020-06-29  7:21       ` Johannes Thumshirn
  2020-06-29 20:29       ` David Sterba
  1 sibling, 0 replies; 9+ messages in thread
From: Johannes Thumshirn @ 2020-06-29  7:21 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs; +Cc: Anand Jain

On 29/06/2020 09:21, Qu Wenruo wrote:
> 
> 
> On 2020/6/29 下午3:13, Johannes Thumshirn wrote:
>> On 28/06/2020 07:19, Qu Wenruo wrote:
>>> -		btrfs_drew_write_unlock(&BTRFS_I(inode)->root->snapshot_lock);
>>> +		btrfs_check_nocow_unlock(BTRF_I(inode));
>>
>> Huhz?
>>
> That's why there is a v7 update...
> 
> 

Ah see it now

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants
  2020-06-29  7:20     ` Qu Wenruo
  2020-06-29  7:21       ` Johannes Thumshirn
@ 2020-06-29 20:29       ` David Sterba
  1 sibling, 0 replies; 9+ messages in thread
From: David Sterba @ 2020-06-29 20:29 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: Johannes Thumshirn, linux-btrfs, Anand Jain

On Mon, Jun 29, 2020 at 03:20:42PM +0800, Qu Wenruo wrote:
> 
> 
> On 2020/6/29 下午3:13, Johannes Thumshirn wrote:
> > On 28/06/2020 07:19, Qu Wenruo wrote:
> >> -		btrfs_drew_write_unlock(&BTRFS_I(inode)->root->snapshot_lock);
> >> +		btrfs_check_nocow_unlock(BTRF_I(inode));
> > 
> > Huhz?
> > 
> That's why there is a v7 update...

vger.kernel.org was down today, I replied that you should not have sent
v7 but rather point out the issue.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-06-29 20:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-28  5:18 [PATCH v6 0/3] btrfs: allow btrfs_truncate_block() to fallback to nocow for data space reservation Qu Wenruo
2020-06-28  5:18 ` [PATCH v6 1/3] " Qu Wenruo
2020-06-28  5:18 ` [PATCH v6 2/3] btrfs: add comments for btrfs_check_can_nocow() and can_nocow_extent() Qu Wenruo
2020-06-28  5:18 ` [PATCH v6 3/3] btrfs: refactor btrfs_check_can_nocow() into two variants Qu Wenruo
2020-06-28  8:41   ` kernel test robot
2020-06-29  7:13   ` Johannes Thumshirn
2020-06-29  7:20     ` Qu Wenruo
2020-06-29  7:21       ` Johannes Thumshirn
2020-06-29 20:29       ` David Sterba

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.