Re: [PATCH] drm/mm: Fix search for smallest hole satisfying constraints

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Mar 18, 2014 at 09:58:14AM +0900, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> entry->size is the size of the node, not the size of the hole after it.
> So the code would actually find the hole which can satisfy the
> constraints and which is preceded by the smallest node, not the smallest
> hole satisfying the constraints.
> 
> Reported-by: "Huang, FrankR" <FrankR.Huang@amd.com>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

But drm-next just gained my kerneldoc patch for drm_mm, so can you please
respin your patch and update the docs too? While at it ... could you
perhaps smash a bit of kerneldoc on top of enum drm_mm_search_flags, I
seem to have missed it. With that this is

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_mm.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index af93cc5..5d921e5 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -306,8 +306,8 @@ static struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm,
>  {
>  	struct drm_mm_node *entry;
>  	struct drm_mm_node *best;
> -	unsigned long adj_start;
> -	unsigned long adj_end;
> +	unsigned long hole_start;
> +	unsigned long hole_end;
>  	unsigned long best_size;
>  
>  	BUG_ON(mm->scanned_blocks);
> @@ -315,7 +315,10 @@ static struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm,
>  	best = NULL;
>  	best_size = ~0UL;
>  
> -	drm_mm_for_each_hole(entry, mm, adj_start, adj_end) {
> +	drm_mm_for_each_hole(entry, mm, hole_start, hole_end) {
> +		unsigned long adj_start = hole_start;
> +		unsigned long adj_end = hole_end;
> +
>  		if (mm->color_adjust) {
>  			mm->color_adjust(entry, color, &adj_start, &adj_end);
>  			if (adj_end <= adj_start)
> @@ -328,9 +331,9 @@ static struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm *mm,
>  		if (!(flags & DRM_MM_SEARCH_BEST))
>  			return entry;
>  
> -		if (entry->size < best_size) {
> +		if ((hole_end - hole_start) < best_size) {
>  			best = entry;
> -			best_size = entry->size;
> +			best_size = hole_end - hole_start;
>  		}
>  	}
>  
> @@ -341,14 +344,14 @@ static struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_
>  							unsigned long size,
>  							unsigned alignment,
>  							unsigned long color,
> -							unsigned long start,
> -							unsigned long end,
> +							unsigned long range_start,
> +							unsigned long range_end,
>  							enum drm_mm_search_flags flags)
>  {
>  	struct drm_mm_node *entry;
>  	struct drm_mm_node *best;
> -	unsigned long adj_start;
> -	unsigned long adj_end;
> +	unsigned long hole_start;
> +	unsigned long hole_end;
>  	unsigned long best_size;
>  
>  	BUG_ON(mm->scanned_blocks);
> @@ -356,11 +359,9 @@ static struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_
>  	best = NULL;
>  	best_size = ~0UL;
>  
> -	drm_mm_for_each_hole(entry, mm, adj_start, adj_end) {
> -		if (adj_start < start)
> -			adj_start = start;
> -		if (adj_end > end)
> -			adj_end = end;
> +	drm_mm_for_each_hole(entry, mm, hole_start, hole_end) {
> +		unsigned long adj_start = max(hole_start, range_start);
> +		unsigned long adj_end = min(hole_end, range_end);
>  
>  		if (mm->color_adjust) {
>  			mm->color_adjust(entry, color, &adj_start, &adj_end);
> @@ -374,9 +375,9 @@ static struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct drm_
>  		if (!(flags & DRM_MM_SEARCH_BEST))
>  			return entry;
>  
> -		if (entry->size < best_size) {
> +		if ((hole_end - hole_start) < best_size) {
>  			best = entry;
> -			best_size = entry->size;
> +			best_size = hole_end - hole_start;
>  		}
>  	}
>  
> -- 
> 1.9.0
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch