| Message ID | 1378031968-22062-1-git-send-email-fdmanana@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Sun, Sep 01, 2013 at 11:39:28AM +0100, Filipe David Borba Manana wrote: > +#ifdef CONFIG_BTRFS_ASSERT > +static int key_search_validate(struct extent_buffer *b, > + struct btrfs_key *key, > + int level) > +{ ... > +} > +#endif > + > +static int key_search(struct extent_buffer *b, struct btrfs_key *key, > + int level, int *prev_cmp, int *slot) > +{ > + if (*prev_cmp != 0) { > + *prev_cmp = bin_search(b, key, level, slot); > + return *prev_cmp; > + } > + > + ASSERT(key_search_validate(b, key, level)); But what if I want to use key_search_validate out of the context of an ASSERT ? I don't see a reason why the function needs to be under #ifdef BTRFS_ASSERT / #endif at all. > + *slot = 0; > + > + return 0; > +} -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 2, 2013 at 2:39 PM, David Sterba <dsterba@suse.cz> wrote: > On Sun, Sep 01, 2013 at 11:39:28AM +0100, Filipe David Borba Manana wrote: >> +#ifdef CONFIG_BTRFS_ASSERT >> +static int key_search_validate(struct extent_buffer *b, >> + struct btrfs_key *key, >> + int level) >> +{ > ... >> +} >> +#endif >> + >> +static int key_search(struct extent_buffer *b, struct btrfs_key *key, >> + int level, int *prev_cmp, int *slot) >> +{ >> + if (*prev_cmp != 0) { >> + *prev_cmp = bin_search(b, key, level, slot); >> + return *prev_cmp; >> + } >> + >> + ASSERT(key_search_validate(b, key, level)); > > But what if I want to use key_search_validate out of the context of an > ASSERT ? Right. But right now nothing else uses it. Shall the need for it come, it's trivial to address. > I don't see a reason why the function needs to be under #ifdef > BTRFS_ASSERT / #endif at all. To avoid the compiler warning, as mentioned before. Between patch versions v5 to v7, I don't have any strong preference. All have correct, small and simple code. > >> + *slot = 0; >> + >> + return 0; >> +}
On Mon, Sep 02, 2013 at 03:40:39PM +0100, Filipe David Manana wrote: > Between patch versions v5 to v7, I don't have any strong preference. > All have correct, small and simple code. I'm ok with v7. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 5fa521b..4d602f7 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -2426,6 +2426,39 @@ done: return ret; } +#ifdef CONFIG_BTRFS_ASSERT +static int key_search_validate(struct extent_buffer *b, + struct btrfs_key *key, + int level) +{ + struct btrfs_disk_key disk_key; + unsigned long offset; + + btrfs_cpu_key_to_disk(&disk_key, key); + + if (level == 0) + offset = offsetof(struct btrfs_leaf, items[0].key); + else + offset = offsetof(struct btrfs_node, ptrs[0].key); + + return !memcmp_extent_buffer(b, &disk_key, offset, sizeof(disk_key)); +} +#endif + +static int key_search(struct extent_buffer *b, struct btrfs_key *key, + int level, int *prev_cmp, int *slot) +{ + if (*prev_cmp != 0) { + *prev_cmp = bin_search(b, key, level, slot); + return *prev_cmp; + } + + ASSERT(key_search_validate(b, key, level)); + *slot = 0; + + return 0; +} + /* * look for key in the tree. path is filled in with nodes along the way * if key is found, we return zero and you can find the item in the leaf @@ -2454,6 +2487,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root int write_lock_level = 0; u8 lowest_level = 0; int min_write_lock_level; + int prev_cmp; lowest_level = p->lowest_level; WARN_ON(lowest_level && ins_len > 0); @@ -2484,6 +2518,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root min_write_lock_level = write_lock_level; again: + prev_cmp = -1; /* * we try very hard to do read locks on the root */ @@ -2584,7 +2619,7 @@ cow_done: if (!cow) btrfs_unlock_up_safe(p, level + 1); - ret = bin_search(b, key, level, &slot); + ret = key_search(b, key, level, &prev_cmp, &slot); if (level != 0) { int dec = 0; @@ -2719,6 +2754,7 @@ int btrfs_search_old_slot(struct btrfs_root *root, struct btrfs_key *key, int level; int lowest_unlock = 1; u8 lowest_level = 0; + int prev_cmp; lowest_level = p->lowest_level; WARN_ON(p->nodes[0] != NULL); @@ -2729,6 +2765,7 @@ int btrfs_search_old_slot(struct btrfs_root *root, struct btrfs_key *key, } again: + prev_cmp = -1; b = get_old_root(root, time_seq); level = btrfs_header_level(b); p->locks[level] = BTRFS_READ_LOCK; @@ -2746,7 +2783,7 @@ again: */ btrfs_unlock_up_safe(p, level + 1); - ret = bin_search(b, key, level, &slot); + ret = key_search(b, key, level, &prev_cmp, &slot); if (level != 0) { int dec = 0;