| Message ID | 20180220093738.1461-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > } > } >
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
Quoting Patchwork (2018-02-20 14:03:13) > == Series Details == > > Series: drm/mm: Fix caching of leftmost node in the interval tree (rev2) > URL : https://patchwork.freedesktop.org/series/38351/ > State : success > > == Summary == > > Test kms_cursor_crc: > Subgroup cursor-256x256-suspend: > skip -> PASS (shard-snb) fdo#103375 > Test perf: > Subgroup buffer-fill: > pass -> FAIL (shard-apl) fdo#103755 Pushed to drm-misc-next, while the code is definitely bogus it doesn't manifest in a bug because fortuitously the leftmost cache is unused. Thanks for the review, -Chris
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; } }
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(-)