[PATCH 1/2] drm/i915: Document our internal limit on object size

[PATCH 1/2] drm/i915: Document our internal limit on object size

[Chris Wilson]

In many places, we try to count pages using a 32 bit integer. That
implies if we are asked to create an object larger than 43bits, we will
subtly crash much later. Catch this on the boundary, and add a warning
to remind ourselves later on our exabyte systems.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 +-
 drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fe875b27a6bf..43eb1a72f19e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3107,7 +3107,7 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj);
 void i915_gem_object_init(struct drm_i915_gem_object *obj,
 			 const struct drm_i915_gem_object_ops *ops);
 struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
-						  size_t size);
+						   u64 size);
 struct drm_i915_gem_object *i915_gem_object_create_from_data(
 		struct drm_device *dev, const void *data, size_t size);
 void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fe92e28ea0a8..0d1dc04302ec 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4131,14 +4131,24 @@ static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
 	.put_pages = i915_gem_object_put_pages_gtt,
 };
 
-struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
-						  size_t size)
+struct drm_i915_gem_object *
+i915_gem_object_create(struct drm_device *dev, u64 size)
 {
 	struct drm_i915_gem_object *obj;
 	struct address_space *mapping;
 	gfp_t mask;
 	int ret;
 
+	/* There is a prevalence of the assumption that we fit the object's
+	 * page count inside a 32bit variable. Let's document this and catch
+	 * if we ever need to fix it.
+	 */
+	if (WARN_ON(size >> PAGE_SHIFT > INT_MAX))
+		return ERR_PTR(-E2BIG);
+
+	if (sizeof(size_t) < sizeof(u64) && size > INT_MAX)
+		return ERR_PTR(-E2BIG);
+
 	obj = i915_gem_object_alloc(dev);
 	if (obj == NULL)
 		return ERR_PTR(-ENOMEM);
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Limit the scattergather coalescing to 32bits

[Chris Wilson]

The scattergather list uses a 32bit size counter, we should avoid
exceeding it.

Fixes: 871dfbd67d4e ("drm/i915: Allow compaction upto SWIOTLB max segment size")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0d1dc04302ec..858569419134 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2238,7 +2238,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 
 	max_segment = swiotlb_max_size();
 	if (!max_segment)
-		max_segment = obj->base.size;
+		max_segment = rounddown(UINT_MAX, PAGE_SIZE);
 
 	st = kmalloc(sizeof(*st), GFP_KERNEL);
 	if (st == NULL)
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Document our internal limit on object size

[Chris Wilson]

On Fri, Oct 14, 2016 at 04:18:10PM +0100, Chris Wilson wrote:
> In many places, we try to count pages using a 32 bit integer. That
> implies if we are asked to create an object larger than 43bits, we will
> subtly crash much later. Catch this on the boundary, and add a warning
> to remind ourselves later on our exabyte systems.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  2 +-
>  drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++++++--
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fe875b27a6bf..43eb1a72f19e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3107,7 +3107,7 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj);
>  void i915_gem_object_init(struct drm_i915_gem_object *obj,
>  			 const struct drm_i915_gem_object_ops *ops);
>  struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
> -						  size_t size);
> +						   u64 size);
>  struct drm_i915_gem_object *i915_gem_object_create_from_data(
>  		struct drm_device *dev, const void *data, size_t size);
>  void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fe92e28ea0a8..0d1dc04302ec 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4131,14 +4131,24 @@ static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
>  	.put_pages = i915_gem_object_put_pages_gtt,
>  };
>  
> -struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
> -						  size_t size)
> +struct drm_i915_gem_object *
> +i915_gem_object_create(struct drm_device *dev, u64 size)
>  {
>  	struct drm_i915_gem_object *obj;
>  	struct address_space *mapping;
>  	gfp_t mask;
>  	int ret;
>  
> +	/* There is a prevalence of the assumption that we fit the object's
> +	 * page count inside a 32bit variable. Let's document this and catch
> +	 * if we ever need to fix it.
> +	 */
> +	if (WARN_ON(size >> PAGE_SHIFT > INT_MAX))
> +		return ERR_PTR(-E2BIG);
> +
> +	if (sizeof(size_t) < sizeof(u64) && size > INT_MAX)

What I was looking for here was SIZE_T_MAX. Any ideas?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Limit the scattergather coalescing to 32bits

[Tvrtko Ursulin]

On 14/10/2016 16:18, Chris Wilson wrote:
> The scattergather list uses a 32bit size counter, we should avoid
> exceeding it.
>
> Fixes: 871dfbd67d4e ("drm/i915: Allow compaction upto SWIOTLB max segment size")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 0d1dc04302ec..858569419134 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2238,7 +2238,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
>   
>   	max_segment = swiotlb_max_size();
>   	if (!max_segment)
> -		max_segment = obj->base.size;
> +		max_segment = rounddown(UINT_MAX, PAGE_SIZE);
>   
>   	st = kmalloc(sizeof(*st), GFP_KERNEL);
>   	if (st == NULL)

I suppose it could be possible to get more that 4GiB of of contiguous pages.

sg_alloc_table_from_pages seems broken as well, for this, and more so, 
for using longs and stuffing them into ints.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Limit the scattergather coalescing to 32bits

[Chris Wilson]

On Fri, Oct 14, 2016 at 04:38:35PM +0100, Tvrtko Ursulin wrote:
> 
> On 14/10/2016 16:18, Chris Wilson wrote:
> >The scattergather list uses a 32bit size counter, we should avoid
> >exceeding it.
> >
> >Fixes: 871dfbd67d4e ("drm/i915: Allow compaction upto SWIOTLB max segment size")
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >---
> >  drivers/gpu/drm/i915/i915_gem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index 0d1dc04302ec..858569419134 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -2238,7 +2238,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
> >  	max_segment = swiotlb_max_size();
> >  	if (!max_segment)
> >-		max_segment = obj->base.size;
> >+		max_segment = rounddown(UINT_MAX, PAGE_SIZE);
> >  	st = kmalloc(sizeof(*st), GFP_KERNEL);
> >  	if (st == NULL)
> 
> I suppose it could be possible to get more that 4GiB of of contiguous pages.
> 
> sg_alloc_table_from_pages seems broken as well, for this, and more
> so, for using longs and stuffing them into ints.

Noticed I was using unsigned long max_segment. Any complaints if I
change that to unsigned int as well?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Document our internal limit on object size

[Tvrtko Ursulin]

On 14/10/2016 16:24, Chris Wilson wrote:
> On Fri, Oct 14, 2016 at 04:18:10PM +0100, Chris Wilson wrote:
>> In many places, we try to count pages using a 32 bit integer. That
>> implies if we are asked to create an object larger than 43bits, we will
>> subtly crash much later. Catch this on the boundary, and add a warning
>> to remind ourselves later on our exabyte systems.
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h |  2 +-
>>   drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++++++--
>>   2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index fe875b27a6bf..43eb1a72f19e 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3107,7 +3107,7 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj);
>>   void i915_gem_object_init(struct drm_i915_gem_object *obj,
>>   			 const struct drm_i915_gem_object_ops *ops);
>>   struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
>> -						  size_t size);
>> +						   u64 size);
>>   struct drm_i915_gem_object *i915_gem_object_create_from_data(
>>   		struct drm_device *dev, const void *data, size_t size);
>>   void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index fe92e28ea0a8..0d1dc04302ec 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -4131,14 +4131,24 @@ static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
>>   	.put_pages = i915_gem_object_put_pages_gtt,
>>   };
>>   
>> -struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
>> -						  size_t size)
>> +struct drm_i915_gem_object *
>> +i915_gem_object_create(struct drm_device *dev, u64 size)
>>   {
>>   	struct drm_i915_gem_object *obj;
>>   	struct address_space *mapping;
>>   	gfp_t mask;
>>   	int ret;
>>   
>> +	/* There is a prevalence of the assumption that we fit the object's
>> +	 * page count inside a 32bit variable. Let's document this and catch
>> +	 * if we ever need to fix it.
>> +	 */
>> +	if (WARN_ON(size >> PAGE_SHIFT > INT_MAX))
>> +		return ERR_PTR(-E2BIG);
>> +
>> +	if (sizeof(size_t) < sizeof(u64) && size > INT_MAX)
> What I was looking for here was SIZE_T_MAX. Any ideas?

Would (1ULL << (sizeof(size_t) * BITS_PER_BYTE)) - 1 work?

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Limit the scattergather coalescing to 32bits

[Tvrtko Ursulin]

On 14/10/2016 16:43, Chris Wilson wrote:
> On Fri, Oct 14, 2016 at 04:38:35PM +0100, Tvrtko Ursulin wrote:
>> On 14/10/2016 16:18, Chris Wilson wrote:
>>> The scattergather list uses a 32bit size counter, we should avoid
>>> exceeding it.
>>>
>>> Fixes: 871dfbd67d4e ("drm/i915: Allow compaction upto SWIOTLB max segment size")
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>   drivers/gpu/drm/i915/i915_gem.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>>> index 0d1dc04302ec..858569419134 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -2238,7 +2238,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
>>>   	max_segment = swiotlb_max_size();
>>>   	if (!max_segment)
>>> -		max_segment = obj->base.size;
>>> +		max_segment = rounddown(UINT_MAX, PAGE_SIZE);
>>>   	st = kmalloc(sizeof(*st), GFP_KERNEL);
>>>   	if (st == NULL)
>> I suppose it could be possible to get more that 4GiB of of contiguous pages.
>>
>> sg_alloc_table_from_pages seems broken as well, for this, and more
>> so, for using longs and stuffing them into ints.
> Noticed I was using unsigned long max_segment. Any complaints if I
> change that to unsigned int as well?

Not from me, should be changed!

Regards,

Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Document our internal limit on object size

[Tvrtko Ursulin]

On 14/10/2016 16:18, Chris Wilson wrote:
> In many places, we try to count pages using a 32 bit integer. That
> implies if we are asked to create an object larger than 43bits, we will
> subtly crash much later. Catch this on the boundary, and add a warning
> to remind ourselves later on our exabyte systems.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.h |  2 +-
>   drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++++++--
>   2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index fe875b27a6bf..43eb1a72f19e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3107,7 +3107,7 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj);
>   void i915_gem_object_init(struct drm_i915_gem_object *obj,
>   			 const struct drm_i915_gem_object_ops *ops);
>   struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
> -						  size_t size);
> +						   u64 size);
>   struct drm_i915_gem_object *i915_gem_object_create_from_data(
>   		struct drm_device *dev, const void *data, size_t size);
>   void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fe92e28ea0a8..0d1dc04302ec 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4131,14 +4131,24 @@ static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
>   	.put_pages = i915_gem_object_put_pages_gtt,
>   };
>   
> -struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
> -						  size_t size)
> +struct drm_i915_gem_object *
> +i915_gem_object_create(struct drm_device *dev, u64 size)
>   {
>   	struct drm_i915_gem_object *obj;
>   	struct address_space *mapping;
>   	gfp_t mask;
>   	int ret;
>   
> +	/* There is a prevalence of the assumption that we fit the object's
> +	 * page count inside a 32bit variable. Let's document this and catch
> +	 * if we ever need to fix it.
> +	 */
> +	if (WARN_ON(size >> PAGE_SHIFT > INT_MAX))
> +		return ERR_PTR(-E2BIG);
> +
> +	if (sizeof(size_t) < sizeof(u64) && size > INT_MAX)
> +		return ERR_PTR(-E2BIG);
> +

Shouldn't it be UINT_MAX in both cases?

We could try to future proof more maybe like 
sizeof(typeof(obj->base.size)), is typeof can be used like that? 
Something similar for sg API if possible. But then again, it could be 
better future proofing to be hardcoded like you wrote it. Yes I think so.

Regards,

Tvrtko



_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Document our internal limit on object size

[Chris Wilson]

On Fri, Oct 14, 2016 at 04:49:33PM +0100, Tvrtko Ursulin wrote:
> 
> On 14/10/2016 16:18, Chris Wilson wrote:
> >In many places, we try to count pages using a 32 bit integer. That
> >implies if we are asked to create an object larger than 43bits, we will
> >subtly crash much later. Catch this on the boundary, and add a warning
> >to remind ourselves later on our exabyte systems.
> >
> >Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >---
> >  drivers/gpu/drm/i915/i915_drv.h |  2 +-
> >  drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++++++--
> >  2 files changed, 13 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index fe875b27a6bf..43eb1a72f19e 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -3107,7 +3107,7 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj);
> >  void i915_gem_object_init(struct drm_i915_gem_object *obj,
> >  			 const struct drm_i915_gem_object_ops *ops);
> >  struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
> >-						  size_t size);
> >+						   u64 size);
> >  struct drm_i915_gem_object *i915_gem_object_create_from_data(
> >  		struct drm_device *dev, const void *data, size_t size);
> >  void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file);
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index fe92e28ea0a8..0d1dc04302ec 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -4131,14 +4131,24 @@ static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
> >  	.put_pages = i915_gem_object_put_pages_gtt,
> >  };
> >-struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
> >-						  size_t size)
> >+struct drm_i915_gem_object *
> >+i915_gem_object_create(struct drm_device *dev, u64 size)
> >  {
> >  	struct drm_i915_gem_object *obj;
> >  	struct address_space *mapping;
> >  	gfp_t mask;
> >  	int ret;
> >+	/* There is a prevalence of the assumption that we fit the object's
> >+	 * page count inside a 32bit variable. Let's document this and catch
> >+	 * if we ever need to fix it.
> >+	 */
> >+	if (WARN_ON(size >> PAGE_SHIFT > INT_MAX))
> >+		return ERR_PTR(-E2BIG);
> >+
> >+	if (sizeof(size_t) < sizeof(u64) && size > INT_MAX)
> >+		return ERR_PTR(-E2BIG);
> >+
> 
> Shouldn't it be UINT_MAX in both cases?

I've spotted a few "int page_count = obj->size / PAGE_SIZE;" so we can't
trust ourselves at all!

> We could try to future proof more maybe like
> sizeof(typeof(obj->base.size)), is typeof can be used like that?
> Something similar for sg API if possible. But then again, it could
> be better future proofing to be hardcoded like you wrote it. Yes I
> think so.

I was just about to write it as obj->base.size, Let's compare!

#define overflows_type(x, T) \
	(sizeof(x) < sizeof(T) && (x) > 1 << (sizeof(T) * BITS_PER_BYTE))

  if (overflows_type(size, obj->base.size)

or

  if (overflows_type(size, size_t))

I think obj->base.size looks better from the self-documentation standpoint.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915: Document our internal limit on object size

[Chris Wilson]

On Fri, Oct 14, 2016 at 05:03:39PM +0100, Chris Wilson wrote:
> > We could try to future proof more maybe like
> > sizeof(typeof(obj->base.size)), is typeof can be used like that?
> > Something similar for sg API if possible. But then again, it could
> > be better future proofing to be hardcoded like you wrote it. Yes I
> > think so.
> 
> I was just about to write it as obj->base.size, Let's compare!
> 
> #define overflows_type(x, T) \
> 	(sizeof(x) < sizeof(T) && (x) > 1 << (sizeof(T) * BITS_PER_BYTE))

	(sizeof(x) > sizeof(T) && ((x) >= 1 << (sizeof(T) * BITS_PER_BYTE)))
> 
>   if (overflows_type(size, obj->base.size)
> 
> or
> 
>   if (overflows_type(size, size_t))
> 
> I think obj->base.size looks better from the self-documentation standpoint.
> -Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: Document our internal limit on object size

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Document our internal limit on object size
URL   : https://patchwork.freedesktop.org/series/13797/
State : warning

== Summary ==

Series 13797v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/13797/revisions/1/mbox/

Test drv_module_reload_basic:
                pass       -> SKIP       (fi-skl-6770hq)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                dmesg-warn -> PASS       (fi-skl-6770hq)
                dmesg-warn -> PASS       (fi-skl-6700k)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-a-frame-sequence:
                dmesg-warn -> PASS       (fi-ilk-650)
        Subgroup read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-ilk-650)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900)
        Subgroup suspend-read-crc-pipe-c:
                incomplete -> PASS       (fi-skl-6700k)
Test vgem_basic:
        Subgroup unload:
                skip       -> PASS       (fi-skl-6770hq)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:214  dwarn:0   dfail:0   fail:1   skip:31 
fi-byt-n2820     total:246  pass:210  dwarn:0   dfail:0   fail:1   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:184  dwarn:0   dfail:0   fail:2   skip:60 
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:223  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:246  pass:229  dwarn:1   dfail:0   fail:1   skip:15 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2725/

38ecbe9082e477953fe165265c76e6c6061aeaf6 drm-intel-nightly: 2016y-10m-14d-16h-23m-48s UTC integration manifest
edfea81 drm/i915: Limit the scattergather coalescing to 32bits
fee9f2e drm/i915: Document our internal limit on object size

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx