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 Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Acked-by: Dave Airlie airlied@redhat.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 be4cf13343d5..985f067c1f0e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2421,9 +2421,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 0ee61fd014df..6ee4f00f620c 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1783,11 +1783,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(args->buffer_count, + sizeof(*exec2_list), + 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 ae9cb2735767..18f2bd7caad5 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3413,8 +3413,9 @@ intel_rotate_fb_obj_pages(struct intel_rotation_info *rot_info, 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 291a9393493d..67883ebf9504 100644 --- a/drivers/gpu/drm/i915/i915_gem_userptr.c +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c @@ -494,10 +494,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;
@@ -634,14 +631,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);
On Tue, Apr 05, 2016 at 01:57:36PM +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 Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Acked-by: Dave Airlie airlied@redhat.com
+static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp) +{
- if (size != 0 && nmemb > SIZE_MAX / size)
return NULL;
I know Dave G. has some fancy code to detect when the size parameter is not constant, but one thing I noticed was that gcc would uninline this function and we would lose the constant folding. Is there anything we can do to convince gcc to avoid a div here (other than pure macro)? -Chris
On 05/04/16 14:05, Chris Wilson wrote:
On Tue, Apr 05, 2016 at 01:57:36PM +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 Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Acked-by: Dave Airlie airlied@redhat.com
+static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp) +{
- if (size != 0 && nmemb > SIZE_MAX / size)
return NULL;
I know Dave G. has some fancy code to detect when the size parameter is not constant, but one thing I noticed was that gcc would uninline this function and we would lose the constant folding. Is there anything we can do to convince gcc to avoid a div here (other than pure macro)?
Don't know, apart from maybe _always_inline if it is not considered too big.
But I wanted to ask, why it is interesting to allow size == 0 ? Why not:
if (size == 0 || nmemb > SIZE_MAX / size) return NULL;
?
Regards,
Tvrtko
On Wed, Apr 06, 2016 at 10:40:19AM +0100, Tvrtko Ursulin wrote:
On 05/04/16 14:05, Chris Wilson wrote:
On Tue, Apr 05, 2016 at 01:57:36PM +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 Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Acked-by: Dave Airlie airlied@redhat.com
+static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp) +{
- if (size != 0 && nmemb > SIZE_MAX / size)
return NULL;
I know Dave G. has some fancy code to detect when the size parameter is not constant, but one thing I noticed was that gcc would uninline this function and we would lose the constant folding. Is there anything we can do to convince gcc to avoid a div here (other than pure macro)?
Don't know, apart from maybe _always_inline if it is not considered too big.
But I wanted to ask, why it is interesting to allow size == 0 ? Why not:
if (size == 0 || nmemb > SIZE_MAX / size) return NULL;
?
Cargo-culting. I guess the only thought was to avoid the div-by-zero and to fallthrough to returning kmalloc(0) for equivalent behaviour. -Chris
dri-devel@lists.freedesktop.org