Thread (18 messages) 18 messages, 4 authors, 2021-08-31

Re: [PATCH 1/8] fs: btrfs: Introduce btrfs_for_each_slot

From: Nikolay Borisov <hidden>
Date: 2021-08-30 12:37:07


On 26.08.21 г. 19:40, Marcos Paulo de Souza wrote:
quoted hunk ↗ jump to hunk
There is a common pattern when search for a key in btrfs:

 * Call btrfs_search_slot
 * Endless loop
         * If the found slot is bigger than the current items in the leaf, check the
           next one
         * If still not found in the next leaf, return 1
         * Do something with the code
         * Increment current slot, and continue

This pattern can be improved by creating an iterator macro, similar to
those for_each_X already existing in the linux kernel. Using this
approach means to reduce significantly boilerplate code, along making it
easier to newcomers to understand how to code works.

Signed-off-by: Marcos Paulo de Souza <redacted>
---
 fs/btrfs/ctree.c | 28 ++++++++++++++++++++++++++++
 fs/btrfs/ctree.h | 25 +++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 84627cbd5b5b..b1aa6e3987d0 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2122,6 +2122,34 @@ 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 start point to validate the slot.
+ *
+ * Return 0 if the item is valid, 1 if not found and < 0 if error.
+ */
+int btrfs_valid_slot(struct btrfs_root *root, struct btrfs_key *key,
nit: The name of this function is a bit misleading since it's not really
a predicate, more like a function that returns the value and if it can't
return the current value pointed to by path it gets the next leaf. I
guess a more apt name would be "btrfs_get_next_valid_item" or some such.
quoted hunk ↗ jump to hunk
+		     struct btrfs_path *path)
+{
+	while (1) {
+		int ret;
+		const int slot = path->slots[0];
+		const struct extent_buffer *leaf = path->nodes[0];
+
+		if (slot >= btrfs_header_nritems(leaf)) {
+			ret = btrfs_next_leaf(root, path);
+			if (ret)
+				return ret;
+			continue;
+		}
+		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 f07c82fafa04..1e3c4a7741ca 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2912,6 +2912,31 @@ 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_valid_slot(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 returned value from btrfs_search_slot and
+ *            btrfs_valid_slot, whatever is executed later.
+ *
+ * The iter_ret is an output variable that will contain the result of the
+ * btrfs_search_slot if it returns an error, or the value returned from
+ * btrfs_valid_slot otherwise. The return value can be 0 if the something was
+ * found, 1 if there weren't bigger leaves, and <0 if 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_valid_slot(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)
 {
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help