| Message ID | 20170204111913.12416-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On la, 2017-02-04 at 11:19 +0000, Chris Wilson wrote: > As we require valid start/end parameters, we can replace the initial > potential NULL with a pointer to the drm_mm.head_node and so reduce the > test on every iteration from a NULL + address comparison to just an > address comparison. > > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-26 (-26) > function old new delta > i915_gem_evict_for_node 719 693 -26 > > (No other users outside of the test harness.) > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Slightly confused by the mixing of [start, end] and [start, end). Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
On Mon, Feb 06, 2017 at 12:21:48PM +0200, Joonas Lahtinen wrote: > On la, 2017-02-04 at 11:19 +0000, Chris Wilson wrote: > > As we require valid start/end parameters, we can replace the initial > > potential NULL with a pointer to the drm_mm.head_node and so reduce the > > test on every iteration from a NULL + address comparison to just an > > address comparison. > > > > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-26 (-26) > > function old new delta > > i915_gem_evict_for_node 719 693 -26 > > > > (No other users outside of the test harness.) > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Slightly confused by the mixing of [start, end] and [start, end). Yup. Getting the code able to address the full 64b range (i.e to use inclusive ends everwhere (end = start + size - 1) like resource management) is on the todo wishlist. Maybe even looking at whether this could be migrated into more generic extents managements. But atm it is fully cappable of describing our hw, so the goal is make sure it is efficient for our and the rest of drm's uses. -Chris
On Mon, Feb 06, 2017 at 12:21:48PM +0200, Joonas Lahtinen wrote: > On la, 2017-02-04 at 11:19 +0000, Chris Wilson wrote: > > As we require valid start/end parameters, we can replace the initial > > potential NULL with a pointer to the drm_mm.head_node and so reduce the > > test on every iteration from a NULL + address comparison to just an > > address comparison. > > > > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-26 (-26) > > function old new delta > > i915_gem_evict_for_node 719 693 -26 > > > > (No other users outside of the test harness.) > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Slightly confused by the mixing of [start, end] and [start, end). > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Applied to drm-misc-next for 4.12. Thanks, Daniel
diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 8bfb0b327267..f794089d30ac 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -170,7 +170,7 @@ struct drm_mm_node * __drm_mm_interval_first(const struct drm_mm *mm, u64 start, u64 last) { return drm_mm_interval_tree_iter_first((struct rb_root *)&mm->interval_tree, - start, last); + start, last) ?: (struct drm_mm_node *)&mm->head_node; } EXPORT_SYMBOL(__drm_mm_interval_first); diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c index 1e71bc182ca9..2958f596081e 100644 --- a/drivers/gpu/drm/selftests/test-drm_mm.c +++ b/drivers/gpu/drm/selftests/test-drm_mm.c @@ -839,16 +839,18 @@ static bool assert_contiguous_in_range(struct drm_mm *mm, n++; } - drm_mm_for_each_node_in_range(node, mm, 0, start) { - if (node) { + if (start > 0) { + node = __drm_mm_interval_first(mm, 0, start - 1); + if (node->allocated) { pr_err("node before start: node=%llx+%llu, start=%llx\n", node->start, node->size, start); return false; } } - drm_mm_for_each_node_in_range(node, mm, end, U64_MAX) { - if (node) { + if (end < U64_MAX) { + node = __drm_mm_interval_first(mm, end, U64_MAX); + if (node->allocated) { pr_err("node after end: node=%llx+%llu, end=%llx\n", node->start, node->size, end); return false; diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index d81b0ba9921f..f262da180117 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -459,10 +459,13 @@ __drm_mm_interval_first(const struct drm_mm *mm, u64 start, u64 last); * but using the internal interval tree to accelerate the search for the * starting node, and so not safe against removal of elements. It assumes * that @end is within (or is the upper limit of) the drm_mm allocator. + * If [@start, @end] are beyond the range of the drm_mm, the iterator may walk + * over the special _unallocated_ &drm_mm.head_node, and may even continue + * indefinitely. */ #define drm_mm_for_each_node_in_range(node__, mm__, start__, end__) \ for (node__ = __drm_mm_interval_first((mm__), (start__), (end__)-1); \ - node__ && node__->start < (end__); \ + node__->start < (end__); \ node__ = list_next_entry(node__, node_list)) void drm_mm_scan_init_with_range(struct drm_mm_scan *scan,
As we require valid start/end parameters, we can replace the initial potential NULL with a pointer to the drm_mm.head_node and so reduce the test on every iteration from a NULL + address comparison to just an address comparison. add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-26 (-26) function old new delta i915_gem_evict_for_node 719 693 -26 (No other users outside of the test harness.) Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/drm_mm.c | 2 +- drivers/gpu/drm/selftests/test-drm_mm.c | 10 ++++++---- include/drm/drm_mm.h | 5 ++++- 3 files changed, 11 insertions(+), 6 deletions(-)