* Re: [PATCH] Btrfs-progs: fix optimization in btrfs_lookup_extent_info
2013-07-04 15:48 ` [PATCH] Btrfs-progs: fix optimization in btrfs_lookup_extent_info Filipe David Borba Manana
@ 2013-07-04 15:55 ` Filipe David Manana
2013-07-04 16:24 ` [PATCH v2] " Filipe David Borba Manana
2013-07-05 20:36 ` [PATCH v3] " Filipe David Borba Manana
2 siblings, 0 replies; 6+ messages in thread
From: Filipe David Manana @ 2013-07-04 15:55 UTC (permalink / raw)
To: linux-btrfs@vger.kernel.org; +Cc: Filipe David Borba Manana, jbacik
On Thu, Jul 4, 2013 at 4:48 PM, Filipe David Borba Manana
<fdmanana@gmail.com> wrote:
> If we did a tree search with the goal to find a metadata item
> but the search failed with return value 1, we attempt to see
> if in the same leaf there's a corresponding extent item, and if
> there's one, just use it instead of doing another tree search
> for this extent item. The check in the leaf was wrong because
> it was seeking for a metadata item instead of an extent item.
>
> This optimization was also being triggered incorrectly, as it
> was evaluating path->slots which always evaluates to true. The
> goal was to see if the leaf level slot was greater than zero
> (i.e. not the first item in the leaf).
>
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
> extent-tree.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/extent-tree.c b/extent-tree.c
> index b0cfe0a..22e6247 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -1515,12 +1515,13 @@ again:
> * to make sure.
> */
> if (ret > 0 && metadata) {
> - if (path->slots) {
> + if (path->slots[0]) {
> path->slots[0]--;
> btrfs_item_key_to_cpu(path->nodes[0], &key,
> path->slots[0]);
> if (key.objectid == bytenr &&
> - key.type == BTRFS_METADATA_ITEM_KEY)
> + key.type == BTRFS_EXTENT_ITEM_KEY &&
> + key.offset == root->leafsize)
> ret = 0;
> }
>
> --
> 1.7.9.5
>
Josef, since git suggests you are the author of this code piece, can
you please review this and comment?
Thanks.
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] Btrfs-progs: fix optimization in btrfs_lookup_extent_info
2013-07-04 15:48 ` [PATCH] Btrfs-progs: fix optimization in btrfs_lookup_extent_info Filipe David Borba Manana
2013-07-04 15:55 ` Filipe David Manana
@ 2013-07-04 16:24 ` Filipe David Borba Manana
2013-07-05 20:29 ` Josef Bacik
2013-07-05 20:36 ` [PATCH v3] " Filipe David Borba Manana
2 siblings, 1 reply; 6+ messages in thread
From: Filipe David Borba Manana @ 2013-07-04 16:24 UTC (permalink / raw)
To: linux-btrfs; +Cc: Filipe David Borba Manana
If we did a tree search with the goal to find a metadata item
but the search failed with return value 1, we attempt to see
if in the same leaf there's a corresponding extent item, and if
there's one, just use it instead of doing another tree search
for this extent item. The check in the leaf was wrong because
it was seeking for a metadata item instead of an extent item.
This optimization was also being triggered incorrectly, as it
was evaluating path->slots which always evaluates to true. The
goal was to see if the leaf level slot was greater than zero
(i.e. not the first item in the leaf).
V2: If previous leaf item is for a different object, ensure the
search key has the target object id.
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
extent-tree.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/extent-tree.c b/extent-tree.c
index b0cfe0a..875dea9 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1515,17 +1515,19 @@ again:
* to make sure.
*/
if (ret > 0 && metadata) {
- if (path->slots) {
+ if (path->slots[0]) {
path->slots[0]--;
btrfs_item_key_to_cpu(path->nodes[0], &key,
path->slots[0]);
if (key.objectid == bytenr &&
- key.type == BTRFS_METADATA_ITEM_KEY)
+ key.type == BTRFS_EXTENT_ITEM_KEY &&
+ key.offset == root->leafsize)
ret = 0;
}
if (ret) {
btrfs_release_path(root, path);
+ key.objectid = bytenr;
key.type = BTRFS_EXTENT_ITEM_KEY;
key.offset = root->leafsize;
metadata = 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] Btrfs-progs: fix optimization in btrfs_lookup_extent_info
2013-07-04 16:24 ` [PATCH v2] " Filipe David Borba Manana
@ 2013-07-05 20:29 ` Josef Bacik
0 siblings, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2013-07-05 20:29 UTC (permalink / raw)
To: Filipe David Borba Manana; +Cc: linux-btrfs
On Thu, Jul 04, 2013 at 05:24:09PM +0100, Filipe David Borba Manana wrote:
> If we did a tree search with the goal to find a metadata item
> but the search failed with return value 1, we attempt to see
> if in the same leaf there's a corresponding extent item, and if
> there's one, just use it instead of doing another tree search
> for this extent item. The check in the leaf was wrong because
> it was seeking for a metadata item instead of an extent item.
>
> This optimization was also being triggered incorrectly, as it
> was evaluating path->slots which always evaluates to true. The
> goal was to see if the leaf level slot was greater than zero
> (i.e. not the first item in the leaf).
>
> V2: If previous leaf item is for a different object, ensure the
> search key has the target object id.
>
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
> extent-tree.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/extent-tree.c b/extent-tree.c
> index b0cfe0a..875dea9 100644
> --- a/extent-tree.c
> +++ b/extent-tree.c
> @@ -1515,17 +1515,19 @@ again:
> * to make sure.
> */
> if (ret > 0 && metadata) {
> - if (path->slots) {
> + if (path->slots[0]) {
> path->slots[0]--;
> btrfs_item_key_to_cpu(path->nodes[0], &key,
> path->slots[0]);
> if (key.objectid == bytenr &&
> - key.type == BTRFS_METADATA_ITEM_KEY)
> + key.type == BTRFS_EXTENT_ITEM_KEY &&
> + key.offset == root->leafsize)
> ret = 0;
> }
>
> if (ret) {
> btrfs_release_path(root, path);
> + key.objectid = bytenr;
> key.type = BTRFS_EXTENT_ITEM_KEY;
> key.offset = root->leafsize;
> metadata = 0;
Reviewed-by: Josef Bacik <jbacik@fusionio.com>
Thanks,
Josef
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3] Btrfs-progs: fix optimization in btrfs_lookup_extent_info
2013-07-04 15:48 ` [PATCH] Btrfs-progs: fix optimization in btrfs_lookup_extent_info Filipe David Borba Manana
2013-07-04 15:55 ` Filipe David Manana
2013-07-04 16:24 ` [PATCH v2] " Filipe David Borba Manana
@ 2013-07-05 20:36 ` Filipe David Borba Manana
2 siblings, 0 replies; 6+ messages in thread
From: Filipe David Borba Manana @ 2013-07-05 20:36 UTC (permalink / raw)
To: linux-btrfs; +Cc: Filipe David Borba Manana
If we did a tree search with the goal to find a metadata item
but the search failed with return value 1, we attempt to see
if in the same leaf there's a corresponding extent item, and if
there's one, just use it instead of doing another tree search
for this extent item. The check in the leaf was wrong because
it was seeking for a metadata item instead of an extent item.
This optimization was also being triggered incorrectly, as it
was evaluating path->slots which always evaluates to true. The
goal was to see if the leaf level slot was greater than zero
(i.e. not the first item in the leaf).
V2: If previous leaf item is for a different object, ensure the
search key has the target object id.
V3: Added Josef Bacik's review mention.
Reviewed-by: Josef Bacik <jbacik@fusionio.com>
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
extent-tree.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/extent-tree.c b/extent-tree.c
index b0cfe0a..875dea9 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -1515,17 +1515,19 @@ again:
* to make sure.
*/
if (ret > 0 && metadata) {
- if (path->slots) {
+ if (path->slots[0]) {
path->slots[0]--;
btrfs_item_key_to_cpu(path->nodes[0], &key,
path->slots[0]);
if (key.objectid == bytenr &&
- key.type == BTRFS_METADATA_ITEM_KEY)
+ key.type == BTRFS_EXTENT_ITEM_KEY &&
+ key.offset == root->leafsize)
ret = 0;
}
if (ret) {
btrfs_release_path(root, path);
+ key.objectid = bytenr;
key.type = BTRFS_EXTENT_ITEM_KEY;
key.offset = root->leafsize;
metadata = 0;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread