* [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots
@ 2022-03-02 16:48 Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 01/14] btrfs: Introduce btrfs_for_each_slot iterator macro Gabriel Niebler
` (14 more replies)
0 siblings, 15 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler
There is a common pattern when searching for a key in btrfs:
* Call btrfs_search_slot to find the slot for the key
* Enter an endless loop:
* If the found slot is larger than the no. of items in the current leaf,
check the next leaf
* If it's still not found in the next leaf, terminate the loop
* Otherwise do something with the found key
* Increment the current slot and continue
To reduce code duplication, we can replace this code pattern with an iterator
macro, similar to the existing for_each_X macros found elsewhere in the kernel.
This also makes the code easier to understand for newcomers by putting a name
to the encapsulated functionality.
This patchset survived a complete fstest run.
Changes from v2:
* Rename btrfs_valid_slot to btrfs_get_next_valid_item (Nikolay)
* Fix comment formatting (David)
* Remove redundant parentheses and indentation in loop condition (David)
* Remove redundant iter_ret var and reuse ret instead (Nikolay)
* Make termination condition more consistent in btrfs_unlink_all_paths
(Nikolay)
* Improved patch organisation by splitting into one patch per function (David)
* Improve doc comment for btrfs_get_next_valid_item (Gabriel)
* Remove `out` label and assoc. gotos from id_create_dir (Gabriel)
* Initialise `ret` in process_all_refs and process_all_new_xattrs (Gabriel)
* Remove unneeded btrfs_item_key_to_cpu call from loop body in
btrfs_read_chunk_tree (Gabriel)
Changes from v1:
* Separate xattr changes from the macro introducing code (Johannes)
Changes from RFC:
* Add documentation to btrfs_for_each_slot macro and btrfs_valid_slot function
(David)
* Add documentation about the ret variable used as a macro argument (David)
* Match function argument from prototype and implementation (David)
* Changed ({ }) block to only () in btrfs_for_each_slot macro (David)
* Add more patches to show the code being reduced by using this approach
(Nikolay)
Marcos Paulo de Souza (14):
btrfs: Introduce btrfs_for_each_slot iterator macro
btrfs: Use btrfs_for_each_slot in find_first_block_group
btrfs: Use btrfs_for_each_slot in mark_block_group_to_copy
btrfs: Use btrfs_for_each_slot in btrfs_search_dir_index_item
btrfs: Use btrfs_for_each_slot in btrfs_real_readdir
btrfs: Use btrfs_for_each_slot in did_create_dir
btrfs: Use btrfs_for_each_slot in can_rmdir
btrfs: Use btrfs_for_each_slot in is_ancestor
btrfs: Use btrfs_for_each_slot in process_all_refs
btrfs: Use btrfs_for_each_slot in process_all_new_xattrs
btrfs: Use btrfs_for_each_slot in process_all_extents
btrfs: Use btrfs_for_each_slot in btrfs_unlink_all_paths
btrfs: Use btrfs_for_each_slot in btrfs_read_chunk_tree
btrfs: Use btrfs_for_each_slot in btrfs_listxattr
fs/btrfs/block-group.c | 26 +-----
fs/btrfs/ctree.c | 33 +++++++
fs/btrfs/ctree.h | 24 ++++++
fs/btrfs/dev-replace.c | 41 ++-------
fs/btrfs/dir-item.c | 31 ++-----
fs/btrfs/inode.c | 35 +++-----
fs/btrfs/send.c | 229 ++++++++++++++-----------------------------------
fs/btrfs/volumes.c | 26 ++----
fs/btrfs/xattr.c | 41 +++------
9 files changed, 168 insertions(+), 318 deletions(-)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 01/14] btrfs: Introduce btrfs_for_each_slot iterator macro
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-08 14:31 ` David Sterba
2022-03-02 16:48 ` [PATCH v3 02/14] btrfs: Use btrfs_for_each_slot in find_first_block_group Gabriel Niebler
` (13 subsequent siblings)
14 siblings, 1 reply; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
There is a common pattern when searching for a key in btrfs:
* Call btrfs_search_slot to find the slot for the key
* Enter an endless loop:
* If the found slot is larger than the no. of items in the current leaf,
check the next leaf
* If it's still not found in the next leaf, terminate the loop
* Otherwise do something with the found key
* Increment the current slot and continue
To reduce code duplication, we can replace this code pattern with an iterator
macro, similar to the existing for_each_X macros found elsewhere in the kernel.
This also makes the code easier to understand for newcomers by putting a name
to the encapsulated functionality.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/ctree.c | 33 +++++++++++++++++++++++++++++++++
fs/btrfs/ctree.h | 24 ++++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index a7db3f6f1b7b..d735bd472616 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2277,6 +2277,39 @@ int btrfs_search_backwards(struct btrfs_root *root, struct btrfs_key *key,
return ret;
}
+/* Search for a valid slot for the given path.
+ *
+ * @root: The root node of the tree.
+ * @key: Will contain a valid item if found.
+ * @path: The starting point to validate the slot.
+ *
+ * Return 0 if the item is valid, 1 if not found and < 0 if error.
+ */
+int btrfs_get_next_valid_item(struct btrfs_root *root, struct btrfs_key *key,
+ struct btrfs_path *path)
+{
+ while (1) {
+ int ret;
+ const int slot = path->slots[0];
+ const struct extent_buffer *leaf = path->nodes[0];
+ /* this is where we start walking through the path */
+ if (slot >= btrfs_header_nritems(leaf)) {
+ /*
+ * if we've reached the last slot in this leaf we need
+ * to go to the next leaf and reset the path
+ */
+ ret = btrfs_next_leaf(root, path);
+ if (ret)
+ return ret;
+ continue;
+ }
+ /* store the found, valid item in key */
+ btrfs_item_key_to_cpu(leaf, key, slot);
+ break;
+ }
+ return 0;
+}
+
/*
* adjust the pointers going up the tree, starting at level
* making sure the right key of each node is points to 'key'.
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 947f04789389..98091334b749 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2976,6 +2976,30 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
int btrfs_search_backwards(struct btrfs_root *root, struct btrfs_key *key,
struct btrfs_path *path);
+int btrfs_get_next_valid_item(struct btrfs_root *root, struct btrfs_key *key,
+ struct btrfs_path *path);
+
+/* Search in @root for a given @key, and store the slot found in @found_key.
+ *
+ * @root: The root node of the tree.
+ * @key: The key we are looking for.
+ * @found_key: Will hold the found item.
+ * @path: Holds the current slot/leaf.
+ * @iter_ret: Contains the value returned from btrfs_search_slot or
+ * btrfs_get_next_valid_item, whichever was executed last.
+ *
+ * The iter_ret is an output variable that will contain the return value of
+ * btrfs_search_slot, if it encountered an error, or the value returned from
+ * btrfs_get_next_valid_item, otherwise. That return value can be 0, if a valid
+ * slot was found, 1 if there were no more leaves, and <0 if there was an error.
+ */
+#define btrfs_for_each_slot(root, key, found_key, path, iter_ret) \
+ for (iter_ret = btrfs_search_slot(NULL, root, key, path, 0, 0); \
+ iter_ret >= 0 && \
+ (iter_ret = btrfs_get_next_valid_item(root, found_key, path)) == 0; \
+ path->slots[0]++ \
+ )
+
static inline int btrfs_next_old_item(struct btrfs_root *root,
struct btrfs_path *p, u64 time_seq)
{
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 02/14] btrfs: Use btrfs_for_each_slot in find_first_block_group
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 01/14] btrfs: Introduce btrfs_for_each_slot iterator macro Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 03/14] btrfs: Use btrfs_for_each_slot in mark_block_group_to_copy Gabriel Niebler
` (12 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/block-group.c | 26 ++------------------------
1 file changed, 2 insertions(+), 24 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 8202ad6aa131..aafd7909d0f8 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1686,35 +1686,13 @@ static int find_first_block_group(struct btrfs_fs_info *fs_info,
struct btrfs_root *root = btrfs_block_group_root(fs_info);
int ret;
struct btrfs_key found_key;
- struct extent_buffer *leaf;
- int slot;
-
- ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
- if (ret < 0)
- return ret;
-
- while (1) {
- slot = path->slots[0];
- leaf = path->nodes[0];
- if (slot >= btrfs_header_nritems(leaf)) {
- ret = btrfs_next_leaf(root, path);
- if (ret == 0)
- continue;
- if (ret < 0)
- goto out;
- break;
- }
- btrfs_item_key_to_cpu(leaf, &found_key, slot);
+ btrfs_for_each_slot(root, key, &found_key, path, ret) {
if (found_key.objectid >= key->objectid &&
found_key.type == BTRFS_BLOCK_GROUP_ITEM_KEY) {
- ret = read_bg_from_eb(fs_info, &found_key, path);
- break;
+ return read_bg_from_eb(fs_info, &found_key, path);
}
-
- path->slots[0]++;
}
-out:
return ret;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 03/14] btrfs: Use btrfs_for_each_slot in mark_block_group_to_copy
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 01/14] btrfs: Introduce btrfs_for_each_slot iterator macro Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 02/14] btrfs: Use btrfs_for_each_slot in find_first_block_group Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-08 14:33 ` David Sterba
2022-03-02 16:48 ` [PATCH v3 04/14] btrfs: Use btrfs_for_each_slot in btrfs_search_dir_index_item Gabriel Niebler
` (11 subsequent siblings)
14 siblings, 1 reply; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/dev-replace.c | 41 ++++++++---------------------------------
1 file changed, 8 insertions(+), 33 deletions(-)
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 62b9651ea662..3357739f427f 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -470,6 +470,7 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
struct btrfs_dev_extent *dev_extent = NULL;
struct btrfs_block_group *cache;
struct btrfs_trans_handle *trans;
+ int iter_ret = 0;
int ret = 0;
u64 chunk_offset;
@@ -520,29 +521,8 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
key.type = BTRFS_DEV_EXTENT_KEY;
key.offset = 0;
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto free_path;
- if (ret > 0) {
- if (path->slots[0] >=
- btrfs_header_nritems(path->nodes[0])) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- goto free_path;
- if (ret > 0) {
- ret = 0;
- goto free_path;
- }
- } else {
- ret = 0;
- }
- }
-
- while (1) {
+ btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
struct extent_buffer *leaf = path->nodes[0];
- int slot = path->slots[0];
-
- btrfs_item_key_to_cpu(leaf, &found_key, slot);
if (found_key.objectid != src_dev->devid)
break;
@@ -553,30 +533,25 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
if (found_key.offset < key.offset)
break;
- dev_extent = btrfs_item_ptr(leaf, slot, struct btrfs_dev_extent);
+ dev_extent = btrfs_item_ptr(leaf, path->slots[0],
+ struct btrfs_dev_extent);
chunk_offset = btrfs_dev_extent_chunk_offset(leaf, dev_extent);
cache = btrfs_lookup_block_group(fs_info, chunk_offset);
if (!cache)
- goto skip;
+ continue;
spin_lock(&cache->lock);
cache->to_copy = 1;
spin_unlock(&cache->lock);
btrfs_put_block_group(cache);
-
-skip:
- ret = btrfs_next_item(root, path);
- if (ret != 0) {
- if (ret > 0)
- ret = 0;
- break;
- }
+ }
+ if (iter_ret < 0) {
+ ret = iter_ret;
}
-free_path:
btrfs_free_path(path);
unlock:
mutex_unlock(&fs_info->chunk_mutex);
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 04/14] btrfs: Use btrfs_for_each_slot in btrfs_search_dir_index_item
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (2 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 03/14] btrfs: Use btrfs_for_each_slot in mark_block_group_to_copy Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 05/14] btrfs: Use btrfs_for_each_slot in btrfs_real_readdir Gabriel Niebler
` (10 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/dir-item.c | 31 ++++++-------------------------
1 file changed, 6 insertions(+), 25 deletions(-)
diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index 3b532bab0755..d7a24f17292d 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -325,36 +325,15 @@ btrfs_search_dir_index_item(struct btrfs_root *root,
struct btrfs_path *path, u64 dirid,
const char *name, int name_len)
{
- struct extent_buffer *leaf;
struct btrfs_dir_item *di;
struct btrfs_key key;
- u32 nritems;
int ret;
key.objectid = dirid;
key.type = BTRFS_DIR_INDEX_KEY;
key.offset = 0;
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- return ERR_PTR(ret);
-
- leaf = path->nodes[0];
- nritems = btrfs_header_nritems(leaf);
-
- while (1) {
- if (path->slots[0] >= nritems) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- return ERR_PTR(ret);
- if (ret > 0)
- break;
- leaf = path->nodes[0];
- nritems = btrfs_header_nritems(leaf);
- continue;
- }
-
- btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
+ btrfs_for_each_slot(root, &key, &key, path, ret) {
if (key.objectid != dirid || key.type != BTRFS_DIR_INDEX_KEY)
break;
@@ -362,10 +341,12 @@ btrfs_search_dir_index_item(struct btrfs_root *root,
name, name_len);
if (di)
return di;
-
- path->slots[0]++;
}
- return NULL;
+ /* Fix return code if key was not found in next leaf. */
+ if (ret > 0) {
+ ret = 0;
+ }
+ return ERR_PTR(ret);
}
struct btrfs_dir_item *btrfs_lookup_xattr(struct btrfs_trans_handle *trans,
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 05/14] btrfs: Use btrfs_for_each_slot in btrfs_real_readdir
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (3 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 04/14] btrfs: Use btrfs_for_each_slot in btrfs_search_dir_index_item Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 06/14] btrfs: Use btrfs_for_each_slot in did_create_dir Gabriel Niebler
` (9 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/inode.c | 35 ++++++++++-------------------------
1 file changed, 10 insertions(+), 25 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 76e530f76e3c..144ca1a9d57f 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5755,8 +5755,6 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
struct list_head ins_list;
struct list_head del_list;
int ret;
- struct extent_buffer *leaf;
- int slot;
char *name_ptr;
int name_len;
int entries = 0;
@@ -5783,35 +5781,20 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
key.offset = ctx->pos;
key.objectid = btrfs_ino(BTRFS_I(inode));
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto err;
-
- while (1) {
+ btrfs_for_each_slot(root, &key, &found_key, path, ret) {
struct dir_entry *entry;
-
- leaf = path->nodes[0];
- slot = path->slots[0];
- if (slot >= btrfs_header_nritems(leaf)) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- goto err;
- else if (ret > 0)
- break;
- continue;
- }
-
- btrfs_item_key_to_cpu(leaf, &found_key, slot);
+ struct extent_buffer *leaf = path->nodes[0];
if (found_key.objectid != key.objectid)
break;
if (found_key.type != BTRFS_DIR_INDEX_KEY)
break;
if (found_key.offset < ctx->pos)
- goto next;
+ continue;
if (btrfs_should_delete_dir_index(&del_list, found_key.offset))
- goto next;
- di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
+ continue;
+ di = btrfs_item_ptr(leaf, path->slots[0],
+ struct btrfs_dir_item);
name_len = btrfs_dir_name_len(leaf, di);
if ((total_len + sizeof(struct dir_entry) + name_len) >=
PAGE_SIZE) {
@@ -5838,8 +5821,10 @@ static int btrfs_real_readdir(struct file *file, struct dir_context *ctx)
entries++;
addr += sizeof(struct dir_entry) + name_len;
total_len += sizeof(struct dir_entry) + name_len;
-next:
- path->slots[0]++;
+ }
+ /* Catch error encountered while searching */
+ if (ret < 0) {
+ goto err;
}
btrfs_release_path(path);
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 06/14] btrfs: Use btrfs_for_each_slot in did_create_dir
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (4 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 05/14] btrfs: Use btrfs_for_each_slot in btrfs_real_readdir Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 07/14] btrfs: Use btrfs_for_each_slot in can_rmdir Gabriel Niebler
` (8 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/send.c | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 201eb2628aea..09715d98145a 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -2680,61 +2680,45 @@ static int send_create_inode(struct send_ctx *sctx, u64 ino)
static int did_create_dir(struct send_ctx *sctx, u64 dir)
{
int ret = 0;
+ int iter_ret = 0;
struct btrfs_path *path = NULL;
struct btrfs_key key;
struct btrfs_key found_key;
struct btrfs_key di_key;
- struct extent_buffer *eb;
struct btrfs_dir_item *di;
- int slot;
path = alloc_path_for_send();
if (!path) {
- ret = -ENOMEM;
- goto out;
+ return -ENOMEM;
}
key.objectid = dir;
key.type = BTRFS_DIR_INDEX_KEY;
key.offset = 0;
- ret = btrfs_search_slot(NULL, sctx->send_root, &key, path, 0, 0);
- if (ret < 0)
- goto out;
- while (1) {
- eb = path->nodes[0];
- slot = path->slots[0];
- if (slot >= btrfs_header_nritems(eb)) {
- ret = btrfs_next_leaf(sctx->send_root, path);
- if (ret < 0) {
- goto out;
- } else if (ret > 0) {
- ret = 0;
- break;
- }
- continue;
- }
+ btrfs_for_each_slot(sctx->send_root, &key, &found_key, path, iter_ret) {
+ struct extent_buffer *eb = path->nodes[0];
- btrfs_item_key_to_cpu(eb, &found_key, slot);
if (found_key.objectid != key.objectid ||
found_key.type != key.type) {
ret = 0;
- goto out;
+ break;
}
- di = btrfs_item_ptr(eb, slot, struct btrfs_dir_item);
+ di = btrfs_item_ptr(eb, path->slots[0], struct btrfs_dir_item);
btrfs_dir_item_key_to_cpu(eb, di, &di_key);
if (di_key.type != BTRFS_ROOT_ITEM_KEY &&
di_key.objectid < sctx->send_progress) {
ret = 1;
- goto out;
+ break;
}
-
- path->slots[0]++;
+ }
+ /* Catch error found on iteration */
+ if (iter_ret < 0) {
+ ret = iter_ret;
}
-out:
btrfs_free_path(path);
return ret;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 07/14] btrfs: Use btrfs_for_each_slot in can_rmdir
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (5 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 06/14] btrfs: Use btrfs_for_each_slot in did_create_dir Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 08/14] btrfs: Use btrfs_for_each_slot in is_ancestor Gabriel Niebler
` (7 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/send.c | 23 ++++++-----------------
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 09715d98145a..7b6d1a65793e 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -2922,6 +2922,7 @@ static int can_rmdir(struct send_ctx *sctx, u64 dir, u64 dir_gen,
u64 send_progress)
{
int ret = 0;
+ int iter_ret = 0;
struct btrfs_root *root = sctx->parent_root;
struct btrfs_path *path;
struct btrfs_key key;
@@ -2948,23 +2949,9 @@ static int can_rmdir(struct send_ctx *sctx, u64 dir, u64 dir_gen,
if (odi)
key.offset = odi->last_dir_index_offset;
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto out;
-
- while (1) {
+ btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
struct waiting_dir_move *dm;
- if (path->slots[0] >= btrfs_header_nritems(path->nodes[0])) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- goto out;
- else if (ret > 0)
- break;
- continue;
- }
- btrfs_item_key_to_cpu(path->nodes[0], &found_key,
- path->slots[0]);
if (found_key.objectid != key.objectid ||
found_key.type != key.type)
break;
@@ -2999,8 +2986,10 @@ static int can_rmdir(struct send_ctx *sctx, u64 dir, u64 dir_gen,
ret = 0;
goto out;
}
-
- path->slots[0]++;
+ }
+ if (iter_ret < 0) {
+ ret = iter_ret;
+ goto out;
}
free_orphan_dir_info(sctx, odi);
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 08/14] btrfs: Use btrfs_for_each_slot in is_ancestor
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (6 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 07/14] btrfs: Use btrfs_for_each_slot in can_rmdir Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 09/14] btrfs: Use btrfs_for_each_slot in process_all_refs Gabriel Niebler
` (6 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/send.c | 25 +++++++------------------
1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 7b6d1a65793e..bb797f411daf 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -3557,7 +3557,7 @@ static int check_ino_in_path(struct btrfs_root *root,
}
/*
- * Check if ino ino1 is an ancestor of inode ino2 in the given root for any
+ * Check if inode ino1 is an ancestor of inode ino2 in the given root for any
* possible path (in case ino2 is not a directory and has multiple hard links).
* Return 1 if true, 0 if false and < 0 on error.
*/
@@ -3569,6 +3569,7 @@ static int is_ancestor(struct btrfs_root *root,
{
bool free_fs_path = false;
int ret = 0;
+ int iter_ret = 0;
struct btrfs_path *path = NULL;
struct btrfs_key key;
@@ -3589,26 +3590,12 @@ static int is_ancestor(struct btrfs_root *root,
key.type = BTRFS_INODE_REF_KEY;
key.offset = 0;
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto out;
-
- while (true) {
+ btrfs_for_each_slot(root, &key, &key, path, iter_ret) {
struct extent_buffer *leaf = path->nodes[0];
int slot = path->slots[0];
u32 cur_offset = 0;
u32 item_size;
- if (slot >= btrfs_header_nritems(leaf)) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- goto out;
- if (ret > 0)
- break;
- continue;
- }
-
- btrfs_item_key_to_cpu(leaf, &key, slot);
if (key.objectid != ino2)
break;
if (key.type != BTRFS_INODE_REF_KEY &&
@@ -3646,10 +3633,12 @@ static int is_ancestor(struct btrfs_root *root,
if (ret)
goto out;
}
- path->slots[0]++;
}
ret = 0;
- out:
+ if (iter_ret < 0) {
+ ret = iter_ret;
+ }
+out:
btrfs_free_path(path);
if (free_fs_path)
fs_path_free(fs_path);
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 09/14] btrfs: Use btrfs_for_each_slot in process_all_refs
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (7 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 08/14] btrfs: Use btrfs_for_each_slot in is_ancestor Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 10/14] btrfs: Use btrfs_for_each_slot in process_all_new_xattrs Gabriel Niebler
` (5 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/send.c | 33 +++++++++------------------------
1 file changed, 9 insertions(+), 24 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index bb797f411daf..e54f168b6fc6 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -4518,13 +4518,12 @@ static int record_changed_ref(struct send_ctx *sctx)
static int process_all_refs(struct send_ctx *sctx,
enum btrfs_compare_tree_result cmd)
{
- int ret;
+ int ret = 0;
+ int iter_ret = 0;
struct btrfs_root *root;
struct btrfs_path *path;
struct btrfs_key key;
struct btrfs_key found_key;
- struct extent_buffer *eb;
- int slot;
iterate_inode_ref_t cb;
int pending_move = 0;
@@ -4540,7 +4539,7 @@ static int process_all_refs(struct send_ctx *sctx,
cb = __record_deleted_ref;
} else {
btrfs_err(sctx->send_root->fs_info,
- "Wrong command %d in process_all_refs", cmd);
+ "Wrong command %d in process_all_refs", cmd);
ret = -EINVAL;
goto out;
}
@@ -4548,24 +4547,7 @@ static int process_all_refs(struct send_ctx *sctx,
key.objectid = sctx->cmp_key->objectid;
key.type = BTRFS_INODE_REF_KEY;
key.offset = 0;
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto out;
-
- while (1) {
- eb = path->nodes[0];
- slot = path->slots[0];
- if (slot >= btrfs_header_nritems(eb)) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- goto out;
- else if (ret > 0)
- break;
- continue;
- }
-
- btrfs_item_key_to_cpu(eb, &found_key, slot);
-
+ btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
if (found_key.objectid != key.objectid ||
(found_key.type != BTRFS_INODE_REF_KEY &&
found_key.type != BTRFS_INODE_EXTREF_KEY))
@@ -4574,8 +4556,11 @@ static int process_all_refs(struct send_ctx *sctx,
ret = iterate_inode_ref(root, path, &found_key, 0, cb, sctx);
if (ret < 0)
goto out;
-
- path->slots[0]++;
+ }
+ /* Catch error found on iteration */
+ if (iter_ret < 0) {
+ ret = iter_ret;
+ goto out;
}
btrfs_release_path(path);
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 10/14] btrfs: Use btrfs_for_each_slot in process_all_new_xattrs
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (8 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 09/14] btrfs: Use btrfs_for_each_slot in process_all_refs Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 11/14] btrfs: Use btrfs_for_each_slot in process_all_extents Gabriel Niebler
` (4 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/send.c | 36 +++++++++---------------------------
1 file changed, 9 insertions(+), 27 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index e54f168b6fc6..2057aca8c3eb 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -4822,13 +4822,12 @@ static int process_changed_xattr(struct send_ctx *sctx)
static int process_all_new_xattrs(struct send_ctx *sctx)
{
- int ret;
+ int ret = 0;
+ int iter_ret = 0;
struct btrfs_root *root;
struct btrfs_path *path;
struct btrfs_key key;
struct btrfs_key found_key;
- struct extent_buffer *eb;
- int slot;
path = alloc_path_for_send();
if (!path)
@@ -4839,39 +4838,22 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
key.objectid = sctx->cmp_key->objectid;
key.type = BTRFS_XATTR_ITEM_KEY;
key.offset = 0;
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto out;
-
- while (1) {
- eb = path->nodes[0];
- slot = path->slots[0];
- if (slot >= btrfs_header_nritems(eb)) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0) {
- goto out;
- } else if (ret > 0) {
- ret = 0;
- break;
- }
- continue;
- }
-
- btrfs_item_key_to_cpu(eb, &found_key, slot);
+ btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
if (found_key.objectid != key.objectid ||
found_key.type != key.type) {
ret = 0;
- goto out;
+ break;
}
ret = iterate_dir_item(root, path, __process_new_xattr, sctx);
if (ret < 0)
- goto out;
-
- path->slots[0]++;
+ break;
+ }
+ /* Catch error found on iteration */
+ if (iter_ret < 0) {
+ ret = iter_ret;
}
-out:
btrfs_free_path(path);
return ret;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 11/14] btrfs: Use btrfs_for_each_slot in process_all_extents
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (9 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 10/14] btrfs: Use btrfs_for_each_slot in process_all_new_xattrs Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 12/14] btrfs: Use btrfs_for_each_slot in btrfs_unlink_all_paths Gabriel Niebler
` (3 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/send.c | 38 +++++++++-----------------------------
1 file changed, 9 insertions(+), 29 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 2057aca8c3eb..7e40c73bb912 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -5899,13 +5899,12 @@ static int process_extent(struct send_ctx *sctx,
static int process_all_extents(struct send_ctx *sctx)
{
- int ret;
+ int ret = 0;
+ int iter_ret = 0;
struct btrfs_root *root;
struct btrfs_path *path;
struct btrfs_key key;
struct btrfs_key found_key;
- struct extent_buffer *eb;
- int slot;
root = sctx->send_root;
path = alloc_path_for_send();
@@ -5915,41 +5914,22 @@ static int process_all_extents(struct send_ctx *sctx)
key.objectid = sctx->cmp_key->objectid;
key.type = BTRFS_EXTENT_DATA_KEY;
key.offset = 0;
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto out;
-
- while (1) {
- eb = path->nodes[0];
- slot = path->slots[0];
-
- if (slot >= btrfs_header_nritems(eb)) {
- ret = btrfs_next_leaf(root, path);
- if (ret < 0) {
- goto out;
- } else if (ret > 0) {
- ret = 0;
- break;
- }
- continue;
- }
-
- btrfs_item_key_to_cpu(eb, &found_key, slot);
-
+ btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
if (found_key.objectid != key.objectid ||
found_key.type != key.type) {
ret = 0;
- goto out;
+ break;
}
ret = process_extent(sctx, path, &found_key);
if (ret < 0)
- goto out;
-
- path->slots[0]++;
+ break;
+ }
+ /* Catch error found on iteration */
+ if (iter_ret < 0) {
+ ret = iter_ret;
}
-out:
btrfs_free_path(path);
return ret;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 12/14] btrfs: Use btrfs_for_each_slot in btrfs_unlink_all_paths
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (10 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 11/14] btrfs: Use btrfs_for_each_slot in process_all_extents Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-08 15:29 ` Josef Bacik
2022-03-02 16:48 ` [PATCH v3 13/14] btrfs: Use btrfs_for_each_slot in btrfs_read_chunk_tree Gabriel Niebler
` (2 subsequent siblings)
14 siblings, 1 reply; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/send.c | 36 +++++++++++++-----------------------
1 file changed, 13 insertions(+), 23 deletions(-)
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 7e40c73bb912..af3668279875 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -6119,8 +6119,11 @@ static int btrfs_unlink_all_paths(struct send_ctx *sctx)
{
LIST_HEAD(deleted_refs);
struct btrfs_path *path;
+ struct btrfs_root *root = sctx->parent_root;
struct btrfs_key key;
+ struct btrfs_key found_key;
struct parent_paths_ctx ctx;
+ int iter_ret = 0;
int ret;
path = alloc_path_for_send();
@@ -6130,39 +6133,26 @@ static int btrfs_unlink_all_paths(struct send_ctx *sctx)
key.objectid = sctx->cur_ino;
key.type = BTRFS_INODE_REF_KEY;
key.offset = 0;
- ret = btrfs_search_slot(NULL, sctx->parent_root, &key, path, 0, 0);
- if (ret < 0)
- goto out;
ctx.refs = &deleted_refs;
ctx.sctx = sctx;
- while (true) {
- struct extent_buffer *eb = path->nodes[0];
- int slot = path->slots[0];
-
- if (slot >= btrfs_header_nritems(eb)) {
- ret = btrfs_next_leaf(sctx->parent_root, path);
- if (ret < 0)
- goto out;
- else if (ret > 0)
- break;
- continue;
- }
-
- btrfs_item_key_to_cpu(eb, &key, slot);
- if (key.objectid != sctx->cur_ino)
+ btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
+ if (found_key.objectid != key.objectid)
break;
- if (key.type != BTRFS_INODE_REF_KEY &&
- key.type != BTRFS_INODE_EXTREF_KEY)
+ if (found_key.type != key.type &&
+ found_key.type != BTRFS_INODE_EXTREF_KEY)
break;
- ret = iterate_inode_ref(sctx->parent_root, path, &key, 1,
+ ret = iterate_inode_ref(root, path, &key, 1,
record_parent_ref, &ctx);
if (ret < 0)
goto out;
-
- path->slots[0]++;
+ }
+ /* Catch error found on iteration */
+ if (iter_ret < 0) {
+ ret = iter_ret;
+ goto out;
}
while (!list_empty(&deleted_refs)) {
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 13/14] btrfs: Use btrfs_for_each_slot in btrfs_read_chunk_tree
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (11 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 12/14] btrfs: Use btrfs_for_each_slot in btrfs_unlink_all_paths Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 14/14] btrfs: Use btrfs_for_each_slot in btrfs_listxattr Gabriel Niebler
2022-03-08 14:27 ` [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots David Sterba
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/volumes.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b07d382d53a8..302efb5881e9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -7575,6 +7575,7 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
struct btrfs_key found_key;
int ret;
int slot;
+ int iter_ret = 0;
u64 total_dev = 0;
u64 last_ra_node = 0;
@@ -7618,30 +7619,17 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
key.offset = 0;
key.type = 0;
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto error;
- while (1) {
- struct extent_buffer *node;
-
+ btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
+ struct extent_buffer *node = path->nodes[1];
leaf = path->nodes[0];
slot = path->slots[0];
- if (slot >= btrfs_header_nritems(leaf)) {
- ret = btrfs_next_leaf(root, path);
- if (ret == 0)
- continue;
- if (ret < 0)
- goto error;
- break;
- }
- node = path->nodes[1];
+
if (node) {
if (last_ra_node != node->start) {
readahead_tree_node_children(node);
last_ra_node = node->start;
}
}
- btrfs_item_key_to_cpu(leaf, &found_key, slot);
if (found_key.type == BTRFS_DEV_ITEM_KEY) {
struct btrfs_dev_item *dev_item;
dev_item = btrfs_item_ptr(leaf, slot,
@@ -7666,7 +7654,11 @@ int btrfs_read_chunk_tree(struct btrfs_fs_info *fs_info)
if (ret)
goto error;
}
- path->slots[0]++;
+ }
+ /* Catch error found on iteration */
+ if (iter_ret < 0) {
+ ret = iter_ret;
+ goto error;
}
/*
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v3 14/14] btrfs: Use btrfs_for_each_slot in btrfs_listxattr
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (12 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 13/14] btrfs: Use btrfs_for_each_slot in btrfs_read_chunk_tree Gabriel Niebler
@ 2022-03-02 16:48 ` Gabriel Niebler
2022-03-08 14:27 ` [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots David Sterba
14 siblings, 0 replies; 19+ messages in thread
From: Gabriel Niebler @ 2022-03-02 16:48 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, Gabriel Niebler, Marcos Paulo de Souza
This function can be simplified by refactoring to use the new iterator macro.
No functional changes.
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Signed-off-by: Gabriel Niebler <gniebler@suse.com>
---
fs/btrfs/xattr.c | 41 ++++++++++++-----------------------------
1 file changed, 12 insertions(+), 29 deletions(-)
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 99abf41b89b9..4b894625b67b 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -271,10 +271,12 @@ int btrfs_setxattr_trans(struct inode *inode, const char *name,
ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
{
+ struct btrfs_key found_key;
struct btrfs_key key;
struct inode *inode = d_inode(dentry);
struct btrfs_root *root = BTRFS_I(inode)->root;
struct btrfs_path *path;
+ int iter_ret = 0;
int ret = 0;
size_t total_size = 0, size_left = size;
@@ -293,44 +295,23 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
path->reada = READA_FORWARD;
/* search for our xattrs */
- ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
- if (ret < 0)
- goto err;
-
- while (1) {
+ btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
struct extent_buffer *leaf;
int slot;
struct btrfs_dir_item *di;
- struct btrfs_key found_key;
u32 item_size;
u32 cur;
leaf = path->nodes[0];
slot = path->slots[0];
- /* this is where we start walking through the path */
- if (slot >= btrfs_header_nritems(leaf)) {
- /*
- * if we've reached the last slot in this leaf we need
- * to go to the next leaf and reset everything
- */
- ret = btrfs_next_leaf(root, path);
- if (ret < 0)
- goto err;
- else if (ret > 0)
- break;
- continue;
- }
-
- btrfs_item_key_to_cpu(leaf, &found_key, slot);
-
/* check to make sure this item is what we want */
if (found_key.objectid != key.objectid)
break;
if (found_key.type > BTRFS_XATTR_ITEM_KEY)
break;
if (found_key.type < BTRFS_XATTR_ITEM_KEY)
- goto next_item;
+ continue;
di = btrfs_item_ptr(leaf, slot, struct btrfs_dir_item);
item_size = btrfs_item_size(leaf, slot);
@@ -350,8 +331,8 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
goto next;
if (!buffer || (name_len + 1) > size_left) {
- ret = -ERANGE;
- goto err;
+ iter_ret = -ERANGE;
+ break;
}
read_extent_buffer(leaf, buffer, name_ptr, name_len);
@@ -363,12 +344,14 @@ ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size)
cur += this_len;
di = (struct btrfs_dir_item *)((char *)di + this_len);
}
-next_item:
- path->slots[0]++;
}
- ret = total_size;
-err:
+ if (iter_ret < 0) {
+ ret = iter_ret;
+ } else {
+ ret = total_size;
+ }
+
btrfs_free_path(path);
return ret;
--
2.35.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
` (13 preceding siblings ...)
2022-03-02 16:48 ` [PATCH v3 14/14] btrfs: Use btrfs_for_each_slot in btrfs_listxattr Gabriel Niebler
@ 2022-03-08 14:27 ` David Sterba
14 siblings, 0 replies; 19+ messages in thread
From: David Sterba @ 2022-03-08 14:27 UTC (permalink / raw)
To: Gabriel Niebler; +Cc: linux-btrfs, dsterba
On Wed, Mar 02, 2022 at 05:48:15PM +0100, Gabriel Niebler wrote:
> There is a common pattern when searching for a key in btrfs:
>
> * Call btrfs_search_slot to find the slot for the key
> * Enter an endless loop:
> * If the found slot is larger than the no. of items in the current leaf,
> check the next leaf
> * If it's still not found in the next leaf, terminate the loop
> * Otherwise do something with the found key
> * Increment the current slot and continue
>
> To reduce code duplication, we can replace this code pattern with an iterator
> macro, similar to the existing for_each_X macros found elsewhere in the kernel.
> This also makes the code easier to understand for newcomers by putting a name
> to the encapsulated functionality.
>
> This patchset survived a complete fstest run.
>
> Changes from v2:
> * Rename btrfs_valid_slot to btrfs_get_next_valid_item (Nikolay)
> * Fix comment formatting (David)
> * Remove redundant parentheses and indentation in loop condition (David)
> * Remove redundant iter_ret var and reuse ret instead (Nikolay)
> * Make termination condition more consistent in btrfs_unlink_all_paths
> (Nikolay)
> * Improved patch organisation by splitting into one patch per function (David)
> * Improve doc comment for btrfs_get_next_valid_item (Gabriel)
> * Remove `out` label and assoc. gotos from id_create_dir (Gabriel)
> * Initialise `ret` in process_all_refs and process_all_new_xattrs (Gabriel)
> * Remove unneeded btrfs_item_key_to_cpu call from loop body in
> btrfs_read_chunk_tree (Gabriel)
I still see some of the style issues not addressed in your version, I'll
point them out in the first example I find but it needs to be fixed in
all of them. Otherwise it looks good. As we're near the code freeze for
5.17 I'm not sure I could put it there so it's 5.18 at the latest.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 01/14] btrfs: Introduce btrfs_for_each_slot iterator macro
2022-03-02 16:48 ` [PATCH v3 01/14] btrfs: Introduce btrfs_for_each_slot iterator macro Gabriel Niebler
@ 2022-03-08 14:31 ` David Sterba
0 siblings, 0 replies; 19+ messages in thread
From: David Sterba @ 2022-03-08 14:31 UTC (permalink / raw)
To: Gabriel Niebler; +Cc: linux-btrfs, dsterba, Marcos Paulo de Souza
On Wed, Mar 02, 2022 at 05:48:16PM +0100, Gabriel Niebler wrote:
> There is a common pattern when searching for a key in btrfs:
>
> * Call btrfs_search_slot to find the slot for the key
> * Enter an endless loop:
> * If the found slot is larger than the no. of items in the current leaf,
> check the next leaf
> * If it's still not found in the next leaf, terminate the loop
> * Otherwise do something with the found key
> * Increment the current slot and continue
>
> To reduce code duplication, we can replace this code pattern with an iterator
> macro, similar to the existing for_each_X macros found elsewhere in the kernel.
> This also makes the code easier to understand for newcomers by putting a name
> to the encapsulated functionality.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> Signed-off-by: Gabriel Niebler <gniebler@suse.com>
> ---
> fs/btrfs/ctree.c | 33 +++++++++++++++++++++++++++++++++
> fs/btrfs/ctree.h | 24 ++++++++++++++++++++++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index a7db3f6f1b7b..d735bd472616 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2277,6 +2277,39 @@ int btrfs_search_backwards(struct btrfs_root *root, struct btrfs_key *key,
> return ret;
> }
>
> +/* Search for a valid slot for the given path.
Multi-line comments should start with
/*
* Text ...
* ...
*/
> + *
> + * @root: The root node of the tree.
> + * @key: Will contain a valid item if found.
> + * @path: The starting point to validate the slot.
Please align the descriptions, an example is here
https://btrfs.wiki.kernel.org/index.php/Development_notes#Comments
> + *
> + * Return 0 if the item is valid, 1 if not found and < 0 if error.
> + */
> +int btrfs_get_next_valid_item(struct btrfs_root *root, struct btrfs_key *key,
> + struct btrfs_path *path)
> +{
> + while (1) {
> + int ret;
> + const int slot = path->slots[0];
> + const struct extent_buffer *leaf = path->nodes[0];
Newline between declarations and statements.
> + /* this is where we start walking through the path */
The comments should read like a sentence, so the first letter should be
uppercase unless it's an identifier.
> + if (slot >= btrfs_header_nritems(leaf)) {
> + /*
> + * if we've reached the last slot in this leaf we need
> + * to go to the next leaf and reset the path
> + */
> + ret = btrfs_next_leaf(root, path);
> + if (ret)
> + return ret;
> + continue;
> + }
> + /* store the found, valid item in key */
> + btrfs_item_key_to_cpu(leaf, key, slot);
> + break;
> + }
> + return 0;
> +}
> +
> /*
> * adjust the pointers going up the tree, starting at level
> * making sure the right key of each node is points to 'key'.
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 947f04789389..98091334b749 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2976,6 +2976,30 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
> int btrfs_search_backwards(struct btrfs_root *root, struct btrfs_key *key,
> struct btrfs_path *path);
>
> +int btrfs_get_next_valid_item(struct btrfs_root *root, struct btrfs_key *key,
> + struct btrfs_path *path);
> +
> +/* Search in @root for a given @key, and store the slot found in @found_key.
> + *
> + * @root: The root node of the tree.
> + * @key: The key we are looking for.
> + * @found_key: Will hold the found item.
> + * @path: Holds the current slot/leaf.
> + * @iter_ret: Contains the value returned from btrfs_search_slot or
> + * btrfs_get_next_valid_item, whichever was executed last.
> + *
> + * The iter_ret is an output variable that will contain the return value of
> + * btrfs_search_slot, if it encountered an error, or the value returned from
> + * btrfs_get_next_valid_item, otherwise. That return value can be 0, if a valid
> + * slot was found, 1 if there were no more leaves, and <0 if there was an error.
> + */
> +#define btrfs_for_each_slot(root, key, found_key, path, iter_ret) \
> + for (iter_ret = btrfs_search_slot(NULL, root, key, path, 0, 0); \
> + iter_ret >= 0 && \
> + (iter_ret = btrfs_get_next_valid_item(root, found_key, path)) == 0; \
> + path->slots[0]++ \
As the arguments can be more than just an identifier it's safer to put
them in ( ) in the definition, ie root/key/found_key/path/iter_ret
shouls be all (root) etc
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 03/14] btrfs: Use btrfs_for_each_slot in mark_block_group_to_copy
2022-03-02 16:48 ` [PATCH v3 03/14] btrfs: Use btrfs_for_each_slot in mark_block_group_to_copy Gabriel Niebler
@ 2022-03-08 14:33 ` David Sterba
0 siblings, 0 replies; 19+ messages in thread
From: David Sterba @ 2022-03-08 14:33 UTC (permalink / raw)
To: Gabriel Niebler; +Cc: linux-btrfs, dsterba, Marcos Paulo de Souza
On Wed, Mar 02, 2022 at 05:48:18PM +0100, Gabriel Niebler wrote:
> This function can be simplified by refactoring to use the new iterator macro.
>
> No functional changes.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> Signed-off-by: Gabriel Niebler <gniebler@suse.com>
> ---
> fs/btrfs/dev-replace.c | 41 ++++++++---------------------------------
> 1 file changed, 8 insertions(+), 33 deletions(-)
>
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 62b9651ea662..3357739f427f 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -470,6 +470,7 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
> struct btrfs_dev_extent *dev_extent = NULL;
> struct btrfs_block_group *cache;
> struct btrfs_trans_handle *trans;
> + int iter_ret = 0;
> int ret = 0;
> u64 chunk_offset;
>
> @@ -520,29 +521,8 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
> key.type = BTRFS_DEV_EXTENT_KEY;
> key.offset = 0;
>
> - ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> - if (ret < 0)
> - goto free_path;
> - if (ret > 0) {
> - if (path->slots[0] >=
> - btrfs_header_nritems(path->nodes[0])) {
> - ret = btrfs_next_leaf(root, path);
> - if (ret < 0)
> - goto free_path;
> - if (ret > 0) {
> - ret = 0;
> - goto free_path;
> - }
> - } else {
> - ret = 0;
> - }
> - }
> -
> - while (1) {
> + btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
> struct extent_buffer *leaf = path->nodes[0];
> - int slot = path->slots[0];
> -
> - btrfs_item_key_to_cpu(leaf, &found_key, slot);
>
> if (found_key.objectid != src_dev->devid)
> break;
> @@ -553,30 +533,25 @@ static int mark_block_group_to_copy(struct btrfs_fs_info *fs_info,
> if (found_key.offset < key.offset)
> break;
>
> - dev_extent = btrfs_item_ptr(leaf, slot, struct btrfs_dev_extent);
> + dev_extent = btrfs_item_ptr(leaf, path->slots[0],
> + struct btrfs_dev_extent);
>
> chunk_offset = btrfs_dev_extent_chunk_offset(leaf, dev_extent);
>
> cache = btrfs_lookup_block_group(fs_info, chunk_offset);
> if (!cache)
> - goto skip;
> + continue;
>
> spin_lock(&cache->lock);
> cache->to_copy = 1;
> spin_unlock(&cache->lock);
>
> btrfs_put_block_group(cache);
> -
> -skip:
> - ret = btrfs_next_item(root, path);
> - if (ret != 0) {
> - if (ret > 0)
> - ret = 0;
> - break;
> - }
> + }
> + if (iter_ret < 0) {
> + ret = iter_ret;
> }
No { } around single statement conditions 'if'
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v3 12/14] btrfs: Use btrfs_for_each_slot in btrfs_unlink_all_paths
2022-03-02 16:48 ` [PATCH v3 12/14] btrfs: Use btrfs_for_each_slot in btrfs_unlink_all_paths Gabriel Niebler
@ 2022-03-08 15:29 ` Josef Bacik
0 siblings, 0 replies; 19+ messages in thread
From: Josef Bacik @ 2022-03-08 15:29 UTC (permalink / raw)
To: Gabriel Niebler; +Cc: linux-btrfs, dsterba, Marcos Paulo de Souza
On Wed, Mar 02, 2022 at 05:48:27PM +0100, Gabriel Niebler wrote:
> This function can be simplified by refactoring to use the new iterator macro.
>
> No functional changes.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> Signed-off-by: Gabriel Niebler <gniebler@suse.com>
> ---
> fs/btrfs/send.c | 36 +++++++++++++-----------------------
> 1 file changed, 13 insertions(+), 23 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 7e40c73bb912..af3668279875 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -6119,8 +6119,11 @@ static int btrfs_unlink_all_paths(struct send_ctx *sctx)
> {
> LIST_HEAD(deleted_refs);
> struct btrfs_path *path;
> + struct btrfs_root *root = sctx->parent_root;
> struct btrfs_key key;
> + struct btrfs_key found_key;
> struct parent_paths_ctx ctx;
> + int iter_ret = 0;
> int ret;
>
> path = alloc_path_for_send();
> @@ -6130,39 +6133,26 @@ static int btrfs_unlink_all_paths(struct send_ctx *sctx)
> key.objectid = sctx->cur_ino;
> key.type = BTRFS_INODE_REF_KEY;
> key.offset = 0;
> - ret = btrfs_search_slot(NULL, sctx->parent_root, &key, path, 0, 0);
> - if (ret < 0)
> - goto out;
>
> ctx.refs = &deleted_refs;
> ctx.sctx = sctx;
>
> - while (true) {
> - struct extent_buffer *eb = path->nodes[0];
> - int slot = path->slots[0];
> -
> - if (slot >= btrfs_header_nritems(eb)) {
> - ret = btrfs_next_leaf(sctx->parent_root, path);
> - if (ret < 0)
> - goto out;
> - else if (ret > 0)
> - break;
> - continue;
> - }
> -
> - btrfs_item_key_to_cpu(eb, &key, slot);
> - if (key.objectid != sctx->cur_ino)
> + btrfs_for_each_slot(root, &key, &found_key, path, iter_ret) {
> + if (found_key.objectid != key.objectid)
> break;
> - if (key.type != BTRFS_INODE_REF_KEY &&
> - key.type != BTRFS_INODE_EXTREF_KEY)
> + if (found_key.type != key.type &&
> + found_key.type != BTRFS_INODE_EXTREF_KEY)
> break;
>
> - ret = iterate_inode_ref(sctx->parent_root, path, &key, 1,
> + ret = iterate_inode_ref(root, path, &key, 1,
> record_parent_ref, &ctx);
This patch is causing btrfs/168 to fail, this should be &found_key, not &key.
Thanks,
Josef
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-03-08 15:30 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-02 16:48 [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 01/14] btrfs: Introduce btrfs_for_each_slot iterator macro Gabriel Niebler
2022-03-08 14:31 ` David Sterba
2022-03-02 16:48 ` [PATCH v3 02/14] btrfs: Use btrfs_for_each_slot in find_first_block_group Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 03/14] btrfs: Use btrfs_for_each_slot in mark_block_group_to_copy Gabriel Niebler
2022-03-08 14:33 ` David Sterba
2022-03-02 16:48 ` [PATCH v3 04/14] btrfs: Use btrfs_for_each_slot in btrfs_search_dir_index_item Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 05/14] btrfs: Use btrfs_for_each_slot in btrfs_real_readdir Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 06/14] btrfs: Use btrfs_for_each_slot in did_create_dir Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 07/14] btrfs: Use btrfs_for_each_slot in can_rmdir Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 08/14] btrfs: Use btrfs_for_each_slot in is_ancestor Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 09/14] btrfs: Use btrfs_for_each_slot in process_all_refs Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 10/14] btrfs: Use btrfs_for_each_slot in process_all_new_xattrs Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 11/14] btrfs: Use btrfs_for_each_slot in process_all_extents Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 12/14] btrfs: Use btrfs_for_each_slot in btrfs_unlink_all_paths Gabriel Niebler
2022-03-08 15:29 ` Josef Bacik
2022-03-02 16:48 ` [PATCH v3 13/14] btrfs: Use btrfs_for_each_slot in btrfs_read_chunk_tree Gabriel Niebler
2022-03-02 16:48 ` [PATCH v3 14/14] btrfs: Use btrfs_for_each_slot in btrfs_listxattr Gabriel Niebler
2022-03-08 14:27 ` [PATCH v3 0/14] btrfs: Introduce macro to iterate over slots 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).