diff mbox

[CI] drm/mm: Fix caching of leftmost node in the interval tree

Message ID 20180220093738.1461-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Feb. 20, 2018, 9:37 a.m. UTC
When we descend the tree to find our slot, if we step to the right, we
are no longer the leftmost node.

Fixes: f808c13fd373 ("lib/interval_tree: fast overlap detection")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Jérôme Glisse <jglisse@redhat.com>
Cc: Christian König <christian.koenig@amd.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/drm_mm.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Christian König Feb. 20, 2018, 9:45 a.m. UTC | #1
Am 20.02.2018 um 10:37 schrieb Chris Wilson:
> When we descend the tree to find our slot, if we step to the right, we
> are no longer the leftmost node.
>
> Fixes: f808c13fd373 ("lib/interval_tree: fast overlap detection")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: Jérôme Glisse <jglisse@redhat.com>
> Cc: Christian König <christian.koenig@amd.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Acked-by: Christian König <christian.koenig@amd.com> for now.

But I agree with Joonas that we should think about using 
rb_insert_augmented instead when we actually don't use the extra 
functionality.

Christian.

> ---
>   drivers/gpu/drm/drm_mm.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 89eef1bb4ddc..3166026a1874 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -180,7 +180,7 @@ static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
>   	struct drm_mm *mm = hole_node->mm;
>   	struct rb_node **link, *rb;
>   	struct drm_mm_node *parent;
> -	bool leftmost = true;
> +	bool leftmost;
>   
>   	node->__subtree_last = LAST(node);
>   
> @@ -201,6 +201,7 @@ static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
>   	} else {
>   		rb = NULL;
>   		link = &mm->interval_tree.rb_root.rb_node;
> +		leftmost = true;
>   	}
>   
>   	while (*link) {
> @@ -208,11 +209,11 @@ static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
>   		parent = rb_entry(rb, struct drm_mm_node, rb);
>   		if (parent->__subtree_last < node->__subtree_last)
>   			parent->__subtree_last = node->__subtree_last;
> -		if (node->start < parent->start)
> +		if (node->start < parent->start) {
>   			link = &parent->rb.rb_left;
> -		else {
> +		} else {
>   			link = &parent->rb.rb_right;
> -			leftmost = true;
> +			leftmost = false;
>   		}
>   	}
>
Chris Wilson Feb. 20, 2018, 2:21 p.m. UTC | #2
Quoting Christian König (2018-02-20 09:45:54)
> Am 20.02.2018 um 10:37 schrieb Chris Wilson:
> > When we descend the tree to find our slot, if we step to the right, we
> > are no longer the leftmost node.
> >
> > Fixes: f808c13fd373 ("lib/interval_tree: fast overlap detection")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Davidlohr Bueso <dbueso@suse.de>
> > Cc: Jérôme Glisse <jglisse@redhat.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Acked-by: Christian König <christian.koenig@amd.com> for now.
> 
> But I agree with Joonas that we should think about using 
> rb_insert_augmented instead when we actually don't use the extra 
> functionality.

The cost is having to reimplement interval_tree_generic.h. Not too
onerous, but trickier than fixing leftmost here. :)
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 89eef1bb4ddc..3166026a1874 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -180,7 +180,7 @@  static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
 	struct drm_mm *mm = hole_node->mm;
 	struct rb_node **link, *rb;
 	struct drm_mm_node *parent;
-	bool leftmost = true;
+	bool leftmost;
 
 	node->__subtree_last = LAST(node);
 
@@ -201,6 +201,7 @@  static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
 	} else {
 		rb = NULL;
 		link = &mm->interval_tree.rb_root.rb_node;
+		leftmost = true;
 	}
 
 	while (*link) {
@@ -208,11 +209,11 @@  static void drm_mm_interval_tree_add_node(struct drm_mm_node *hole_node,
 		parent = rb_entry(rb, struct drm_mm_node, rb);
 		if (parent->__subtree_last < node->__subtree_last)
 			parent->__subtree_last = node->__subtree_last;
-		if (node->start < parent->start)
+		if (node->start < parent->start) {
 			link = &parent->rb.rb_left;
-		else {
+		} else {
 			link = &parent->rb.rb_right;
-			leftmost = true;
+			leftmost = false;
 		}
 	}