* [PATCH 1/2] lib/rbtree,drm/mm: Add rbtree_replace_node_cached()
@ 2017-11-09 21:24 Chris Wilson
2017-11-09 21:24 ` [PATCH 2/2] drm/selftests/mm: Add callsite indicator to common asserts Chris Wilson
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Chris Wilson @ 2017-11-09 21:24 UTC (permalink / raw)
To: dri-devel
Cc: Davidlohr Bueso, Daniel Vetter, intel-gfx, Joonas Lahtinen,
Jérôme Glisse, Andrew Morton
Add a variant of rbtree_replace_node() that maintains the leftmost
cached of struct rbtree_root_cached when replacing nodes within the
rbtree.
As drm_mm is the only rb_replace_node() being used on an interval tree,
the mistake looks fairly self-contained. Furthermore the only user of
drm_mm_replace_node() is its testsuite...
Fixes: f808c13fd373 ("lib/interval_tree: fast overlap detection")
Testcase: igt/drm_mm/replace
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: Andrew Morton <akpm@linux-foundation.org>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_mm.c | 8 +++++---
include/linux/rbtree.h | 2 ++
lib/rbtree.c | 10 ++++++++++
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index eb86bc3f753b..186c4e90cc1c 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -575,21 +575,23 @@ EXPORT_SYMBOL(drm_mm_remove_node);
*/
void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
{
+ struct drm_mm *mm = old->mm;
+
DRM_MM_BUG_ON(!old->allocated);
*new = *old;
list_replace(&old->node_list, &new->node_list);
- rb_replace_node(&old->rb, &new->rb, &old->mm->interval_tree.rb_root);
+ rb_replace_node_cached(&old->rb, &new->rb, &mm->interval_tree);
if (drm_mm_hole_follows(old)) {
list_replace(&old->hole_stack, &new->hole_stack);
rb_replace_node(&old->rb_hole_size,
&new->rb_hole_size,
- &old->mm->holes_size);
+ &mm->holes_size);
rb_replace_node(&old->rb_hole_addr,
&new->rb_hole_addr,
- &old->mm->holes_addr);
+ &mm->holes_addr);
}
old->allocated = false;
diff --git a/include/linux/rbtree.h b/include/linux/rbtree.h
index d574361943ea..fcbeed4053ef 100644
--- a/include/linux/rbtree.h
+++ b/include/linux/rbtree.h
@@ -99,6 +99,8 @@ extern void rb_replace_node(struct rb_node *victim, struct rb_node *new,
struct rb_root *root);
extern void rb_replace_node_rcu(struct rb_node *victim, struct rb_node *new,
struct rb_root *root);
+extern void rb_replace_node_cached(struct rb_node *victim, struct rb_node *new,
+ struct rb_root_cached *root);
static inline void rb_link_node(struct rb_node *node, struct rb_node *parent,
struct rb_node **rb_link)
diff --git a/lib/rbtree.c b/lib/rbtree.c
index ba4a9d165f1b..d3ff682fd4b8 100644
--- a/lib/rbtree.c
+++ b/lib/rbtree.c
@@ -603,6 +603,16 @@ void rb_replace_node(struct rb_node *victim, struct rb_node *new,
}
EXPORT_SYMBOL(rb_replace_node);
+void rb_replace_node_cached(struct rb_node *victim, struct rb_node *new,
+ struct rb_root_cached *root)
+{
+ rb_replace_node(victim, new, &root->rb_root);
+
+ if (root->rb_leftmost == victim)
+ root->rb_leftmost = new;
+}
+EXPORT_SYMBOL(rb_replace_node_cached);
+
void rb_replace_node_rcu(struct rb_node *victim, struct rb_node *new,
struct rb_root *root)
{
--
2.15.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/selftests/mm: Add callsite indicator to common asserts
2017-11-09 21:24 [PATCH 1/2] lib/rbtree,drm/mm: Add rbtree_replace_node_cached() Chris Wilson
@ 2017-11-09 21:24 ` Chris Wilson
2017-11-21 8:07 ` Joonas Lahtinen
2017-11-10 14:26 ` [PATCH 1/2] lib/rbtree, drm/mm: Add rbtree_replace_node_cached() Davidlohr Bueso
2017-11-22 10:07 ` [PATCH] " Chris Wilson
2 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-11-09 21:24 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, intel-gfx
The majority of the asserts (validating nodes and ranges) are shared
amongst several subtests. This makes identification of which caller
failed hard; but we uniquely identify them if we include the callsite
into the assertion error message (a single frame stacktrace).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/selftests/test-drm_mm.c | 46 +++++++++++++++++++++++----------
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c
index 7cc935d7b7aa..5f1477329bee 100644
--- a/drivers/gpu/drm/selftests/test-drm_mm.c
+++ b/drivers/gpu/drm/selftests/test-drm_mm.c
@@ -59,13 +59,16 @@ static bool assert_no_holes(const struct drm_mm *mm)
drm_mm_for_each_hole(hole, mm, hole_start, hole_end)
count++;
if (count) {
- pr_err("Expected to find no holes (after reserve), found %lu instead\n", count);
+ pr_err("%pS: Expected to find no holes (after reserve), found %lu instead\n",
+ __builtin_return_address(0),
+ count);
return false;
}
drm_mm_for_each_node(hole, mm) {
if (drm_mm_hole_follows(hole)) {
- pr_err("Hole follows node, expected none!\n");
+ pr_err("%pS: Hole follows node, expected none!\n",
+ __builtin_return_address(0));
return false;
}
}
@@ -87,7 +90,8 @@ static bool assert_one_hole(const struct drm_mm *mm, u64 start, u64 end)
drm_mm_for_each_hole(hole, mm, hole_start, hole_end) {
if (start != hole_start || end != hole_end) {
if (ok)
- pr_err("empty mm has incorrect hole, found (%llx, %llx), expect (%llx, %llx)\n",
+ pr_err("%pS: empty mm has incorrect hole, found (%llx, %llx), expect (%llx, %llx)\n",
+ __builtin_return_address(0),
hole_start, hole_end,
start, end);
ok = false;
@@ -95,7 +99,9 @@ static bool assert_one_hole(const struct drm_mm *mm, u64 start, u64 end)
count++;
}
if (count != 1) {
- pr_err("Expected to find one hole, found %lu instead\n", count);
+ pr_err("%pS: Expected to find one hole, found %lu instead\n",
+ __builtin_return_address(0),
+ count);
ok = false;
}
@@ -108,40 +114,48 @@ static bool assert_continuous(const struct drm_mm *mm, u64 size)
unsigned long n;
u64 addr;
- if (!assert_no_holes(mm))
+ if (!assert_no_holes(mm)) {
+ pr_err("%pS: expected no holes!\n",
+ __builtin_return_address(0));
return false;
+ }
n = 0;
addr = 0;
drm_mm_for_each_node(node, mm) {
if (node->start != addr) {
- pr_err("node[%ld] list out of order, expected %llx found %llx\n",
+ pr_err("%pS: node[%ld] list out of order, expected %llx found %llx\n",
+ __builtin_return_address(0),
n, addr, node->start);
return false;
}
if (node->size != size) {
- pr_err("node[%ld].size incorrect, expected %llx, found %llx\n",
+ pr_err("%pS: node[%ld].size incorrect, expected %llx, found %llx\n",
+ __builtin_return_address(0),
n, size, node->size);
return false;
}
if (drm_mm_hole_follows(node)) {
- pr_err("node[%ld] is followed by a hole!\n", n);
+ pr_err("%pS: node[%ld] is followed by a hole!\n",
+ __builtin_return_address(0), n);
return false;
}
found = NULL;
drm_mm_for_each_node_in_range(check, mm, addr, addr + size) {
if (node != check) {
- pr_err("lookup return wrong node, expected start %llx, found %llx\n",
+ pr_err("%pS: lookup return wrong node, expected start %llx, found %llx\n",
+ __builtin_return_address(0),
node->start, check->start);
return false;
}
found = check;
}
if (!found) {
- pr_err("lookup failed for node %llx + %llx\n",
+ pr_err("%pS: lookup failed for node %llx + %llx\n",
+ __builtin_return_address(0),
addr, size);
return false;
}
@@ -170,24 +184,28 @@ static bool assert_node(struct drm_mm_node *node, struct drm_mm *mm,
bool ok = true;
if (!drm_mm_node_allocated(node) || node->mm != mm) {
- pr_err("node not allocated\n");
+ pr_err("%pS: node not allocated\n",
+ __builtin_return_address(0));
ok = false;
}
if (node->size != size) {
- pr_err("node has wrong size, found %llu, expected %llu\n",
+ pr_err("%pS: node has wrong size, found %llu, expected %llu\n",
+ __builtin_return_address(0),
node->size, size);
ok = false;
}
if (misalignment(node, alignment)) {
- pr_err("node is misaligned, start %llx rem %llu, expected alignment %llu\n",
+ pr_err("%pS: node is misaligned, start %llx rem %llu, expected alignment %llu\n",
+ __builtin_return_address(0),
node->start, misalignment(node, alignment), alignment);
ok = false;
}
if (node->color != color) {
- pr_err("node has wrong color, found %lu, expected %lu\n",
+ pr_err("%pS: node has wrong color, found %lu, expected %lu\n",
+ __builtin_return_address(0),
node->color, color);
ok = false;
}
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] lib/rbtree, drm/mm: Add rbtree_replace_node_cached()
2017-11-09 21:24 [PATCH 1/2] lib/rbtree,drm/mm: Add rbtree_replace_node_cached() Chris Wilson
2017-11-09 21:24 ` [PATCH 2/2] drm/selftests/mm: Add callsite indicator to common asserts Chris Wilson
@ 2017-11-10 14:26 ` Davidlohr Bueso
2017-11-21 8:49 ` [PATCH 1/2] lib/rbtree,drm/mm: " Joonas Lahtinen
2017-11-22 10:07 ` [PATCH] " Chris Wilson
2 siblings, 1 reply; 6+ messages in thread
From: Davidlohr Bueso @ 2017-11-10 14:26 UTC (permalink / raw)
To: Chris Wilson
Cc: Daniel Vetter, intel-gfx, dri-devel, Jérôme Glisse,
Andrew Morton
On 2017-11-09 13:24, Chris Wilson wrote:
> Add a variant of rbtree_replace_node() that maintains the leftmost
> cached of struct rbtree_root_cached when replacing nodes within the
> rbtree.
>
> As drm_mm is the only rb_replace_node() being used on an interval tree,
> the mistake looks fairly self-contained. Furthermore the only user of
> drm_mm_replace_node() is its testsuite...
>
> Fixes: f808c13fd373 ("lib/interval_tree: fast overlap detection")
> Testcase: igt/drm_mm/replace
> 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: Andrew Morton <akpm@linux-foundation.org>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Thanks!
Acked-by: Davidlohr Bueso <dbueso@suse.de>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/selftests/mm: Add callsite indicator to common asserts
2017-11-09 21:24 ` [PATCH 2/2] drm/selftests/mm: Add callsite indicator to common asserts Chris Wilson
@ 2017-11-21 8:07 ` Joonas Lahtinen
0 siblings, 0 replies; 6+ messages in thread
From: Joonas Lahtinen @ 2017-11-21 8:07 UTC (permalink / raw)
To: Chris Wilson, dri-devel; +Cc: Daniel Vetter, intel-gfx
On Thu, 2017-11-09 at 21:24 +0000, Chris Wilson wrote:
> The majority of the asserts (validating nodes and ranges) are shared
> amongst several subtests. This makes identification of which caller
> failed hard; but we uniquely identify them if we include the callsite
> into the assertion error message (a single frame stacktrace).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] lib/rbtree,drm/mm: Add rbtree_replace_node_cached()
2017-11-10 14:26 ` [PATCH 1/2] lib/rbtree, drm/mm: Add rbtree_replace_node_cached() Davidlohr Bueso
@ 2017-11-21 8:49 ` Joonas Lahtinen
0 siblings, 0 replies; 6+ messages in thread
From: Joonas Lahtinen @ 2017-11-21 8:49 UTC (permalink / raw)
To: Davidlohr Bueso, Chris Wilson
Cc: Andrew Morton, Daniel Vetter, intel-gfx, Jérôme Glisse,
dri-devel
On Fri, 2017-11-10 at 06:26 -0800, Davidlohr Bueso wrote:
> On 2017-11-09 13:24, Chris Wilson wrote:
> > Add a variant of rbtree_replace_node() that maintains the leftmost
> > cached of struct rbtree_root_cached when replacing nodes within the
> > rbtree.
> >
> > As drm_mm is the only rb_replace_node() being used on an interval tree,
> > the mistake looks fairly self-contained. Furthermore the only user of
> > drm_mm_replace_node() is its testsuite...
> >
> > Fixes: f808c13fd373 ("lib/interval_tree: fast overlap detection")
> > Testcase: igt/drm_mm/replace
> > 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: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Thanks!
>
> Acked-by: Davidlohr Bueso <dbueso@suse.de>
Would you like us to merge this patch through the DRM tree or how would
you like us to proceed?
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] lib/rbtree,drm/mm: Add rbtree_replace_node_cached()
2017-11-09 21:24 [PATCH 1/2] lib/rbtree,drm/mm: Add rbtree_replace_node_cached() Chris Wilson
2017-11-09 21:24 ` [PATCH 2/2] drm/selftests/mm: Add callsite indicator to common asserts Chris Wilson
2017-11-10 14:26 ` [PATCH 1/2] lib/rbtree, drm/mm: Add rbtree_replace_node_cached() Davidlohr Bueso
@ 2017-11-22 10:07 ` Chris Wilson
2 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2017-11-22 10:07 UTC (permalink / raw)
To: akpm
Cc: Davidlohr Bueso, Daniel Vetter, intel-gfx, Joonas Lahtinen,
linux-kernel, dri-devel, Jérôme Glisse
Add a variant of rbtree_replace_node() that maintains the leftmost
cache of struct rbtree_root_cached when replacing nodes within the
rbtree.
As drm_mm is the only rb_replace_node() being used on an interval tree,
the mistake looks fairly self-contained. Furthermore the only user of
drm_mm_replace_node() is its testsuite...
Fixes: f808c13fd373 ("lib/interval_tree: fast overlap detection")
Testcase: igt/drm_mm/replace
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: Andrew Morton <akpm@linux-foundation.org>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20171109212435.9265-1-chris@chris-wilson.co.uk
Acked-by: Davidlohr Bueso <dbueso@suse.de>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/drm_mm.c | 8 +++++---
include/linux/rbtree.h | 2 ++
lib/rbtree.c | 10 ++++++++++
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 61a1c8ea74bc..c3c79ee6119e 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -575,21 +575,23 @@ EXPORT_SYMBOL(drm_mm_remove_node);
*/
void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
{
+ struct drm_mm *mm = old->mm;
+
DRM_MM_BUG_ON(!old->allocated);
*new = *old;
list_replace(&old->node_list, &new->node_list);
- rb_replace_node(&old->rb, &new->rb, &old->mm->interval_tree.rb_root);
+ rb_replace_node_cached(&old->rb, &new->rb, &mm->interval_tree);
if (drm_mm_hole_follows(old)) {
list_replace(&old->hole_stack, &new->hole_stack);
rb_replace_node(&old->rb_hole_size,
&new->rb_hole_size,
- &old->mm->holes_size);
+ &mm->holes_size);
rb_replace_node(&old->rb_hole_addr,
&new->rb_hole_addr,
- &old->mm->holes_addr);
+ &mm->holes_addr);
}
old->allocated = false;
diff --git a/include/linux/rbtree.h b/include/linux/rbtree.h
index d574361943ea..fcbeed4053ef 100644
--- a/include/linux/rbtree.h
+++ b/include/linux/rbtree.h
@@ -99,6 +99,8 @@ extern void rb_replace_node(struct rb_node *victim, struct rb_node *new,
struct rb_root *root);
extern void rb_replace_node_rcu(struct rb_node *victim, struct rb_node *new,
struct rb_root *root);
+extern void rb_replace_node_cached(struct rb_node *victim, struct rb_node *new,
+ struct rb_root_cached *root);
static inline void rb_link_node(struct rb_node *node, struct rb_node *parent,
struct rb_node **rb_link)
diff --git a/lib/rbtree.c b/lib/rbtree.c
index ba4a9d165f1b..d3ff682fd4b8 100644
--- a/lib/rbtree.c
+++ b/lib/rbtree.c
@@ -603,6 +603,16 @@ void rb_replace_node(struct rb_node *victim, struct rb_node *new,
}
EXPORT_SYMBOL(rb_replace_node);
+void rb_replace_node_cached(struct rb_node *victim, struct rb_node *new,
+ struct rb_root_cached *root)
+{
+ rb_replace_node(victim, new, &root->rb_root);
+
+ if (root->rb_leftmost == victim)
+ root->rb_leftmost = new;
+}
+EXPORT_SYMBOL(rb_replace_node_cached);
+
void rb_replace_node_rcu(struct rb_node *victim, struct rb_node *new,
struct rb_root *root)
{
--
2.15.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-11-22 10:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-09 21:24 [PATCH 1/2] lib/rbtree,drm/mm: Add rbtree_replace_node_cached() Chris Wilson
2017-11-09 21:24 ` [PATCH 2/2] drm/selftests/mm: Add callsite indicator to common asserts Chris Wilson
2017-11-21 8:07 ` Joonas Lahtinen
2017-11-10 14:26 ` [PATCH 1/2] lib/rbtree, drm/mm: Add rbtree_replace_node_cached() Davidlohr Bueso
2017-11-21 8:49 ` [PATCH 1/2] lib/rbtree,drm/mm: " Joonas Lahtinen
2017-11-22 10:07 ` [PATCH] " Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).