Re: [PATCH] drm,i915: Introduce drm_malloc_gfp()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Oct 08, 2015 at 02:39:24PM +0100, Chris Wilson wrote:
> I have instances where I want to use drm_malloc_ab() but with a custom
> gfp mask. And with those, where I want a temporary allocation, I want to
> try a high-order kmalloc() before using a vmalloc().
> 
> So refactor my usage into drm_malloc_gfp().
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c            |  4 +---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  8 +++-----
>  drivers/gpu/drm/i915/i915_gem_gtt.c        |  5 +++--
>  drivers/gpu/drm/i915/i915_gem_userptr.c    | 15 ++++-----------
>  include/drm/drm_mem_util.h                 | 19 +++++++++++++++++++
>  5 files changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fe672d4a4d73..c81127518083 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2346,9 +2346,7 @@ void *i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj)
>  		int n;
>  
>  		n = obj->base.size >> PAGE_SHIFT;
> -		pages = kmalloc(n*sizeof(*pages), GFP_TEMPORARY | __GFP_NOWARN);
> -		if (pages == NULL)
> -			pages = drm_malloc_ab(n, sizeof(*pages));
> +		pages = drm_malloc_gfp(n, sizeof(*pages), GFP_TEMPORARY);
>  		if (pages != NULL) {
>  			n = 0;
>  			for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index c35c9dc526e7..91fb7417efc0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1730,11 +1730,9 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
>  		return -EINVAL;
>  	}
>  
> -	exec2_list = kmalloc(sizeof(*exec2_list)*args->buffer_count,
> -			     GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
> -	if (exec2_list == NULL)
> -		exec2_list = drm_malloc_ab(sizeof(*exec2_list),
> -					   args->buffer_count);
> +	exec2_list = drm_malloc_gfp(sizeof(*exec2_list),
> +				    args->buffer_count,
> +				    GFP_TEMPORARY);
>  	if (exec2_list == NULL) {
>  		DRM_DEBUG("Failed to allocate exec list for %d buffers\n",
>  			  args->buffer_count);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 6e8c86d829d2..7820e8983136 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3221,8 +3221,9 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
>  	int ret = -ENOMEM;
>  
>  	/* Allocate a temporary list of source pages for random access. */
> -	page_addr_list = drm_malloc_ab(obj->base.size / PAGE_SIZE,
> -				       sizeof(dma_addr_t));
> +	page_addr_list = drm_malloc_gfp(obj->base.size / PAGE_SIZE,
> +					sizeof(dma_addr_t),
> +					GFP_TEMPORARY);
>  	if (!page_addr_list)
>  		return ERR_PTR(ret);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index afd4c2c4cc04..885605c38e7c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -575,10 +575,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
>  	ret = -ENOMEM;
>  	pinned = 0;
>  
> -	pvec = kmalloc(npages*sizeof(struct page *),
> -		       GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
> -	if (pvec == NULL)
> -		pvec = drm_malloc_ab(npages, sizeof(struct page *));
> +	pvec = drm_malloc_gfp(npages, sizeof(struct page *), GFP_TEMPORARY);
>  	if (pvec != NULL) {
>  		struct mm_struct *mm = obj->userptr.mm->mm;
>  
> @@ -715,14 +712,10 @@ i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
>  	pvec = NULL;
>  	pinned = 0;
>  	if (obj->userptr.mm->mm == current->mm) {
> -		pvec = kmalloc(num_pages*sizeof(struct page *),
> -			       GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
> +		pvec = drm_malloc_gfp(num_pages, sizeof(struct page *), GFP_TEMPORARY);
>  		if (pvec == NULL) {
> -			pvec = drm_malloc_ab(num_pages, sizeof(struct page *));
> -			if (pvec == NULL) {
> -				__i915_gem_userptr_set_active(obj, false);
> -				return -ENOMEM;
> -			}
> +			__i915_gem_userptr_set_active(obj, false);
> +			return -ENOMEM;
>  		}
>  
>  		pinned = __get_user_pages_fast(obj->userptr.ptr, num_pages,
> diff --git a/include/drm/drm_mem_util.h b/include/drm/drm_mem_util.h
> index e42495ad8136..741ce75a72b4 100644
> --- a/include/drm/drm_mem_util.h
> +++ b/include/drm/drm_mem_util.h
> @@ -54,6 +54,25 @@ static __inline__ void *drm_malloc_ab(size_t nmemb, size_t size)
>  			 GFP_KERNEL | __GFP_HIGHMEM, PAGE_KERNEL);
>  }
>  
> +static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp)
> +{
> +	if (size != 0 && nmemb > SIZE_MAX / size)
> +		return NULL;
> +
> +	if (size * nmemb <= PAGE_SIZE)
> +	    return kmalloc(nmemb * size, gfp);
> +
> +	if (gfp & __GFP_RECLAIMABLE) {
> +		void *ptr = kmalloc(nmemb * size,
> +				    gfp | __GFP_NOWARN | __GFP_NORETRY);
> +		if (ptr)
> +			return ptr;
> +	}
> +
> +	return __vmalloc(size * nmemb,
> +			 gfp | __GFP_HIGHMEM, PAGE_KERNEL);
> +}
> +
>  static __inline void drm_free_large(void *ptr)
>  {
>  	kvfree(ptr);
> -- 
> 2.6.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx