* [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.