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