Re: [PATCH v4 26/38] drm: Extract struct drm_mm_scan from struct drm_mm

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@chris-wilson.co.uk Reviewed-by: Joonas Lahtinen joonas.lahtinen@linux.intel.com

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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel