On Thu, Dec 22, 2016 at 08:36:29AM +0000, Chris Wilson wrote: > The scan state occupies a large proportion of the struct drm_mm and is > rarely used and only contains temporary state. That makes it suitable to > moving to its struct and onto the stack of the callers. > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> Applied to drm-misc, after fixing a missed scan_remove_block in etnaviv_mmu.c. For the future please also cc: driver maintainers so they can ack this. I have some bikesheds on the kernel-doc (or lack thereof for drm_mm_scan), but I think I'll review the entire thing at the end and then send a patch. Thanks, Daniel > --- > drivers/gpu/drm/drm_mm.c | 124 ++++++++++++++++++-------------- > drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 7 +- > drivers/gpu/drm/i915/i915_gem_evict.c | 19 +++-- > drivers/gpu/drm/selftests/test-drm_mm.c | 45 ++++++------ > include/drm/drm_mm.h | 43 +++++++---- > 5 files changed, 138 insertions(+), 100 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c > index b80305484124..21bd2e13738b 100644 > --- a/drivers/gpu/drm/drm_mm.c > +++ b/drivers/gpu/drm/drm_mm.c > @@ -574,7 +574,7 @@ static struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm, > u64 adj_end; > u64 best_size; > > - DRM_MM_BUG_ON(mm->scanned_blocks); > + DRM_MM_BUG_ON(mm->scan_active); > > best = NULL; > best_size = ~0UL; > @@ -618,7 +618,7 @@ static struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_ > u64 adj_end; > u64 best_size; > > - DRM_MM_BUG_ON(mm->scanned_blocks); > + DRM_MM_BUG_ON(mm->scan_active); > > best = NULL; > best_size = ~0UL; > @@ -693,7 +693,7 @@ EXPORT_SYMBOL(drm_mm_replace_node); > * > * The DRM range allocator supports this use-case through the scanning > * interfaces. First a scan operation needs to be initialized with > - * drm_mm_init_scan() or drm_mm_init_scan_with_range(). The driver adds > + * drm_mm_scan_init() or drm_mm_scan_init_with_range(). The driver adds > * objects to the roaster (probably by walking an LRU list, but this can be > * freely implemented) until a suitable hole is found or there's no further > * evictable object. > @@ -710,7 +710,8 @@ EXPORT_SYMBOL(drm_mm_replace_node); > */ > > /** > - * drm_mm_init_scan - initialize lru scanning > + * drm_mm_scan_init - initialize lru scanning > + * @scan: scan state > * @mm: drm_mm to scan > * @size: size of the allocation > * @alignment: alignment of the allocation > @@ -724,26 +725,33 @@ EXPORT_SYMBOL(drm_mm_replace_node); > * As long as the scan list is non-empty, no other operations than > * adding/removing nodes to/from the scan list are allowed. > */ > -void drm_mm_init_scan(struct drm_mm *mm, > +void drm_mm_scan_init(struct drm_mm_scan *scan, > + struct drm_mm *mm, > u64 size, > u64 alignment, > unsigned long color) > { > DRM_MM_BUG_ON(!size); > + DRM_MM_BUG_ON(mm->scan_active); > > - mm->scan_color = color; > - mm->scan_alignment = alignment; > - mm->scan_size = size; > - mm->scanned_blocks = 0; > - mm->scan_hit_start = 0; > - mm->scan_hit_end = 0; > - mm->scan_check_range = 0; > - mm->prev_scanned_node = NULL; > + scan->mm = mm; > + > + scan->color = color; > + scan->alignment = alignment; > + scan->size = size; > + > + scan->check_range = 0; > + > + scan->hit_start = U64_MAX; > + scan->hit_end = 0; > + > + scan->prev_scanned_node = NULL; > } > -EXPORT_SYMBOL(drm_mm_init_scan); > +EXPORT_SYMBOL(drm_mm_scan_init); > > /** > - * drm_mm_init_scan - initialize range-restricted lru scanning > + * drm_mm_scan_init_with_range - initialize range-restricted lru scanning > + * @scan: scan state > * @mm: drm_mm to scan > * @size: size of the allocation > * @alignment: alignment of the allocation > @@ -759,7 +767,8 @@ EXPORT_SYMBOL(drm_mm_init_scan); > * As long as the scan list is non-empty, no other operations than > * adding/removing nodes to/from the scan list are allowed. > */ > -void drm_mm_init_scan_with_range(struct drm_mm *mm, > +void drm_mm_scan_init_with_range(struct drm_mm_scan *scan, > + struct drm_mm *mm, > u64 size, > u64 alignment, > unsigned long color, > @@ -768,19 +777,25 @@ void drm_mm_init_scan_with_range(struct drm_mm *mm, > { > DRM_MM_BUG_ON(start >= end); > DRM_MM_BUG_ON(!size || size > end - start); > + DRM_MM_BUG_ON(mm->scan_active); > + > + scan->mm = mm; > + > + scan->color = color; > + scan->alignment = alignment; > + scan->size = size; > + > + DRM_MM_BUG_ON(end <= start); > + scan->range_start = start; > + scan->range_end = end; > + scan->check_range = 1; > > - mm->scan_color = color; > - mm->scan_alignment = alignment; > - mm->scan_size = size; > - mm->scanned_blocks = 0; > - mm->scan_hit_start = 0; > - mm->scan_hit_end = 0; > - mm->scan_start = start; > - mm->scan_end = end; > - mm->scan_check_range = 1; > - mm->prev_scanned_node = NULL; > + scan->hit_start = U64_MAX; > + scan->hit_end = 0; > + > + scan->prev_scanned_node = NULL; > } > -EXPORT_SYMBOL(drm_mm_init_scan_with_range); > +EXPORT_SYMBOL(drm_mm_scan_init_with_range); > > /** > * drm_mm_scan_add_block - add a node to the scan list > @@ -792,46 +807,46 @@ EXPORT_SYMBOL(drm_mm_init_scan_with_range); > * Returns: > * True if a hole has been found, false otherwise. > */ > -bool drm_mm_scan_add_block(struct drm_mm_node *node) > +bool drm_mm_scan_add_block(struct drm_mm_scan *scan, > + struct drm_mm_node *node) > { > - struct drm_mm *mm = node->mm; > + struct drm_mm *mm = scan->mm; > struct drm_mm_node *prev_node; > u64 hole_start, hole_end; > u64 adj_start, adj_end; > > - mm->scanned_blocks++; > - > + DRM_MM_BUG_ON(node->mm != mm); > + DRM_MM_BUG_ON(!node->allocated); > DRM_MM_BUG_ON(node->scanned_block); > node->scanned_block = 1; > + mm->scan_active++; > > - prev_node = list_entry(node->node_list.prev, struct drm_mm_node, > - node_list); > + prev_node = list_prev_entry(node, node_list); > > node->scanned_preceeds_hole = prev_node->hole_follows; > prev_node->hole_follows = 1; > list_del(&node->node_list); > node->node_list.prev = &prev_node->node_list; > - node->node_list.next = &mm->prev_scanned_node->node_list; > - mm->prev_scanned_node = node; > + node->node_list.next = &scan->prev_scanned_node->node_list; > + scan->prev_scanned_node = node; > > adj_start = hole_start = drm_mm_hole_node_start(prev_node); > adj_end = hole_end = drm_mm_hole_node_end(prev_node); > > - if (mm->scan_check_range) { > - if (adj_start < mm->scan_start) > - adj_start = mm->scan_start; > - if (adj_end > mm->scan_end) > - adj_end = mm->scan_end; > + if (scan->check_range) { > + if (adj_start < scan->range_start) > + adj_start = scan->range_start; > + if (adj_end > scan->range_end) > + adj_end = scan->range_end; > } > > if (mm->color_adjust) > - mm->color_adjust(prev_node, mm->scan_color, > - &adj_start, &adj_end); > + mm->color_adjust(prev_node, scan->color, &adj_start, &adj_end); > > if (check_free_hole(adj_start, adj_end, > - mm->scan_size, mm->scan_alignment)) { > - mm->scan_hit_start = hole_start; > - mm->scan_hit_end = hole_end; > + scan->size, scan->alignment)) { > + scan->hit_start = hole_start; > + scan->hit_end = hole_end; > return true; > } > > @@ -856,24 +871,25 @@ EXPORT_SYMBOL(drm_mm_scan_add_block); > * True if this block should be evicted, false otherwise. Will always > * return false when no hole has been found. > */ > -bool drm_mm_scan_remove_block(struct drm_mm_node *node) > +bool drm_mm_scan_remove_block(struct drm_mm_scan *scan, > + struct drm_mm_node *node) > { > - struct drm_mm *mm = node->mm; > struct drm_mm_node *prev_node; > > - mm->scanned_blocks--; > - > + DRM_MM_BUG_ON(node->mm != scan->mm); > DRM_MM_BUG_ON(!node->scanned_block); > node->scanned_block = 0; > > - prev_node = list_entry(node->node_list.prev, struct drm_mm_node, > - node_list); > + DRM_MM_BUG_ON(!node->mm->scan_active); > + node->mm->scan_active--; > + > + prev_node = list_prev_entry(node, node_list); > > prev_node->hole_follows = node->scanned_preceeds_hole; > list_add(&node->node_list, &prev_node->node_list); > > - return (drm_mm_hole_node_end(node) > mm->scan_hit_start && > - node->start < mm->scan_hit_end); > + return (drm_mm_hole_node_end(node) > scan->hit_start && > + node->start < scan->hit_end); > } > EXPORT_SYMBOL(drm_mm_scan_remove_block); > > @@ -890,7 +906,7 @@ void drm_mm_init(struct drm_mm *mm, u64 start, u64 size) > DRM_MM_BUG_ON(start + size <= start); > > INIT_LIST_HEAD(&mm->hole_stack); > - mm->scanned_blocks = 0; > + mm->scan_active = 0; > > /* Clever trick to avoid a special case in the free hole tracking. */ > INIT_LIST_HEAD(&mm->head_node.node_list); > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > index 169ac96e8f08..fe1e886dcabb 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c > @@ -113,6 +113,7 @@ static int etnaviv_iommu_find_iova(struct etnaviv_iommu *mmu, > > while (1) { > struct etnaviv_vram_mapping *m, *n; > + struct drm_mm_scan scan; > struct list_head list; > bool found; > > @@ -134,7 +135,7 @@ static int etnaviv_iommu_find_iova(struct etnaviv_iommu *mmu, > } > > /* Try to retire some entries */ > - drm_mm_init_scan(&mmu->mm, size, 0, 0); > + drm_mm_scan_init(&scan, &mmu->mm, size, 0, 0); > > found = 0; > INIT_LIST_HEAD(&list); > @@ -151,7 +152,7 @@ static int etnaviv_iommu_find_iova(struct etnaviv_iommu *mmu, > continue; > > list_add(&free->scan_node, &list); > - if (drm_mm_scan_add_block(&free->vram_node)) { > + if (drm_mm_scan_add_block(&scan, &free->vram_node)) { > found = true; > break; > } > @@ -171,7 +172,7 @@ static int etnaviv_iommu_find_iova(struct etnaviv_iommu *mmu, > * can leave the block pinned. > */ > list_for_each_entry_safe(m, n, &list, scan_node) > - if (!drm_mm_scan_remove_block(&m->vram_node)) > + if (!drm_mm_scan_remove_block(&scan, &m->vram_node)) > list_del_init(&m->scan_node); > > /* > diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c > index 6457fd0c33a8..6db0d73c0aa7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_evict.c > +++ b/drivers/gpu/drm/i915/i915_gem_evict.c > @@ -51,7 +51,10 @@ static bool ggtt_is_idle(struct drm_i915_private *dev_priv) > } > > static bool > -mark_free(struct i915_vma *vma, unsigned int flags, struct list_head *unwind) > +mark_free(struct drm_mm_scan *scan, > + struct i915_vma *vma, > + unsigned int flags, > + struct list_head *unwind) > { > if (i915_vma_is_pinned(vma)) > return false; > @@ -63,7 +66,7 @@ mark_free(struct i915_vma *vma, unsigned int flags, struct list_head *unwind) > return false; > > list_add(&vma->exec_list, unwind); > - return drm_mm_scan_add_block(&vma->node); > + return drm_mm_scan_add_block(scan, &vma->node); > } > > /** > @@ -97,6 +100,7 @@ i915_gem_evict_something(struct i915_address_space *vm, > unsigned flags) > { > struct drm_i915_private *dev_priv = vm->i915; > + struct drm_mm_scan scan; > struct list_head eviction_list; > struct list_head *phases[] = { > &vm->inactive_list, > @@ -123,11 +127,12 @@ i915_gem_evict_something(struct i915_address_space *vm, > * object on the TAIL. > */ > if (start != 0 || end != vm->total) { > - drm_mm_init_scan_with_range(&vm->mm, min_size, > + drm_mm_scan_init_with_range(&scan, &vm->mm, min_size, > alignment, cache_level, > start, end); > } else > - drm_mm_init_scan(&vm->mm, min_size, alignment, cache_level); > + drm_mm_scan_init(&scan, &vm->mm, min_size, > + alignment, cache_level); > > /* Retire before we search the active list. Although we have > * reasonable accuracy in our retirement lists, we may have > @@ -144,13 +149,13 @@ i915_gem_evict_something(struct i915_address_space *vm, > phase = phases; > do { > list_for_each_entry(vma, *phase, vm_link) > - if (mark_free(vma, flags, &eviction_list)) > + if (mark_free(&scan, vma, flags, &eviction_list)) > goto found; > } while (*++phase); > > /* Nothing found, clean up and bail out! */ > list_for_each_entry_safe(vma, next, &eviction_list, exec_list) { > - ret = drm_mm_scan_remove_block(&vma->node); > + ret = drm_mm_scan_remove_block(&scan, &vma->node); > BUG_ON(ret); > > INIT_LIST_HEAD(&vma->exec_list); > @@ -199,7 +204,7 @@ i915_gem_evict_something(struct i915_address_space *vm, > * of any of our objects, thus corrupting the list). > */ > list_for_each_entry_safe(vma, next, &eviction_list, exec_list) { > - if (drm_mm_scan_remove_block(&vma->node)) > + if (drm_mm_scan_remove_block(&scan, &vma->node)) > __i915_vma_pin(vma); > else > list_del_init(&vma->exec_list); > diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c > index e2ed9efb361c..997f2bc93b9b 100644 > --- a/drivers/gpu/drm/selftests/test-drm_mm.c > +++ b/drivers/gpu/drm/selftests/test-drm_mm.c > @@ -1114,11 +1114,11 @@ static int igt_align64(void *ignored) > return igt_align_pot(64); > } > > -static void show_scan(const struct drm_mm *scan) > +static void show_scan(const struct drm_mm_scan *scan) > { > pr_info("scan: hit [%llx, %llx], size=%lld, align=%lld, color=%ld\n", > - scan->scan_hit_start, scan->scan_hit_end, > - scan->scan_size, scan->scan_alignment, scan->scan_color); > + scan->hit_start, scan->hit_end, > + scan->size, scan->alignment, scan->color); > } > > static void show_holes(const struct drm_mm *mm, int count) > @@ -1158,7 +1158,7 @@ struct evict_node { > struct list_head link; > }; > > -static bool evict_nodes(struct drm_mm *mm, > +static bool evict_nodes(struct drm_mm_scan *scan, > struct evict_node *nodes, > unsigned int *order, > unsigned int count, > @@ -1170,18 +1170,16 @@ static bool evict_nodes(struct drm_mm *mm, > for (i = 0; i < count; i++) { > e = &nodes[order ? order[i] : i]; > list_add(&e->link, evict_list); > - if (drm_mm_scan_add_block(&e->node)) > + if (drm_mm_scan_add_block(scan, &e->node)) > break; > } > list_for_each_entry_safe(e, en, evict_list, link) { > - if (!drm_mm_scan_remove_block(&e->node)) > + if (!drm_mm_scan_remove_block(scan, &e->node)) > list_del(&e->link); > } > if (list_empty(evict_list)) { > pr_err("Failed to find eviction: size=%lld [avail=%d], align=%lld (color=%lu)\n", > - mm->scan_size, count, > - mm->scan_alignment, > - mm->scan_color); > + scan->size, count, scan->alignment, scan->color); > return false; > } > > @@ -1195,19 +1193,20 @@ static bool evict_nothing(struct drm_mm *mm, > unsigned int total_size, > struct evict_node *nodes) > { > + struct drm_mm_scan scan; > LIST_HEAD(evict_list); > struct evict_node *e; > struct drm_mm_node *node; > unsigned int n; > > - drm_mm_init_scan(mm, 1, 0, 0); > + drm_mm_scan_init(&scan, mm, 1, 0, 0); > for (n = 0; n < total_size; n++) { > e = &nodes[n]; > list_add(&e->link, &evict_list); > - drm_mm_scan_add_block(&e->node); > + drm_mm_scan_add_block(&scan, &e->node); > } > list_for_each_entry(e, &evict_list, link) > - drm_mm_scan_remove_block(&e->node); > + drm_mm_scan_remove_block(&scan, &e->node); > > for (n = 0; n < total_size; n++) { > e = &nodes[n]; > @@ -1241,19 +1240,21 @@ static bool evict_everything(struct drm_mm *mm, > unsigned int total_size, > struct evict_node *nodes) > { > + struct drm_mm_scan scan; > LIST_HEAD(evict_list); > struct evict_node *e; > unsigned int n; > int err; > > - drm_mm_init_scan(mm, total_size, 0, 0); > + drm_mm_scan_init(&scan, mm, total_size, 0, 0); > for (n = 0; n < total_size; n++) { > e = &nodes[n]; > list_add(&e->link, &evict_list); > - drm_mm_scan_add_block(&e->node); > + if (drm_mm_scan_add_block(&scan, &e->node)) > + break; > } > list_for_each_entry(e, &evict_list, link) { > - if (!drm_mm_scan_remove_block(&e->node)) { > + if (!drm_mm_scan_remove_block(&scan, &e->node)) { > pr_err("Node %lld not marked for eviction!\n", > e->node.start); > list_del(&e->link); > @@ -1287,15 +1288,16 @@ static int evict_something(struct drm_mm *mm, > unsigned int alignment, > const struct insert_mode *mode) > { > + struct drm_mm_scan scan; > LIST_HEAD(evict_list); > struct evict_node *e; > struct drm_mm_node tmp; > int err; > > - drm_mm_init_scan_with_range(mm, > + drm_mm_scan_init_with_range(&scan, mm, > size, alignment, 0, > range_start, range_end); > - if (!evict_nodes(mm, > + if (!evict_nodes(&scan, > nodes, order, count, > &evict_list)) > return -EINVAL; > @@ -1307,7 +1309,7 @@ static int evict_something(struct drm_mm *mm, > if (err) { > pr_err("Failed to insert into eviction hole: size=%d, align=%d\n", > size, alignment); > - show_scan(mm); > + show_scan(&scan); > show_holes(mm, 3); > return err; > } > @@ -1864,15 +1866,16 @@ static int evict_color(struct drm_mm *mm, > unsigned long color, > const struct insert_mode *mode) > { > + struct drm_mm_scan scan; > LIST_HEAD(evict_list); > struct evict_node *e; > struct drm_mm_node tmp; > int err; > > - drm_mm_init_scan_with_range(mm, > + drm_mm_scan_init_with_range(&scan, mm, > size, alignment, color, > range_start, range_end); > - if (!evict_nodes(mm, > + if (!evict_nodes(&scan, > nodes, order, count, > &evict_list)) > return -EINVAL; > @@ -1884,7 +1887,7 @@ static int evict_color(struct drm_mm *mm, > if (err) { > pr_err("Failed to insert into eviction hole: size=%d, align=%d, color=%lu, err=%d\n", > size, alignment, color, err); > - show_scan(mm); > + show_scan(&scan); > show_holes(mm, 3); > return err; > } > diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h > index 72e0c0ddf8d0..fcad718c5fb4 100644 > --- a/include/drm/drm_mm.h > +++ b/include/drm/drm_mm.h > @@ -98,20 +98,29 @@ struct drm_mm { > /* Keep an interval_tree for fast lookup of drm_mm_nodes by address. */ > struct rb_root interval_tree; > > - unsigned int scan_check_range : 1; > - unsigned int scanned_blocks; > - unsigned long scan_color; > - u64 scan_alignment; > - u64 scan_size; > - u64 scan_hit_start; > - u64 scan_hit_end; > - u64 scan_start; > - u64 scan_end; > - struct drm_mm_node *prev_scanned_node; > - > void (*color_adjust)(const struct drm_mm_node *node, > unsigned long color, > u64 *start, u64 *end); > + > + unsigned long scan_active; > +}; > + > +struct drm_mm_scan { > + struct drm_mm *mm; > + > + u64 size; > + u64 alignment; > + > + u64 range_start; > + u64 range_end; > + > + u64 hit_start; > + u64 hit_end; > + > + struct drm_mm_node *prev_scanned_node; > + > + unsigned long color; > + bool check_range : 1; > }; > > /** > @@ -378,18 +387,22 @@ __drm_mm_interval_first(const struct drm_mm *mm, u64 start, u64 last); > node__ && node__->start < (end__); \ > node__ = list_next_entry(node__, node_list)) > > -void drm_mm_init_scan(struct drm_mm *mm, > +void drm_mm_scan_init(struct drm_mm_scan *scan, > + struct drm_mm *mm, > u64 size, > u64 alignment, > unsigned long color); > -void drm_mm_init_scan_with_range(struct drm_mm *mm, > +void drm_mm_scan_init_with_range(struct drm_mm_scan *scan, > + struct drm_mm *mm, > u64 size, > u64 alignment, > unsigned long color, > u64 start, > u64 end); > -bool drm_mm_scan_add_block(struct drm_mm_node *node); > -bool drm_mm_scan_remove_block(struct drm_mm_node *node); > +bool drm_mm_scan_add_block(struct drm_mm_scan *scan, > + struct drm_mm_node *node); > +bool drm_mm_scan_remove_block(struct drm_mm_scan *scan, > + struct drm_mm_node *node); > > void drm_mm_debug_table(const struct drm_mm *mm, const char *prefix); > #ifdef CONFIG_DEBUG_FS > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx