* [PATCH 0/3] Cleanup btrfs_find_name_in* functions
@ 2019-08-27 11:46 Nikolay Borisov
2019-08-27 11:46 ` [PATCH 1/3] btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct Nikolay Borisov
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Nikolay Borisov @ 2019-08-27 11:46 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
This series cleans up the calling convention of the 2 function used to search
the INODE_REF items for the presence of particular name. Namely, I switch the 2
functions to directly return a struct btrfs_inode_(ext)?ref. This reduces the
number of parameters when calling the functions.
While doing the aforementioned cleanup I stumbled upon backref_in_log which was
opencoding btrfs_find_name_in_backref hence I just refactored the function.
This series survived xfstest.
Nikolay Borisov (3):
btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct
btrfs: Make btrfs_find_name_in_ext_backref return struct
btrfs_inode_extref
btrfs: Use btrfs_find_name_in_backref in backref_in_log
fs/btrfs/ctree.h | 15 +++++-----
fs/btrfs/inode-item.c | 62 ++++++++++++++++++-----------------------
fs/btrfs/tree-log.c | 65 +++++++++++++------------------------------
3 files changed, 54 insertions(+), 88 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct
2019-08-27 11:46 [PATCH 0/3] Cleanup btrfs_find_name_in* functions Nikolay Borisov
@ 2019-08-27 11:46 ` Nikolay Borisov
2019-08-27 14:13 ` David Sterba
2019-08-27 11:46 ` [PATCH 2/3] btrfs: Make btrfs_find_name_in_ext_backref return struct btrfs_inode_extref Nikolay Borisov
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2019-08-27 11:46 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
btrfs_find_name_in_backref returns either 0/1 depending on whether it
found a backref for the given name. If it returns true then the actual
inode_ref struct is returned in one of its parameters. That's pointless,
instead refactor the function such that it returns either a pointer
to the btrfs_inode_ref or NULL it it didn't find anything. This
streamlines the function calling convention.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/ctree.h | 6 +++---
fs/btrfs/inode-item.c | 29 ++++++++++++++---------------
fs/btrfs/tree-log.c | 4 ++--
3 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 6052f7403032..11312aeb6ff6 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2860,9 +2860,9 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
u64 inode_objectid, u64 ref_objectid, int ins_len,
int cow);
-int btrfs_find_name_in_backref(struct extent_buffer *leaf, int slot,
- const char *name,
- int name_len, struct btrfs_inode_ref **ref_ret);
+struct btrfs_inode_ref *btrfs_find_name_in_backref(struct extent_buffer *leaf,
+ int slot, const char *name,
+ int name_len);
int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
u64 ref_objectid, const char *name,
int name_len,
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
index 30d62ef918b9..6f43bedd3700 100644
--- a/fs/btrfs/inode-item.c
+++ b/fs/btrfs/inode-item.c
@@ -8,9 +8,9 @@
#include "transaction.h"
#include "print-tree.h"
-int btrfs_find_name_in_backref(struct extent_buffer *leaf, int slot,
- const char *name,
- int name_len, struct btrfs_inode_ref **ref_ret)
+struct btrfs_inode_ref *btrfs_find_name_in_backref(struct extent_buffer *leaf,
+ int slot, const char *name,
+ int name_len)
{
struct btrfs_inode_ref *ref;
unsigned long ptr;
@@ -28,13 +28,10 @@ int btrfs_find_name_in_backref(struct extent_buffer *leaf, int slot,
cur_offset += len + sizeof(*ref);
if (len != name_len)
continue;
- if (memcmp_extent_buffer(leaf, name, name_ptr, name_len) == 0) {
- if (ref_ret)
- *ref_ret = ref;
- return 1;
- }
+ if (memcmp_extent_buffer(leaf, name, name_ptr, name_len) == 0)
+ return ref;
}
- return 0;
+ return NULL;
}
int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
@@ -213,8 +210,10 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans,
} else if (ret < 0) {
goto out;
}
- if (!btrfs_find_name_in_backref(path->nodes[0], path->slots[0],
- name, name_len, &ref)) {
+
+ ref = btrfs_find_name_in_backref(path->nodes[0], path->slots[0], name,
+ name_len);
+ if (!ref) {
ret = -ENOENT;
search_ext_refs = 1;
goto out;
@@ -341,9 +340,9 @@ int btrfs_insert_inode_ref(struct btrfs_trans_handle *trans,
ins_len);
if (ret == -EEXIST) {
u32 old_size;
-
- if (btrfs_find_name_in_backref(path->nodes[0], path->slots[0],
- name, name_len, &ref))
+ ref = btrfs_find_name_in_backref(path->nodes[0], path->slots[0],
+ name, name_len);
+ if (ref)
goto out;
old_size = btrfs_item_size_nr(path->nodes[0], path->slots[0]);
@@ -359,7 +358,7 @@ int btrfs_insert_inode_ref(struct btrfs_trans_handle *trans,
if (ret == -EOVERFLOW) {
if (btrfs_find_name_in_backref(path->nodes[0],
path->slots[0],
- name, name_len, &ref))
+ name, name_len))
ret = -EEXIST;
else
ret = -EMLINK;
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 19a4b9dc669f..12151eb81976 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1271,7 +1271,7 @@ static int unlink_old_inode_refs(struct btrfs_trans_handle *trans,
namelen, NULL);
else
ret = btrfs_find_name_in_backref(log_eb, log_slot, name,
- namelen, NULL);
+ namelen) != NULL;
if (!ret) {
struct inode *dir;
@@ -1338,7 +1338,7 @@ static int btrfs_inode_ref_exists(struct inode *inode, struct inode *dir,
name, namelen, NULL);
else
ret = btrfs_find_name_in_backref(path->nodes[0], path->slots[0],
- name, namelen, NULL);
+ name, namelen) != NULL;
out:
btrfs_free_path(path);
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] btrfs: Make btrfs_find_name_in_ext_backref return struct btrfs_inode_extref
2019-08-27 11:46 [PATCH 0/3] Cleanup btrfs_find_name_in* functions Nikolay Borisov
2019-08-27 11:46 ` [PATCH 1/3] btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct Nikolay Borisov
@ 2019-08-27 11:46 ` Nikolay Borisov
2019-08-27 15:53 ` David Sterba
2019-08-27 11:46 ` [PATCH 3/3] btrfs: Use btrfs_find_name_in_backref in backref_in_log Nikolay Borisov
2019-08-27 16:04 ` [PATCH 0/3] Cleanup btrfs_find_name_in* functions David Sterba
3 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2019-08-27 11:46 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
btrfs_find_name_in_ext_backref returns either 0/1 depending on whether it
found a backref for the given name. If it returns true then the actual
inode_ref struct is returned in one of its parameters. That's pointless,
instead refactor the function such that it returns either a pointer
to the btrfs_inode_extref or NULL it it didn't find anything. This
streamlines the function calling convention.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/ctree.h | 9 ++++-----
fs/btrfs/inode-item.c | 33 +++++++++++++--------------------
fs/btrfs/tree-log.c | 6 +++---
3 files changed, 20 insertions(+), 28 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 11312aeb6ff6..8b9469df8e3f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2863,11 +2863,10 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
struct btrfs_inode_ref *btrfs_find_name_in_backref(struct extent_buffer *leaf,
int slot, const char *name,
int name_len);
-int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
- u64 ref_objectid, const char *name,
- int name_len,
- struct btrfs_inode_extref **extref_ret);
-
+struct btrfs_inode_extref *
+btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
+ u64 ref_objectid, const char *name,
+ int name_len);
/* file-item.c */
struct btrfs_dio_private;
int btrfs_del_csums(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
index 6f43bedd3700..d1ee56dde758 100644
--- a/fs/btrfs/inode-item.c
+++ b/fs/btrfs/inode-item.c
@@ -34,10 +34,9 @@ struct btrfs_inode_ref *btrfs_find_name_in_backref(struct extent_buffer *leaf,
return NULL;
}
-int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
- u64 ref_objectid,
- const char *name, int name_len,
- struct btrfs_inode_extref **extref_ret)
+struct btrfs_inode_extref *
+btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
+ u64 ref_objectid, const char *name, int name_len)
{
struct btrfs_inode_extref *extref;
unsigned long ptr;
@@ -62,15 +61,12 @@ int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
if (ref_name_len == name_len &&
btrfs_inode_extref_parent(leaf, extref) == ref_objectid &&
- (memcmp_extent_buffer(leaf, name, name_ptr, name_len) == 0)) {
- if (extref_ret)
- *extref_ret = extref;
- return 1;
- }
+ (memcmp_extent_buffer(leaf, name, name_ptr, name_len) == 0))
+ return extref;
cur_offset += ref_name_len + sizeof(*extref);
}
- return 0;
+ return NULL;
}
/* Returns NULL if no extref found */
@@ -84,7 +80,6 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
{
int ret;
struct btrfs_key key;
- struct btrfs_inode_extref *extref;
key.objectid = inode_objectid;
key.type = BTRFS_INODE_EXTREF_KEY;
@@ -95,11 +90,9 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
return ERR_PTR(ret);
if (ret > 0)
return NULL;
- if (!btrfs_find_name_in_ext_backref(path->nodes[0], path->slots[0],
- ref_objectid, name, name_len,
- &extref))
- return NULL;
- return extref;
+ return btrfs_find_name_in_ext_backref(path->nodes[0], path->slots[0],
+ ref_objectid, name, name_len);
+
}
static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
@@ -139,9 +132,9 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
* This should always succeed so error here will make the FS
* readonly.
*/
- if (!btrfs_find_name_in_ext_backref(path->nodes[0], path->slots[0],
- ref_objectid,
- name, name_len, &extref)) {
+ extref = btrfs_find_name_in_ext_backref(path->nodes[0], path->slots[0],
+ ref_objectid, name, name_len);
+ if (!extref) {
btrfs_handle_fs_error(root->fs_info, -ENOENT, NULL);
ret = -EROFS;
goto out;
@@ -284,7 +277,7 @@ static int btrfs_insert_inode_extref(struct btrfs_trans_handle *trans,
if (btrfs_find_name_in_ext_backref(path->nodes[0],
path->slots[0],
ref_objectid,
- name, name_len, NULL))
+ name, name_len))
goto out;
btrfs_extend_item(path, ins_len);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 12151eb81976..7d45a4869bc9 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -967,7 +967,7 @@ static noinline int backref_in_log(struct btrfs_root *log,
if (btrfs_find_name_in_ext_backref(path->nodes[0],
path->slots[0],
ref_objectid,
- name, namelen, NULL))
+ name, namelen))
match = 1;
goto out;
@@ -1268,7 +1268,7 @@ static int unlink_old_inode_refs(struct btrfs_trans_handle *trans,
if (key->type == BTRFS_INODE_EXTREF_KEY)
ret = btrfs_find_name_in_ext_backref(log_eb, log_slot,
parent_id, name,
- namelen, NULL);
+ namelen) != NULL;
else
ret = btrfs_find_name_in_backref(log_eb, log_slot, name,
namelen) != NULL;
@@ -1335,7 +1335,7 @@ static int btrfs_inode_ref_exists(struct inode *inode, struct inode *dir,
if (key.type == BTRFS_INODE_EXTREF_KEY)
ret = btrfs_find_name_in_ext_backref(path->nodes[0],
path->slots[0], parent_id,
- name, namelen, NULL);
+ name, namelen) != NULL;
else
ret = btrfs_find_name_in_backref(path->nodes[0], path->slots[0],
name, namelen) != NULL;
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] btrfs: Use btrfs_find_name_in_backref in backref_in_log
2019-08-27 11:46 [PATCH 0/3] Cleanup btrfs_find_name_in* functions Nikolay Borisov
2019-08-27 11:46 ` [PATCH 1/3] btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct Nikolay Borisov
2019-08-27 11:46 ` [PATCH 2/3] btrfs: Make btrfs_find_name_in_ext_backref return struct btrfs_inode_extref Nikolay Borisov
@ 2019-08-27 11:46 ` Nikolay Borisov
2019-08-27 16:03 ` David Sterba
2019-08-27 16:04 ` [PATCH 0/3] Cleanup btrfs_find_name_in* functions David Sterba
3 siblings, 1 reply; 9+ messages in thread
From: Nikolay Borisov @ 2019-08-27 11:46 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
By not opencoding btrfs_find_name_in_backref most of the local variables
can be removed, this in turn alleviates stack pressure. Additionally,
backref_in_log is only used as predicate so make it return bool.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/tree-log.c | 57 +++++++++++++--------------------------------
1 file changed, 16 insertions(+), 41 deletions(-)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 7d45a4869bc9..070016e023b8 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -938,60 +938,35 @@ static noinline int inode_in_dir(struct btrfs_root *root,
* want to delete valid links to a file from the subvolume if that
* link is also in the log.
*/
-static noinline int backref_in_log(struct btrfs_root *log,
- struct btrfs_key *key,
- u64 ref_objectid,
- const char *name, int namelen)
+static noinline bool backref_in_log(struct btrfs_root *log,
+ struct btrfs_key *key,
+ u64 ref_objectid,
+ const char *name, int namelen)
{
struct btrfs_path *path;
- struct btrfs_inode_ref *ref;
- unsigned long ptr;
- unsigned long ptr_end;
- unsigned long name_ptr;
- int found_name_len;
- int item_size;
+ bool found = false;
int ret;
- int match = 0;
path = btrfs_alloc_path();
if (!path)
- return -ENOMEM;
+ return false;
ret = btrfs_search_slot(NULL, log, key, path, 0, 0);
if (ret != 0)
goto out;
- ptr = btrfs_item_ptr_offset(path->nodes[0], path->slots[0]);
-
- if (key->type == BTRFS_INODE_EXTREF_KEY) {
- if (btrfs_find_name_in_ext_backref(path->nodes[0],
- path->slots[0],
- ref_objectid,
- name, namelen))
- match = 1;
-
- goto out;
- }
-
- item_size = btrfs_item_size_nr(path->nodes[0], path->slots[0]);
- ptr_end = ptr + item_size;
- while (ptr < ptr_end) {
- ref = (struct btrfs_inode_ref *)ptr;
- found_name_len = btrfs_inode_ref_name_len(path->nodes[0], ref);
- if (found_name_len == namelen) {
- name_ptr = (unsigned long)(ref + 1);
- ret = memcmp_extent_buffer(path->nodes[0], name,
- name_ptr, namelen);
- if (ret == 0) {
- match = 1;
- goto out;
- }
- }
- ptr = (unsigned long)(ref + 1) + found_name_len;
- }
+ if (key->type == BTRFS_INODE_EXTREF_KEY)
+ found = !!btrfs_find_name_in_ext_backref(path->nodes[0],
+ path->slots[0],
+ ref_objectid,
+ name, namelen);
+ else
+ found = !!btrfs_find_name_in_backref(path->nodes[0],
+ path->slots[0],
+ name, namelen);
out:
btrfs_free_path(path);
- return match;
+ return found;
}
static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct
2019-08-27 11:46 ` [PATCH 1/3] btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct Nikolay Borisov
@ 2019-08-27 14:13 ` David Sterba
2019-08-27 15:45 ` David Sterba
0 siblings, 1 reply; 9+ messages in thread
From: David Sterba @ 2019-08-27 14:13 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Tue, Aug 27, 2019 at 02:46:28PM +0300, Nikolay Borisov wrote:
> ret = btrfs_find_name_in_backref(log_eb, log_slot, name,
> - namelen, NULL);
> + namelen) != NULL;
Isn't there a less confusing and obscure way to do that?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct
2019-08-27 14:13 ` David Sterba
@ 2019-08-27 15:45 ` David Sterba
0 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-08-27 15:45 UTC (permalink / raw)
To: dsterba, Nikolay Borisov, linux-btrfs
On Tue, Aug 27, 2019 at 04:13:41PM +0200, David Sterba wrote:
> On Tue, Aug 27, 2019 at 02:46:28PM +0300, Nikolay Borisov wrote:
> > ret = btrfs_find_name_in_backref(log_eb, log_slot, name,
> > - namelen, NULL);
> > + namelen) != NULL;
>
> Isn't there a less confusing and obscure way to do that?
As patch 3/3 does, ret = !!btrfs_..., I'll switch to it where needed.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] btrfs: Make btrfs_find_name_in_ext_backref return struct btrfs_inode_extref
2019-08-27 11:46 ` [PATCH 2/3] btrfs: Make btrfs_find_name_in_ext_backref return struct btrfs_inode_extref Nikolay Borisov
@ 2019-08-27 15:53 ` David Sterba
0 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-08-27 15:53 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Tue, Aug 27, 2019 at 02:46:29PM +0300, Nikolay Borisov wrote:
> btrfs_find_name_in_ext_backref returns either 0/1 depending on whether it
> found a backref for the given name. If it returns true then the actual
> inode_ref struct is returned in one of its parameters. That's pointless,
> instead refactor the function such that it returns either a pointer
> to the btrfs_inode_extref or NULL it it didn't find anything. This
> streamlines the function calling convention.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/ctree.h | 9 ++++-----
> fs/btrfs/inode-item.c | 33 +++++++++++++--------------------
> fs/btrfs/tree-log.c | 6 +++---
> 3 files changed, 20 insertions(+), 28 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 11312aeb6ff6..8b9469df8e3f 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2863,11 +2863,10 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
> struct btrfs_inode_ref *btrfs_find_name_in_backref(struct extent_buffer *leaf,
> int slot, const char *name,
> int name_len);
> -int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
> - u64 ref_objectid, const char *name,
> - int name_len,
> - struct btrfs_inode_extref **extref_ret);
> -
> +struct btrfs_inode_extref *
> +btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
> + u64 ref_objectid, const char *name,
> + int name_len);
Please use the common style for function declarations/definitions with
type and name on one line.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] btrfs: Use btrfs_find_name_in_backref in backref_in_log
2019-08-27 11:46 ` [PATCH 3/3] btrfs: Use btrfs_find_name_in_backref in backref_in_log Nikolay Borisov
@ 2019-08-27 16:03 ` David Sterba
0 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-08-27 16:03 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Tue, Aug 27, 2019 at 02:46:30PM +0300, Nikolay Borisov wrote:
> By not opencoding btrfs_find_name_in_backref most of the local variables
> can be removed, this in turn alleviates stack pressure. Additionally,
> backref_in_log is only used as predicate so make it return bool.
While it is used as bool, it returns error numbers so the callsites
should be updated to distinguish errors from true/false.
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/tree-log.c | 57 +++++++++++++--------------------------------
> 1 file changed, 16 insertions(+), 41 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 7d45a4869bc9..070016e023b8 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -938,60 +938,35 @@ static noinline int inode_in_dir(struct btrfs_root *root,
> * want to delete valid links to a file from the subvolume if that
> * link is also in the log.
> */
> -static noinline int backref_in_log(struct btrfs_root *log,
> - struct btrfs_key *key,
> - u64 ref_objectid,
> - const char *name, int namelen)
> +static noinline bool backref_in_log(struct btrfs_root *log,
> + struct btrfs_key *key,
> + u64 ref_objectid,
> + const char *name, int namelen)
> {
> struct btrfs_path *path;
> - struct btrfs_inode_ref *ref;
> - unsigned long ptr;
> - unsigned long ptr_end;
> - unsigned long name_ptr;
> - int found_name_len;
> - int item_size;
> + bool found = false;
> int ret;
> - int match = 0;
>
> path = btrfs_alloc_path();
> if (!path)
> - return -ENOMEM;
> + return false;
There would have to be a very good reasoning behind that but I doubt
it's correct.
> ret = btrfs_search_slot(NULL, log, key, path, 0, 0);
> if (ret != 0)
This should also check for errors, though it's not changed in your
patch.
So if you want to de-opencode btrfs_find_name_in_backref, then fine, but
the other changes should be separate and possibly updating the whole
callchain.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] Cleanup btrfs_find_name_in* functions
2019-08-27 11:46 [PATCH 0/3] Cleanup btrfs_find_name_in* functions Nikolay Borisov
` (2 preceding siblings ...)
2019-08-27 11:46 ` [PATCH 3/3] btrfs: Use btrfs_find_name_in_backref in backref_in_log Nikolay Borisov
@ 2019-08-27 16:04 ` David Sterba
3 siblings, 0 replies; 9+ messages in thread
From: David Sterba @ 2019-08-27 16:04 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Tue, Aug 27, 2019 at 02:46:27PM +0300, Nikolay Borisov wrote:
> This series cleans up the calling convention of the 2 function used to search
> the INODE_REF items for the presence of particular name. Namely, I switch the 2
> functions to directly return a struct btrfs_inode_(ext)?ref. This reduces the
> number of parameters when calling the functions.
>
> While doing the aforementioned cleanup I stumbled upon backref_in_log which was
> opencoding btrfs_find_name_in_backref hence I just refactored the function.
>
> This series survived xfstest.
>
> Nikolay Borisov (3):
> btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct
> btrfs: Make btrfs_find_name_in_ext_backref return struct
> btrfs_inode_extref
> btrfs: Use btrfs_find_name_in_backref in backref_in_log
1 and 2 merged to misc-next, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-08-27 16:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 11:46 [PATCH 0/3] Cleanup btrfs_find_name_in* functions Nikolay Borisov
2019-08-27 11:46 ` [PATCH 1/3] btrfs: Make btrfs_find_name_in_backref return btrfs_inode_ref struct Nikolay Borisov
2019-08-27 14:13 ` David Sterba
2019-08-27 15:45 ` David Sterba
2019-08-27 11:46 ` [PATCH 2/3] btrfs: Make btrfs_find_name_in_ext_backref return struct btrfs_inode_extref Nikolay Borisov
2019-08-27 15:53 ` David Sterba
2019-08-27 11:46 ` [PATCH 3/3] btrfs: Use btrfs_find_name_in_backref in backref_in_log Nikolay Borisov
2019-08-27 16:03 ` David Sterba
2019-08-27 16:04 ` [PATCH 0/3] Cleanup btrfs_find_name_in* functions David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).