* [PATCH v2] btrfs: Remove force argument from run_delalloc_nocow() @ 2021-02-25 20:58 Goldwyn Rodrigues 2021-02-25 23:22 ` Amy Parker 2021-02-26 17:14 ` David Sterba 0 siblings, 2 replies; 4+ messages in thread From: Goldwyn Rodrigues @ 2021-02-25 20:58 UTC (permalink / raw) To: linux-btrfs; +Cc: dsterba force_nocow can be calculated by btrfs_inode and does not need to be passed as an argument. This simplifies run_delalloc_nocow() call from btrfs_run_delalloc_range() since the decision whether the extent is cow'd or not can be derived from need_force_cow(). Since BTRFS_INODE_NODATACOW and BTRFS_INODE_PREALLOC flags are checked in need_force_cow(), there is no need to check it again. Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> Change since v1: - Kept need_force_cow() as it is --- fs/btrfs/inode.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 4f2f1e932751..e5dd8d7ef0c8 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -1516,7 +1516,7 @@ static int fallback_to_cow(struct btrfs_inode *inode, struct page *locked_page, static noinline int run_delalloc_nocow(struct btrfs_inode *inode, struct page *locked_page, const u64 start, const u64 end, - int *page_started, int force, + int *page_started, unsigned long *nr_written) { struct btrfs_fs_info *fs_info = inode->root->fs_info; @@ -1530,6 +1530,7 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode, u64 ino = btrfs_ino(inode); bool nocow = false; u64 disk_bytenr = 0; + bool force = inode->flags & BTRFS_INODE_NODATACOW; path = btrfs_alloc_path(); if (!path) { @@ -1891,17 +1892,12 @@ int btrfs_run_delalloc_range(struct btrfs_inode *inode, struct page *locked_page struct writeback_control *wbc) { int ret; - int force_cow = need_force_cow(inode, start, end); const bool zoned = btrfs_is_zoned(inode->root->fs_info); - if (inode->flags & BTRFS_INODE_NODATACOW && !force_cow) { + if (!need_force_cow(inode, start, end)) { ASSERT(!zoned); ret = run_delalloc_nocow(inode, locked_page, start, end, - page_started, 1, nr_written); - } else if (inode->flags & BTRFS_INODE_PREALLOC && !force_cow) { - ASSERT(!zoned); - ret = run_delalloc_nocow(inode, locked_page, start, end, - page_started, 0, nr_written); + page_started, nr_written); } else if (!inode_can_compress(inode) || !inode_need_compress(inode, start, end)) { if (zoned) -- 2.30.1 -- Goldwyn ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] btrfs: Remove force argument from run_delalloc_nocow() 2021-02-25 20:58 [PATCH v2] btrfs: Remove force argument from run_delalloc_nocow() Goldwyn Rodrigues @ 2021-02-25 23:22 ` Amy Parker 2021-02-26 17:14 ` David Sterba 1 sibling, 0 replies; 4+ messages in thread From: Amy Parker @ 2021-02-25 23:22 UTC (permalink / raw) To: Goldwyn Rodrigues; +Cc: Btrfs BTRFS, dsterba On Thu, Feb 25, 2021 at 1:04 PM Goldwyn Rodrigues <rgoldwyn@suse.de> wrote: > > force_nocow can be calculated by btrfs_inode and does not need to be > passed as an argument. > > This simplifies run_delalloc_nocow() call from btrfs_run_delalloc_range() > since the decision whether the extent is cow'd or not can be derived > from need_force_cow(). Since BTRFS_INODE_NODATACOW and > BTRFS_INODE_PREALLOC flags are checked in need_force_cow(), there is > no need to check it again. > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > > Change since v1: > - Kept need_force_cow() as it is > > --- > fs/btrfs/inode.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 4f2f1e932751..e5dd8d7ef0c8 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -1516,7 +1516,7 @@ static int fallback_to_cow(struct btrfs_inode *inode, struct page *locked_page, > static noinline int run_delalloc_nocow(struct btrfs_inode *inode, > struct page *locked_page, > const u64 start, const u64 end, > - int *page_started, int force, > + int *page_started, > unsigned long *nr_written) > { > struct btrfs_fs_info *fs_info = inode->root->fs_info; > @@ -1530,6 +1530,7 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode, > u64 ino = btrfs_ino(inode); > bool nocow = false; > u64 disk_bytenr = 0; > + bool force = inode->flags & BTRFS_INODE_NODATACOW; > > path = btrfs_alloc_path(); > if (!path) { > @@ -1891,17 +1892,12 @@ int btrfs_run_delalloc_range(struct btrfs_inode *inode, struct page *locked_page > struct writeback_control *wbc) > { > int ret; > - int force_cow = need_force_cow(inode, start, end); > const bool zoned = btrfs_is_zoned(inode->root->fs_info); > > - if (inode->flags & BTRFS_INODE_NODATACOW && !force_cow) { > + if (!need_force_cow(inode, start, end)) { > ASSERT(!zoned); > ret = run_delalloc_nocow(inode, locked_page, start, end, > - page_started, 1, nr_written); > - } else if (inode->flags & BTRFS_INODE_PREALLOC && !force_cow) { > - ASSERT(!zoned); > - ret = run_delalloc_nocow(inode, locked_page, start, end, > - page_started, 0, nr_written); > + page_started, nr_written); > } else if (!inode_can_compress(inode) || > !inode_need_compress(inode, start, end)) { > if (zoned) > -- > 2.30.1 > > > -- > Goldwyn Acked-by: Amy Parker <enbyamy@gmail.com> Not super familiar with the btrfs inode implementation - but from what I know of it and what I can read here it looks good to me. -Amy IP ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] btrfs: Remove force argument from run_delalloc_nocow() 2021-02-25 20:58 [PATCH v2] btrfs: Remove force argument from run_delalloc_nocow() Goldwyn Rodrigues 2021-02-25 23:22 ` Amy Parker @ 2021-02-26 17:14 ` David Sterba 2021-02-26 18:47 ` Goldwyn Rodrigues 1 sibling, 1 reply; 4+ messages in thread From: David Sterba @ 2021-02-26 17:14 UTC (permalink / raw) To: Goldwyn Rodrigues; +Cc: linux-btrfs, dsterba On Thu, Feb 25, 2021 at 02:58:22PM -0600, Goldwyn Rodrigues wrote: > force_nocow can be calculated by btrfs_inode and does not need to be > passed as an argument. > > This simplifies run_delalloc_nocow() call from btrfs_run_delalloc_range() > since the decision whether the extent is cow'd or not can be derived > from need_force_cow(). Since BTRFS_INODE_NODATACOW and > BTRFS_INODE_PREALLOC flags are checked in need_force_cow(), there is > no need to check it again. > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > > Change since v1: > - Kept need_force_cow() as it is Added to misc-next, thanks. > --- > fs/btrfs/inode.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 4f2f1e932751..e5dd8d7ef0c8 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -1516,7 +1516,7 @@ static int fallback_to_cow(struct btrfs_inode *inode, struct page *locked_page, > static noinline int run_delalloc_nocow(struct btrfs_inode *inode, > struct page *locked_page, > const u64 start, const u64 end, > - int *page_started, int force, > + int *page_started, > unsigned long *nr_written) > { > struct btrfs_fs_info *fs_info = inode->root->fs_info; > @@ -1530,6 +1530,7 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode, > u64 ino = btrfs_ino(inode); > bool nocow = false; > u64 disk_bytenr = 0; > + bool force = inode->flags & BTRFS_INODE_NODATACOW; > > path = btrfs_alloc_path(); > if (!path) { > @@ -1891,17 +1892,12 @@ int btrfs_run_delalloc_range(struct btrfs_inode *inode, struct page *locked_page > struct writeback_control *wbc) > { > int ret; > - int force_cow = need_force_cow(inode, start, end); > const bool zoned = btrfs_is_zoned(inode->root->fs_info); > > - if (inode->flags & BTRFS_INODE_NODATACOW && !force_cow) { > + if (!need_force_cow(inode, start, end)) { We may want to reverse the logic and naming so it says 'need_force_nocow', right now it's "we don't need to force COW, so let's do NOCOW", because COW is the default and the condition should pick the exceptional case. > ASSERT(!zoned); > ret = run_delalloc_nocow(inode, locked_page, start, end, > - page_started, 1, nr_written); > - } else if (inode->flags & BTRFS_INODE_PREALLOC && !force_cow) { > - ASSERT(!zoned); > - ret = run_delalloc_nocow(inode, locked_page, start, end, > - page_started, 0, nr_written); > + page_started, nr_written); > } else if (!inode_can_compress(inode) || > !inode_need_compress(inode, start, end)) { > if (zoned) > -- > 2.30.1 > > > -- > Goldwyn ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] btrfs: Remove force argument from run_delalloc_nocow() 2021-02-26 17:14 ` David Sterba @ 2021-02-26 18:47 ` Goldwyn Rodrigues 0 siblings, 0 replies; 4+ messages in thread From: Goldwyn Rodrigues @ 2021-02-26 18:47 UTC (permalink / raw) To: dsterba, linux-btrfs On 18:14 26/02, David Sterba wrote: > On Thu, Feb 25, 2021 at 02:58:22PM -0600, Goldwyn Rodrigues wrote: > > force_nocow can be calculated by btrfs_inode and does not need to be > > passed as an argument. > > > > This simplifies run_delalloc_nocow() call from btrfs_run_delalloc_range() > > since the decision whether the extent is cow'd or not can be derived > > from need_force_cow(). Since BTRFS_INODE_NODATACOW and > > BTRFS_INODE_PREALLOC flags are checked in need_force_cow(), there is > > no need to check it again. > > > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > > > > Change since v1: > > - Kept need_force_cow() as it is > > Added to misc-next, thanks. > > > --- > > fs/btrfs/inode.c | 12 ++++-------- > > 1 file changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > > index 4f2f1e932751..e5dd8d7ef0c8 100644 > > --- a/fs/btrfs/inode.c > > +++ b/fs/btrfs/inode.c > > @@ -1516,7 +1516,7 @@ static int fallback_to_cow(struct btrfs_inode *inode, struct page *locked_page, > > static noinline int run_delalloc_nocow(struct btrfs_inode *inode, > > struct page *locked_page, > > const u64 start, const u64 end, > > - int *page_started, int force, > > + int *page_started, > > unsigned long *nr_written) > > { > > struct btrfs_fs_info *fs_info = inode->root->fs_info; > > @@ -1530,6 +1530,7 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode, > > u64 ino = btrfs_ino(inode); > > bool nocow = false; > > u64 disk_bytenr = 0; > > + bool force = inode->flags & BTRFS_INODE_NODATACOW; > > > > path = btrfs_alloc_path(); > > if (!path) { > > @@ -1891,17 +1892,12 @@ int btrfs_run_delalloc_range(struct btrfs_inode *inode, struct page *locked_page > > struct writeback_control *wbc) > > { > > int ret; > > - int force_cow = need_force_cow(inode, start, end); > > const bool zoned = btrfs_is_zoned(inode->root->fs_info); > > > > - if (inode->flags & BTRFS_INODE_NODATACOW && !force_cow) { > > + if (!need_force_cow(inode, start, end)) { > > We may want to reverse the logic and naming so it says > 'need_force_nocow', right now it's "we don't need to force COW, so let's > do NOCOW", because COW is the default and the condition should pick the > exceptional case. Calling it need_force_nocow() or should_nocow() will result in the same concept as previous patch where EXTENT_DEFRAG range would need to be checked before checking inode flags BTRFS_INODE_NODATACOW and BTRFS_INODE_PREALLOC. That's because EXTENT_DEFRAG in range should result in a COW sequence immaterial of what CoW sequence the inode flags say. Isn't it? Maybe something like this would keep inode flags checks earlier than test_range_bit(): static inline bool should_nocow(struct btrfs_inode *inode, u64 start, u64 end) { if (inode->flags & (BTRFS_INODE_NODATACOW | BTRFS_INODE_PREALLOC)) { if (inode->defrag_bytes && test_range_bit(&inode->io_tree, start, end, EXTENT_DEFRAG, 0, NULL)) return false; return true; } return false; } > > > ASSERT(!zoned); > > ret = run_delalloc_nocow(inode, locked_page, start, end, > > - page_started, 1, nr_written); > > - } else if (inode->flags & BTRFS_INODE_PREALLOC && !force_cow) { > > - ASSERT(!zoned); > > - ret = run_delalloc_nocow(inode, locked_page, start, end, > > - page_started, 0, nr_written); > > + page_started, nr_written); > > } else if (!inode_can_compress(inode) || > > !inode_need_compress(inode, start, end)) { > > if (zoned) -- Goldwyn ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-26 18:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-25 20:58 [PATCH v2] btrfs: Remove force argument from run_delalloc_nocow() Goldwyn Rodrigues 2021-02-25 23:22 ` Amy Parker 2021-02-26 17:14 ` David Sterba 2021-02-26 18:47 ` Goldwyn Rodrigues
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.