* [PATCH v5 1/7] drm,i915: introduce drm_malloc_gfp()
[not found] <1456744394-29831-1-git-send-email-david.s.gordon@intel.com>
@ 2016-02-29 11:13 ` Dave Gordon
2016-02-29 11:13 ` [PATCH v5 2/7] drm: add parameter-order checking to drm memory allocators Dave Gordon
1 sibling, 0 replies; 5+ messages in thread
From: Dave Gordon @ 2016-02-29 11:13 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
From: Chris Wilson <chris@chris-wilson.co.uk>
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>
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
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 +++++++++++++++++++
4 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8fd00d2..18a5dcc 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1776,11 +1776,9 @@ static bool only_mappable_for_reloc(unsigned int flags)
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 9127f8f..2b04c3a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3414,8 +3414,9 @@ struct i915_vma *
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 7107f2f..b4be458 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -494,10 +494,7 @@ struct get_pages_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 @@ struct get_pages_work {
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 e42495a..741ce75 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);
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 2/7] drm: add parameter-order checking to drm memory allocators
[not found] <1456744394-29831-1-git-send-email-david.s.gordon@intel.com>
2016-02-29 11:13 ` [PATCH v5 1/7] drm,i915: introduce drm_malloc_gfp() Dave Gordon
@ 2016-02-29 11:13 ` Dave Gordon
2016-02-29 16:16 ` Tvrtko Ursulin
1 sibling, 1 reply; 5+ messages in thread
From: Dave Gordon @ 2016-02-29 11:13 UTC (permalink / raw)
To: intel-gfx; +Cc: Dave Gordon, dri-devel
After the recent addition of drm_malloc_gfp(), it was noticed that
some callers of these functions has swapped the parameters in the
call - it's supposed to be 'number of members' and 'sizeof(element)',
but a few callers had got the size first and the count second. This
isn't otherwise detected because they're both type 'size_t', and
the implementation at present just multiplies them anyway, so the
result is still right. But some future implementation might treat
them differently (e.g. allowing 0 elements but not zero size), so
let's add some compile-time checks and complain if the second (size)
parameter isn't a sizeof() expression, or at least a compile-time
constant.
This patch also fixes those callers where the order was wrong.
Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>Cc: dri-
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 ++++----
include/drm/drm_mem_util.h | 30 +++++++++++++++++++++++++---
3 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 1aba01a..9ae4a71 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -340,7 +340,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
*/
bos = drm_malloc_ab(args->nr_bos, sizeof(*bos));
relocs = drm_malloc_ab(args->nr_relocs, sizeof(*relocs));
- stream = drm_malloc_ab(1, args->stream_size);
+ stream = drm_malloc_ab(args->stream_size, sizeof(*stream));
cmdbuf = etnaviv_gpu_cmdbuf_new(gpu, ALIGN(args->stream_size, 8) + 8,
args->nr_bos);
if (!bos || !relocs || !stream || !cmdbuf) {
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 18a5dcc..865876d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1687,8 +1687,8 @@ static bool only_mappable_for_reloc(unsigned int flags)
}
/* Copy in the exec list from userland */
- exec_list = drm_malloc_ab(sizeof(*exec_list), args->buffer_count);
- exec2_list = drm_malloc_ab(sizeof(*exec2_list), args->buffer_count);
+ exec_list = drm_malloc_ab(args->buffer_count, sizeof(*exec_list));
+ exec2_list = drm_malloc_ab(args->buffer_count, sizeof(*exec2_list));
if (exec_list == NULL || exec2_list == NULL) {
DRM_DEBUG("Failed to allocate exec list for %d buffers\n",
args->buffer_count);
@@ -1776,8 +1776,8 @@ static bool only_mappable_for_reloc(unsigned int flags)
return -EINVAL;
}
- exec2_list = drm_malloc_gfp(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",
diff --git a/include/drm/drm_mem_util.h b/include/drm/drm_mem_util.h
index 741ce75..886ff0a 100644
--- a/include/drm/drm_mem_util.h
+++ b/include/drm/drm_mem_util.h
@@ -29,7 +29,7 @@
#include <linux/vmalloc.h>
-static __inline__ void *drm_calloc_large(size_t nmemb, size_t size)
+static __inline__ void *__drm_calloc_large(size_t nmemb, const size_t size)
{
if (size != 0 && nmemb > SIZE_MAX / size)
return NULL;
@@ -41,8 +41,15 @@ static __inline__ void *drm_calloc_large(size_t nmemb, size_t size)
GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO, PAGE_KERNEL);
}
+#define drm_calloc_large(nmemb, size) \
+({ \
+ BUILD_BUG_ON_MSG(!__builtin_constant_p(size), \
+ "Non-constant 'size' - check argument ordering?"); \
+ __drm_calloc_large(nmemb, size); \
+})
+
/* Modeled after cairo's malloc_ab, it's like calloc but without the zeroing. */
-static __inline__ void *drm_malloc_ab(size_t nmemb, size_t size)
+static __inline__ void *__drm_malloc_ab(size_t nmemb, const size_t size)
{
if (size != 0 && nmemb > SIZE_MAX / size)
return NULL;
@@ -54,8 +61,18 @@ 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)
+#define drm_malloc_ab(nmemb, size) \
+({ \
+ BUILD_BUG_ON_MSG(!__builtin_constant_p(size), \
+ "Non-constant 'size' - check argument ordering?"); \
+ __drm_malloc_ab(nmemb, size); \
+})
+
+static __inline__ void *__drm_malloc_gfp(size_t nmemb, const size_t size, gfp_t gfp)
{
+ BUILD_BUG_ON_MSG(!__builtin_constant_p(size),
+ "Non-constant 'size' - check argument ordering?");
+
if (size != 0 && nmemb > SIZE_MAX / size)
return NULL;
@@ -73,6 +90,13 @@ static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp)
gfp | __GFP_HIGHMEM, PAGE_KERNEL);
}
+#define drm_malloc_gfp(nmemb, size, gfp) \
+({ \
+ BUILD_BUG_ON_MSG(!__builtin_constant_p(size), \
+ "Non-constant 'size' - check argument ordering?"); \
+ __drm_malloc_gfp(nmemb, size, gfp); \
+})
+
static __inline void drm_free_large(void *ptr)
{
kvfree(ptr);
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 2/7] drm: add parameter-order checking to drm memory allocators
2016-02-29 11:13 ` [PATCH v5 2/7] drm: add parameter-order checking to drm memory allocators Dave Gordon
@ 2016-02-29 16:16 ` Tvrtko Ursulin
2016-02-29 16:55 ` [Intel-gfx] " Chris Wilson
2016-02-29 17:00 ` Emil Velikov
0 siblings, 2 replies; 5+ messages in thread
From: Tvrtko Ursulin @ 2016-02-29 16:16 UTC (permalink / raw)
To: Dave Gordon, intel-gfx; +Cc: dri-devel
On 29/02/16 11:13, Dave Gordon wrote:
> After the recent addition of drm_malloc_gfp(), it was noticed that
> some callers of these functions has swapped the parameters in the
> call - it's supposed to be 'number of members' and 'sizeof(element)',
> but a few callers had got the size first and the count second. This
> isn't otherwise detected because they're both type 'size_t', and
> the implementation at present just multiplies them anyway, so the
> result is still right. But some future implementation might treat
> them differently (e.g. allowing 0 elements but not zero size), so
> let's add some compile-time checks and complain if the second (size)
> parameter isn't a sizeof() expression, or at least a compile-time
> constant.
>
> This patch also fixes those callers where the order was wrong.
>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>Cc: dri-
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 ++++----
> include/drm/drm_mem_util.h | 30 +++++++++++++++++++++++++---
> 3 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 1aba01a..9ae4a71 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -340,7 +340,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
> */
> bos = drm_malloc_ab(args->nr_bos, sizeof(*bos));
> relocs = drm_malloc_ab(args->nr_relocs, sizeof(*relocs));
> - stream = drm_malloc_ab(1, args->stream_size);
> + stream = drm_malloc_ab(args->stream_size, sizeof(*stream));
I was surprised sizeof(void) == 1. On further research that seems to be
an GCC extension.
I am not sure how active projects to compile the kernel with for example
ICC are, just remember some talks about it in the past. Maybe it is even
possible? In that case it would be better to just leave "1" there to not
rely on GCC extensions.
And this use of array allocator in Etnaviv is strange since they are
allocating an unstructured buffer, but whatever, it existing and
external code. (I am not even sure would I bother touching it, since, is
the logical view to allocate *one* _buffer_ of a specified size, or
specified size of bytes make a buffer? :) )
> cmdbuf = etnaviv_gpu_cmdbuf_new(gpu, ALIGN(args->stream_size, 8) + 8,
> args->nr_bos);
> if (!bos || !relocs || !stream || !cmdbuf) {
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 18a5dcc..865876d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1687,8 +1687,8 @@ static bool only_mappable_for_reloc(unsigned int flags)
> }
>
> /* Copy in the exec list from userland */
> - exec_list = drm_malloc_ab(sizeof(*exec_list), args->buffer_count);
> - exec2_list = drm_malloc_ab(sizeof(*exec2_list), args->buffer_count);
> + exec_list = drm_malloc_ab(args->buffer_count, sizeof(*exec_list));
> + exec2_list = drm_malloc_ab(args->buffer_count, sizeof(*exec2_list));
> if (exec_list == NULL || exec2_list == NULL) {
> DRM_DEBUG("Failed to allocate exec list for %d buffers\n",
> args->buffer_count);
> @@ -1776,8 +1776,8 @@ static bool only_mappable_for_reloc(unsigned int flags)
> return -EINVAL;
> }
>
> - exec2_list = drm_malloc_gfp(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",
> diff --git a/include/drm/drm_mem_util.h b/include/drm/drm_mem_util.h
> index 741ce75..886ff0a 100644
> --- a/include/drm/drm_mem_util.h
> +++ b/include/drm/drm_mem_util.h
> @@ -29,7 +29,7 @@
>
> #include <linux/vmalloc.h>
>
> -static __inline__ void *drm_calloc_large(size_t nmemb, size_t size)
> +static __inline__ void *__drm_calloc_large(size_t nmemb, const size_t size)
> {
> if (size != 0 && nmemb > SIZE_MAX / size)
> return NULL;
> @@ -41,8 +41,15 @@ static __inline__ void *drm_calloc_large(size_t nmemb, size_t size)
> GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO, PAGE_KERNEL);
> }
>
> +#define drm_calloc_large(nmemb, size) \
> +({ \
> + BUILD_BUG_ON_MSG(!__builtin_constant_p(size), \
> + "Non-constant 'size' - check argument ordering?"); \
> + __drm_calloc_large(nmemb, size); \
> +})
> +
> /* Modeled after cairo's malloc_ab, it's like calloc but without the zeroing. */
> -static __inline__ void *drm_malloc_ab(size_t nmemb, size_t size)
> +static __inline__ void *__drm_malloc_ab(size_t nmemb, const size_t size)
> {
> if (size != 0 && nmemb > SIZE_MAX / size)
> return NULL;
> @@ -54,8 +61,18 @@ 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)
> +#define drm_malloc_ab(nmemb, size) \
> +({ \
> + BUILD_BUG_ON_MSG(!__builtin_constant_p(size), \
> + "Non-constant 'size' - check argument ordering?"); \
> + __drm_malloc_ab(nmemb, size); \
> +})
> +
> +static __inline__ void *__drm_malloc_gfp(size_t nmemb, const size_t size, gfp_t gfp)
> {
> + BUILD_BUG_ON_MSG(!__builtin_constant_p(size),
> + "Non-constant 'size' - check argument ordering?");
> +
> if (size != 0 && nmemb > SIZE_MAX / size)
> return NULL;
>
> @@ -73,6 +90,13 @@ static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp)
> gfp | __GFP_HIGHMEM, PAGE_KERNEL);
> }
>
> +#define drm_malloc_gfp(nmemb, size, gfp) \
> +({ \
> + BUILD_BUG_ON_MSG(!__builtin_constant_p(size), \
> + "Non-constant 'size' - check argument ordering?"); \
> + __drm_malloc_gfp(nmemb, size, gfp); \
> +})
> +
> static __inline void drm_free_large(void *ptr)
> {
> kvfree(ptr);
>
i915 cleanups are good but I am unsure of whether it is good to add this
constant constraints. All current code seems to use it like that, true,
but I am not sure that it should be a requirement.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Intel-gfx] [PATCH v5 2/7] drm: add parameter-order checking to drm memory allocators
2016-02-29 16:16 ` Tvrtko Ursulin
@ 2016-02-29 16:55 ` Chris Wilson
2016-02-29 17:00 ` Emil Velikov
1 sibling, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2016-02-29 16:55 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Dave Gordon, intel-gfx, dri-devel
On Mon, Feb 29, 2016 at 04:16:57PM +0000, Tvrtko Ursulin wrote:
> i915 cleanups are good but I am unsure of whether it is good to add
> this constant constraints. All current code seems to use it like
> that, true, but I am not sure that it should be a requirement.
The drm_mem_util allocators are written under that presumption in order
to allow gcc to do some constant-expression elimination - but obviously
that is not strictly required.
I like the assertions, they help describe the API and should allow us to
warn about potential bugs.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 2/7] drm: add parameter-order checking to drm memory allocators
2016-02-29 16:16 ` Tvrtko Ursulin
2016-02-29 16:55 ` [Intel-gfx] " Chris Wilson
@ 2016-02-29 17:00 ` Emil Velikov
1 sibling, 0 replies; 5+ messages in thread
From: Emil Velikov @ 2016-02-29 17:00 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-gfx@lists.freedesktop.org, ML dri-devel
On 29 February 2016 at 16:16, Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
> On 29/02/16 11:13, Dave Gordon wrote:
>>
>> After the recent addition of drm_malloc_gfp(), it was noticed that
>> some callers of these functions has swapped the parameters in the
>> call - it's supposed to be 'number of members' and 'sizeof(element)',
>> but a few callers had got the size first and the count second. This
>> isn't otherwise detected because they're both type 'size_t', and
>> the implementation at present just multiplies them anyway, so the
>> result is still right. But some future implementation might treat
>> them differently (e.g. allowing 0 elements but not zero size), so
>> let's add some compile-time checks and complain if the second (size)
>> parameter isn't a sizeof() expression, or at least a compile-time
>> constant.
>>
>> This patch also fixes those callers where the order was wrong.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>Cc: dri-
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 ++++----
>> include/drm/drm_mem_util.h | 30
>> +++++++++++++++++++++++++---
>> 3 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> index 1aba01a..9ae4a71 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> @@ -340,7 +340,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev,
>> void *data,
>> */
>> bos = drm_malloc_ab(args->nr_bos, sizeof(*bos));
>> relocs = drm_malloc_ab(args->nr_relocs, sizeof(*relocs));
>> - stream = drm_malloc_ab(1, args->stream_size);
>> + stream = drm_malloc_ab(args->stream_size, sizeof(*stream));
>
>
> I was surprised sizeof(void) == 1. On further research that seems to be an
> GCC extension.
>
Afaict pointer arithmetic (-Wpointer-arith) has/is been used in the
kernel extensively. In userspace we try to avoid it libdrm and mesa,
there might be a few cases where it's still around. No too sure about
libva{,-intel}.
> I am not sure how active projects to compile the kernel with for example ICC
> are, just remember some talks about it in the past. Maybe it is even
> possible? In that case it would be better to just leave "1" there to not
> rely on GCC extensions.
>
While I'm all for the idea, I doubt we'll get there any time soon. My
last suggestion (do not use zero sized arrays) went down in flames :-(
Regards,
Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-29 17:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1456744394-29831-1-git-send-email-david.s.gordon@intel.com>
2016-02-29 11:13 ` [PATCH v5 1/7] drm,i915: introduce drm_malloc_gfp() Dave Gordon
2016-02-29 11:13 ` [PATCH v5 2/7] drm: add parameter-order checking to drm memory allocators Dave Gordon
2016-02-29 16:16 ` Tvrtko Ursulin
2016-02-29 16:55 ` [Intel-gfx] " Chris Wilson
2016-02-29 17:00 ` Emil Velikov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).