drm: Only evict the blocks required to create the requested hole

Commit Message

Chris Wilson Dec. 16, 2012, 5:51 p.m. UTC

Avoid clobbering adjacent blocks if they happen to expire earlier and
amalgamate together to form the requested hole.

In passing this fixes a regression from
commit ea7b1dd44867e9cd6bac67e7c9fc3f128b5b255c
Author: Daniel Vetter <[e-mail]>
Date:   Fri Feb 18 17:59:12 2011 +0100

    drm: mm: track free areas implicitly

which swaps the end address for size (with a potential overflow) and
effectively causes the eviction code to clobber almost all earlier
buffers above the evictee.

Signed-off-by: Chris Wilson <[e-mail]>
Cc: Daniel Vetter <[e-mail]>
---
 drivers/gpu/drm/drm_mm.c |   58 +++++++++++++++++++++++++---------------------
 include/drm/drm_mm.h     |    2 +-
 2 files changed, 33 insertions(+), 27 deletions(-)

Comments

Chris Wilson Dec. 18, 2012, 10:37 a.m. UTC | #1

On Sun, 16 Dec 2012 17:51:41 +0000, Chris Wilson <[e-mail]> wrote:
> Avoid clobbering adjacent blocks if they happen to expire earlier and
> amalgamate together to form the requested hole.
> 
> In passing this fixes a regression from
> commit ea7b1dd44867e9cd6bac67e7c9fc3f128b5b255c
> Author: Daniel Vetter <[e-mail]>
> Date:   Fri Feb 18 17:59:12 2011 +0100
> 
>     drm: mm: track free areas implicitly
> 
> which swaps the end address for size (with a potential overflow) and
> effectively causes the eviction code to clobber almost all earlier
> buffers above the evictee.
> 
> Signed-off-by: Chris Wilson <[e-mail]>
> Cc: Daniel Vetter <[e-mail]>

On IRC I mentioned that I feared there be dragon lurking here. They
turned out to be figments of my own code - a later patch to adjust the
allocation of nodes was incomplete.

Please review and consider this patch.
-Chris

Patch

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 7103aa3..c93729c 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -421,6 +421,25 @@  static int check_free_hole(unsigned long start, unsigned long end,
 	return end >= start + size;
 }
 
+static int adjust_free_hole(unsigned long *start, unsigned long *end,
+			    unsigned long size, unsigned alignment)
+{
+	if (*end - *start < size)
+		return 0;
+
+	if (alignment) {
+		unsigned tmp = *start % alignment;
+		if (tmp)
+			*start += alignment - tmp;
+	}
+
+	if  (*end - *start < size)
+		return 0;
+
+	*end = *start + size;
+	return 1;
+}
+
 struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm,
 					       unsigned long size,
 					       unsigned alignment,
@@ -545,7 +564,7 @@  void drm_mm_init_scan(struct drm_mm *mm,
 	mm->scan_size = size;
 	mm->scanned_blocks = 0;
 	mm->scan_hit_start = 0;
-	mm->scan_hit_size = 0;
+	mm->scan_hit_end = 0;
 	mm->scan_check_range = 0;
 	mm->prev_scanned_node = NULL;
 }
@@ -572,7 +591,7 @@  void drm_mm_init_scan_with_range(struct drm_mm *mm,
 	mm->scan_size = size;
 	mm->scanned_blocks = 0;
 	mm->scan_hit_start = 0;
-	mm->scan_hit_size = 0;
+	mm->scan_hit_end = 0;
 	mm->scan_start = start;
 	mm->scan_end = end;
 	mm->scan_check_range = 1;
@@ -591,8 +610,6 @@  int drm_mm_scan_add_block(struct drm_mm_node *node)
 	struct drm_mm *mm = node->mm;
 	struct drm_mm_node *prev_node;
 	unsigned long hole_start, hole_end;
-	unsigned long adj_start;
-	unsigned long adj_end;
 
 	mm->scanned_blocks++;
 
@@ -612,24 +629,21 @@  int drm_mm_scan_add_block(struct drm_mm_node *node)
 	hole_start = drm_mm_hole_node_start(prev_node);
 	hole_end = drm_mm_hole_node_end(prev_node);
 
-	adj_start = hole_start;
-	adj_end = hole_end;
-
 	if (mm->color_adjust)
-		mm->color_adjust(prev_node, mm->scan_color, &adj_start, &adj_end);
+		mm->color_adjust(prev_node, mm->scan_color,
+				 &hole_start, &hole_end);
 
 	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 (hole_start < mm->scan_start)
+			hole_start = mm->scan_start;
+		if (hole_end > mm->scan_end)
+			hole_end = mm->scan_end;
 	}
 
-	if (check_free_hole(adj_start, adj_end,
-			    mm->scan_size, mm->scan_alignment)) {
+	if (adjust_free_hole(&hole_start, &hole_end,
+			     mm->scan_size, mm->scan_alignment)) {
 		mm->scan_hit_start = hole_start;
-		mm->scan_hit_size = hole_end;
-
+		mm->scan_hit_end = hole_end;
 		return 1;
 	}
 
@@ -668,16 +682,8 @@  int drm_mm_scan_remove_block(struct drm_mm_node *node)
 	INIT_LIST_HEAD(&node->node_list);
 	list_add(&node->node_list, &prev_node->node_list);
 
-	/* Only need to check for containement because start&size for the
-	 * complete resulting free block (not just the desired part) is
-	 * stored. */
-	if (node->start >= mm->scan_hit_start &&
-	    node->start + node->size
-	    		<= mm->scan_hit_start + mm->scan_hit_size) {
-		return 1;
-	}
-
-	return 0;
+	return (node->start + node->size >= mm->scan_hit_start &&
+		node->start <= mm->scan_hit_end);
 }
 EXPORT_SYMBOL(drm_mm_scan_remove_block);
 
diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
index 9a08a45..9e06ca1 100644
--- a/include/drm/drm_mm.h
+++ b/include/drm/drm_mm.h
@@ -70,7 +70,7 @@  struct drm_mm {
 	unsigned long scan_color;
 	unsigned long scan_size;
 	unsigned long scan_hit_start;
-	unsigned scan_hit_size;
+	unsigned long scan_hit_end;
 	unsigned scanned_blocks;
 	unsigned long scan_start;
 	unsigned long scan_end;