* [PATCH 1/2] drm/i915: introduce GEM_WARN_ON
@ 2016-12-02 13:23 Matthew Auld
2016-12-02 13:23 ` [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Matthew Auld @ 2016-12-02 13:23 UTC (permalink / raw)
To: intel-gfx
In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
will enable us to freely add warnings which our CI will hopefully catch
but without fear of impacting production machines.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_gem.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 51ec793f2e20..04c777e6d0bf 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -27,8 +27,10 @@
#ifdef CONFIG_DRM_I915_DEBUG_GEM
#define GEM_BUG_ON(expr) BUG_ON(expr)
+#define GEM_WARN_ON(expr) WARN_ON(expr)
#else
#define GEM_BUG_ON(expr) do { } while (0)
+#define GEM_WARN_ON(expr) do { } while (0)
#endif
#define I915_NUM_ENGINES 5
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind
2016-12-02 13:23 [PATCH 1/2] drm/i915: introduce GEM_WARN_ON Matthew Auld
@ 2016-12-02 13:23 ` Matthew Auld
2016-12-03 17:53 ` kbuild test robot
2016-12-03 18:07 ` kbuild test robot
2016-12-02 13:34 ` [PATCH 1/2] drm/i915: introduce GEM_WARN_ON Ville Syrjälä
` (2 subsequent siblings)
3 siblings, 2 replies; 12+ messages in thread
From: Matthew Auld @ 2016-12-02 13:23 UTC (permalink / raw)
To: intel-gfx
If we move the sanity checking from gen8_alloc_va_range_3lvl and
gen6_alloc_va_range into i915_vma_bind, we will increase our coverage to
now both callbacks. We also convert each WARN_ON over to a GEM_WARN_ON.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 12 ------------
drivers/gpu/drm/i915/i915_vma.c | 6 ++++++
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 02fb063302bf..e0746eb5dc54 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1304,15 +1304,6 @@ static int gen8_alloc_va_range_3lvl(struct i915_address_space *vm,
uint32_t pdpes = I915_PDPES_PER_PDP(dev_priv);
int ret;
- /* Wrap is never okay since we can only represent 48b, and we don't
- * actually use the other side of the canonical address space.
- */
- if (WARN_ON(start + length < start))
- return -ENODEV;
-
- if (WARN_ON(start + length > vm->total))
- return -ENODEV;
-
ret = alloc_gen8_temp_bitmaps(&new_page_dirs, &new_page_tables, pdpes);
if (ret)
return ret;
@@ -1930,9 +1921,6 @@ static int gen6_alloc_va_range(struct i915_address_space *vm,
uint32_t pde;
int ret;
- if (WARN_ON(start_in + length_in > ppgtt->base.total))
- return -ENODEV;
-
start = start_save = start_in;
length = length_save = length_in;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 4c91a68ecb6d..2c494191d7b3 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -176,6 +176,12 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
if (bind_flags == 0)
return 0;
+ if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
+ return -ENODEV;
+
+ if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
+ return -ENODEV;
+
if (vma_flags == 0 && vma->vm->allocate_va_range) {
trace_i915_va_alloc(vma);
ret = vma->vm->allocate_va_range(vma->vm,
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON
2016-12-02 13:23 [PATCH 1/2] drm/i915: introduce GEM_WARN_ON Matthew Auld
2016-12-02 13:23 ` [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
@ 2016-12-02 13:34 ` Ville Syrjälä
2016-12-02 13:54 ` Chris Wilson
2016-12-02 14:15 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
3 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2016-12-02 13:34 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote:
> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
> will enable us to freely add warnings which our CI will hopefully catch
> but without fear of impacting production machines.
Impacting performance?
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 51ec793f2e20..04c777e6d0bf 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -27,8 +27,10 @@
>
> #ifdef CONFIG_DRM_I915_DEBUG_GEM
> #define GEM_BUG_ON(expr) BUG_ON(expr)
> +#define GEM_WARN_ON(expr) WARN_ON(expr)
> #else
> #define GEM_BUG_ON(expr) do { } while (0)
> +#define GEM_WARN_ON(expr) do { } while (0)
> #endif
>
> #define I915_NUM_ENGINES 5
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON
2016-12-02 13:23 [PATCH 1/2] drm/i915: introduce GEM_WARN_ON Matthew Auld
2016-12-02 13:23 ` [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
2016-12-02 13:34 ` [PATCH 1/2] drm/i915: introduce GEM_WARN_ON Ville Syrjälä
@ 2016-12-02 13:54 ` Chris Wilson
2016-12-02 15:11 ` Matthew Auld
2016-12-02 14:15 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
3 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2016-12-02 13:54 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote:
> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
> will enable us to freely add warnings which our CI will hopefully catch
> but without fear of impacting production machines.
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 51ec793f2e20..04c777e6d0bf 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -27,8 +27,10 @@
>
> #ifdef CONFIG_DRM_I915_DEBUG_GEM
> #define GEM_BUG_ON(expr) BUG_ON(expr)
> +#define GEM_WARN_ON(expr) WARN_ON(expr)
> #else
> #define GEM_BUG_ON(expr) do { } while (0)
> +#define GEM_WARN_ON(expr) do { } while (0)
#define GEM_WARN_ON 0
i.e. so that if (GEM_WARN_ON(insane_condition()) compiles out correctly
without DEBUG_GEM enabled.
-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
^ permalink raw reply [flat|nested] 12+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/2] drm/i915: introduce GEM_WARN_ON
2016-12-02 13:23 [PATCH 1/2] drm/i915: introduce GEM_WARN_ON Matthew Auld
` (2 preceding siblings ...)
2016-12-02 13:54 ` Chris Wilson
@ 2016-12-02 14:15 ` Patchwork
3 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2016-12-02 14:15 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: introduce GEM_WARN_ON
URL : https://patchwork.freedesktop.org/series/16278/
State : warning
== Summary ==
Series 16278v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/16278/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (fi-skl-6770hq)
fi-bdw-5557u total:245 pass:230 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:245 pass:205 dwarn:0 dfail:0 fail:0 skip:40
fi-bxt-t5700 total:245 pass:217 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-j1900 total:245 pass:217 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-n2820 total:245 pass:213 dwarn:0 dfail:0 fail:0 skip:32
fi-hsw-4770 total:245 pass:225 dwarn:0 dfail:0 fail:0 skip:20
fi-hsw-4770r total:245 pass:225 dwarn:0 dfail:0 fail:0 skip:20
fi-ilk-650 total:245 pass:192 dwarn:0 dfail:0 fail:0 skip:53
fi-ivb-3520m total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22
fi-ivb-3770 total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22
fi-kbl-7500u total:245 pass:223 dwarn:0 dfail:0 fail:0 skip:22
fi-skl-6260u total:245 pass:231 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:245 pass:224 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6700k total:245 pass:223 dwarn:1 dfail:0 fail:0 skip:21
fi-skl-6770hq total:245 pass:230 dwarn:1 dfail:0 fail:0 skip:14
fi-snb-2520m total:245 pass:213 dwarn:0 dfail:0 fail:0 skip:32
fi-snb-2600 total:245 pass:212 dwarn:0 dfail:0 fail:0 skip:33
273202788eba907049316b3ccc2f4d1fd30aff1d drm-tip: 2016y-12m-02d-11h-27m-42s UTC integration manifest
05e8d2b drm/i915: move vma sanity checking into i915_vma_bind
91fae78 drm/i915: introduce GEM_WARN_ON
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3175/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON
2016-12-02 13:54 ` Chris Wilson
@ 2016-12-02 15:11 ` Matthew Auld
2016-12-02 15:25 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Matthew Auld @ 2016-12-02 15:11 UTC (permalink / raw)
To: Chris Wilson, Matthew Auld, Intel Graphics Development
On 2 December 2016 at 13:54, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote:
>> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
>> will enable us to freely add warnings which our CI will hopefully catch
>> but without fear of impacting production machines.
>>
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_gem.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
>> index 51ec793f2e20..04c777e6d0bf 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.h
>> +++ b/drivers/gpu/drm/i915/i915_gem.h
>> @@ -27,8 +27,10 @@
>>
>> #ifdef CONFIG_DRM_I915_DEBUG_GEM
>> #define GEM_BUG_ON(expr) BUG_ON(expr)
>> +#define GEM_WARN_ON(expr) WARN_ON(expr)
>> #else
>> #define GEM_BUG_ON(expr) do { } while (0)
>> +#define GEM_WARN_ON(expr) do { } while (0)
>
> #define GEM_WARN_ON 0
Oops. As an alternative, would you be offended with something like:
#define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
Then the compiler will still check the validity of the expression, but
will still compile everything out, and then we don't accidentally
break the build when compiling without DEBUG_GEM ?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON
2016-12-02 15:11 ` Matthew Auld
@ 2016-12-02 15:25 ` Chris Wilson
0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2016-12-02 15:25 UTC (permalink / raw)
To: Matthew Auld; +Cc: Intel Graphics Development, Matthew Auld
On Fri, Dec 02, 2016 at 03:11:05PM +0000, Matthew Auld wrote:
> On 2 December 2016 at 13:54, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Fri, Dec 02, 2016 at 01:23:13PM +0000, Matthew Auld wrote:
> >> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, this
> >> will enable us to freely add warnings which our CI will hopefully catch
> >> but without fear of impacting production machines.
> >>
> >> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_gem.h | 2 ++
> >> 1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> >> index 51ec793f2e20..04c777e6d0bf 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem.h
> >> +++ b/drivers/gpu/drm/i915/i915_gem.h
> >> @@ -27,8 +27,10 @@
> >>
> >> #ifdef CONFIG_DRM_I915_DEBUG_GEM
> >> #define GEM_BUG_ON(expr) BUG_ON(expr)
> >> +#define GEM_WARN_ON(expr) WARN_ON(expr)
> >> #else
> >> #define GEM_BUG_ON(expr) do { } while (0)
> >> +#define GEM_WARN_ON(expr) do { } while (0)
> >
> > #define GEM_WARN_ON 0
> Oops. As an alternative, would you be offended with something like:
>
> #define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
>
> Then the compiler will still check the validity of the expression, but
> will still compile everything out, and then we don't accidentally
> break the build when compiling without DEBUG_GEM ?
Ooh, new shiny. Uses sizeof()! Can you prepare a patch to fixup
GEM_BUG_ON 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind
2016-12-02 13:23 ` [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
@ 2016-12-03 17:53 ` kbuild test robot
2016-12-03 18:07 ` kbuild test robot
1 sibling, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-12-03 17:53 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 10800 bytes --]
Hi Matthew,
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on next-20161202]
[cannot apply to v4.9-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Matthew-Auld/drm-i915-introduce-GEM_WARN_ON/20161203-231346
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-s3-12040051 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/io-mapping.h:21,
from drivers/gpu/drm/i915/i915_vma.h:28,
from drivers/gpu/drm/i915/i915_vma.c:25:
drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind':
drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
#define GEM_WARN_ON(expr) do { } while (0)
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/gpu/drm/i915/i915_vma.c:179:2: note: in expansion of macro 'if'
if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
^~
drivers/gpu/drm/i915/i915_vma.c:179:6: note: in expansion of macro 'GEM_WARN_ON'
if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
^~~~~~~~~~~
drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
#define GEM_WARN_ON(expr) do { } while (0)
^
include/linux/compiler.h:149:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/gpu/drm/i915/i915_vma.c:179:2: note: in expansion of macro 'if'
if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
^~
drivers/gpu/drm/i915/i915_vma.c:179:6: note: in expansion of macro 'GEM_WARN_ON'
if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
^~~~~~~~~~~
drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
#define GEM_WARN_ON(expr) do { } while (0)
^
include/linux/compiler.h:160:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> drivers/gpu/drm/i915/i915_vma.c:179:2: note: in expansion of macro 'if'
if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
^~
drivers/gpu/drm/i915/i915_vma.c:179:6: note: in expansion of macro 'GEM_WARN_ON'
if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
^~~~~~~~~~~
drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
#define GEM_WARN_ON(expr) do { } while (0)
^
include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
drivers/gpu/drm/i915/i915_vma.c:182:2: note: in expansion of macro 'if'
if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
^~
drivers/gpu/drm/i915/i915_vma.c:182:6: note: in expansion of macro 'GEM_WARN_ON'
if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
^~~~~~~~~~~
drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
#define GEM_WARN_ON(expr) do { } while (0)
^
include/linux/compiler.h:149:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
drivers/gpu/drm/i915/i915_vma.c:182:2: note: in expansion of macro 'if'
if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
^~
drivers/gpu/drm/i915/i915_vma.c:182:6: note: in expansion of macro 'GEM_WARN_ON'
if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
^~~~~~~~~~~
drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
#define GEM_WARN_ON(expr) do { } while (0)
^
include/linux/compiler.h:160:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
drivers/gpu/drm/i915/i915_vma.c:182:2: note: in expansion of macro 'if'
if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
^~
drivers/gpu/drm/i915/i915_vma.c:182:6: note: in expansion of macro 'GEM_WARN_ON'
if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
^~~~~~~~~~~
vim +/if +179 drivers/gpu/drm/i915/i915_vma.c
19 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
20 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
21 * IN THE SOFTWARE.
22 *
23 */
24
> 25 #include "i915_vma.h"
26
27 #include "i915_drv.h"
28 #include "intel_ringbuffer.h"
29 #include "intel_frontbuffer.h"
30
31 #include <drm/drm_gem.h>
32
33 static void
34 i915_vma_retire(struct i915_gem_active *active,
35 struct drm_i915_gem_request *rq)
36 {
37 const unsigned int idx = rq->engine->id;
38 struct i915_vma *vma =
39 container_of(active, struct i915_vma, last_read[idx]);
40 struct drm_i915_gem_object *obj = vma->obj;
41
42 GEM_BUG_ON(!i915_vma_has_active_engine(vma, idx));
43
44 i915_vma_clear_active(vma, idx);
45 if (i915_vma_is_active(vma))
46 return;
47
48 list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
49 if (unlikely(i915_vma_is_closed(vma) && !i915_vma_is_pinned(vma)))
50 WARN_ON(i915_vma_unbind(vma));
51
52 GEM_BUG_ON(!i915_gem_object_is_active(obj));
53 if (--obj->active_count)
54 return;
55
56 /* Bump our place on the bound list to keep it roughly in LRU order
57 * so that we don't steal from recently used but inactive objects
58 * (unless we are forced to ofc!)
59 */
60 if (obj->bind_count)
61 list_move_tail(&obj->global_link, &rq->i915->mm.bound_list);
62
63 obj->mm.dirty = true; /* be paranoid */
64
65 if (i915_gem_object_has_active_reference(obj)) {
66 i915_gem_object_clear_active_reference(obj);
67 i915_gem_object_put(obj);
68 }
69 }
70
71 static struct i915_vma *
72 __i915_vma_create(struct drm_i915_gem_object *obj,
73 struct i915_address_space *vm,
74 const struct i915_ggtt_view *view)
75 {
76 struct i915_vma *vma;
77 struct rb_node *rb, **p;
78 int i;
79
80 GEM_BUG_ON(vm->closed);
81
82 vma = kmem_cache_zalloc(to_i915(obj->base.dev)->vmas, GFP_KERNEL);
83 if (vma == NULL)
84 return ERR_PTR(-ENOMEM);
85
86 INIT_LIST_HEAD(&vma->exec_list);
87 for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
88 init_request_active(&vma->last_read[i], i915_vma_retire);
89 init_request_active(&vma->last_fence, NULL);
90 list_add(&vma->vm_link, &vm->unbound_list);
91 vma->vm = vm;
92 vma->obj = obj;
93 vma->size = obj->base.size;
94
95 if (view) {
96 vma->ggtt_view = *view;
97 if (view->type == I915_GGTT_VIEW_PARTIAL) {
98 vma->size = view->params.partial.size;
99 vma->size <<= PAGE_SHIFT;
100 } else if (view->type == I915_GGTT_VIEW_ROTATED) {
101 vma->size =
102 intel_rotation_info_size(&view->params.rotated);
103 vma->size <<= PAGE_SHIFT;
104 }
105 }
106
107 if (i915_is_ggtt(vm)) {
108 vma->flags |= I915_VMA_GGTT;
109 list_add(&vma->obj_link, &obj->vma_list);
110 } else {
111 i915_ppgtt_get(i915_vm_to_ppgtt(vm));
112 list_add_tail(&vma->obj_link, &obj->vma_list);
113 }
114
115 rb = NULL;
116 p = &obj->vma_tree.rb_node;
117 while (*p) {
118 struct i915_vma *pos;
119
120 rb = *p;
121 pos = rb_entry(rb, struct i915_vma, obj_node);
122 if (i915_vma_compare(pos, vm, view) < 0)
123 p = &rb->rb_right;
124 else
125 p = &rb->rb_left;
126 }
127 rb_link_node(&vma->obj_node, rb, p);
128 rb_insert_color(&vma->obj_node, &obj->vma_tree);
129
130 return vma;
131 }
132
133 struct i915_vma *
134 i915_vma_create(struct drm_i915_gem_object *obj,
135 struct i915_address_space *vm,
136 const struct i915_ggtt_view *view)
137 {
138 lockdep_assert_held(&obj->base.dev->struct_mutex);
139 GEM_BUG_ON(view && !i915_is_ggtt(vm));
140 GEM_BUG_ON(i915_gem_obj_to_vma(obj, vm, view));
141
142 return __i915_vma_create(obj, vm, view);
143 }
144
145 /**
146 * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space.
147 * @vma: VMA to map
148 * @cache_level: mapping cache level
149 * @flags: flags like global or local mapping
150 *
151 * DMA addresses are taken from the scatter-gather table of this object (or of
152 * this VMA in case of non-default GGTT views) and PTE entries set up.
153 * Note that DMA addresses are also the only part of the SG table we care about.
154 */
155 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
156 u32 flags)
157 {
158 u32 bind_flags;
159 u32 vma_flags;
160 int ret;
161
162 if (WARN_ON(flags == 0))
163 return -EINVAL;
164
165 bind_flags = 0;
166 if (flags & PIN_GLOBAL)
167 bind_flags |= I915_VMA_GLOBAL_BIND;
168 if (flags & PIN_USER)
169 bind_flags |= I915_VMA_LOCAL_BIND;
170
171 vma_flags = vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND);
172 if (flags & PIN_UPDATE)
173 bind_flags |= vma_flags;
174 else
175 bind_flags &= ~vma_flags;
176 if (bind_flags == 0)
177 return 0;
178
> 179 if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
180 return -ENODEV;
181
182 if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26044 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind
2016-12-02 13:23 ` [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
2016-12-03 17:53 ` kbuild test robot
@ 2016-12-03 18:07 ` kbuild test robot
1 sibling, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2016-12-03 18:07 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 7394 bytes --]
Hi Matthew,
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20161202]
[cannot apply to v4.9-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Matthew-Auld/drm-i915-introduce-GEM_WARN_ON/20161203-231346
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/i915/i915_gem_request.h:30:0,
from drivers/gpu/drm/i915/i915_gem_timeline.h:30,
from drivers/gpu/drm/i915/i915_gem_gtt.h:40,
from drivers/gpu/drm/i915/i915_vma.h:32,
from drivers/gpu/drm/i915/i915_vma.c:25:
drivers/gpu/drm/i915/i915_vma.c: In function 'i915_vma_bind':
>> drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
#define GEM_WARN_ON(expr) do { } while (0)
^
>> drivers/gpu/drm/i915/i915_vma.c:179:6: note: in expansion of macro 'GEM_WARN_ON'
if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
^~~~~~~~~~~
>> drivers/gpu/drm/i915/i915_gem.h:33:27: error: expected expression before 'do'
#define GEM_WARN_ON(expr) do { } while (0)
^
drivers/gpu/drm/i915/i915_vma.c:182:6: note: in expansion of macro 'GEM_WARN_ON'
if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
^~~~~~~~~~~
vim +/GEM_WARN_ON +179 drivers/gpu/drm/i915/i915_vma.c
19 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
20 * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
21 * IN THE SOFTWARE.
22 *
23 */
24
> 25 #include "i915_vma.h"
26
27 #include "i915_drv.h"
28 #include "intel_ringbuffer.h"
29 #include "intel_frontbuffer.h"
30
31 #include <drm/drm_gem.h>
32
33 static void
34 i915_vma_retire(struct i915_gem_active *active,
35 struct drm_i915_gem_request *rq)
36 {
37 const unsigned int idx = rq->engine->id;
38 struct i915_vma *vma =
39 container_of(active, struct i915_vma, last_read[idx]);
40 struct drm_i915_gem_object *obj = vma->obj;
41
42 GEM_BUG_ON(!i915_vma_has_active_engine(vma, idx));
43
44 i915_vma_clear_active(vma, idx);
45 if (i915_vma_is_active(vma))
46 return;
47
48 list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
49 if (unlikely(i915_vma_is_closed(vma) && !i915_vma_is_pinned(vma)))
50 WARN_ON(i915_vma_unbind(vma));
51
52 GEM_BUG_ON(!i915_gem_object_is_active(obj));
53 if (--obj->active_count)
54 return;
55
56 /* Bump our place on the bound list to keep it roughly in LRU order
57 * so that we don't steal from recently used but inactive objects
58 * (unless we are forced to ofc!)
59 */
60 if (obj->bind_count)
61 list_move_tail(&obj->global_link, &rq->i915->mm.bound_list);
62
63 obj->mm.dirty = true; /* be paranoid */
64
65 if (i915_gem_object_has_active_reference(obj)) {
66 i915_gem_object_clear_active_reference(obj);
67 i915_gem_object_put(obj);
68 }
69 }
70
71 static struct i915_vma *
72 __i915_vma_create(struct drm_i915_gem_object *obj,
73 struct i915_address_space *vm,
74 const struct i915_ggtt_view *view)
75 {
76 struct i915_vma *vma;
77 struct rb_node *rb, **p;
78 int i;
79
80 GEM_BUG_ON(vm->closed);
81
82 vma = kmem_cache_zalloc(to_i915(obj->base.dev)->vmas, GFP_KERNEL);
83 if (vma == NULL)
84 return ERR_PTR(-ENOMEM);
85
86 INIT_LIST_HEAD(&vma->exec_list);
87 for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
88 init_request_active(&vma->last_read[i], i915_vma_retire);
89 init_request_active(&vma->last_fence, NULL);
90 list_add(&vma->vm_link, &vm->unbound_list);
91 vma->vm = vm;
92 vma->obj = obj;
93 vma->size = obj->base.size;
94
95 if (view) {
96 vma->ggtt_view = *view;
97 if (view->type == I915_GGTT_VIEW_PARTIAL) {
98 vma->size = view->params.partial.size;
99 vma->size <<= PAGE_SHIFT;
100 } else if (view->type == I915_GGTT_VIEW_ROTATED) {
101 vma->size =
102 intel_rotation_info_size(&view->params.rotated);
103 vma->size <<= PAGE_SHIFT;
104 }
105 }
106
107 if (i915_is_ggtt(vm)) {
108 vma->flags |= I915_VMA_GGTT;
109 list_add(&vma->obj_link, &obj->vma_list);
110 } else {
111 i915_ppgtt_get(i915_vm_to_ppgtt(vm));
112 list_add_tail(&vma->obj_link, &obj->vma_list);
113 }
114
115 rb = NULL;
116 p = &obj->vma_tree.rb_node;
117 while (*p) {
118 struct i915_vma *pos;
119
120 rb = *p;
121 pos = rb_entry(rb, struct i915_vma, obj_node);
122 if (i915_vma_compare(pos, vm, view) < 0)
123 p = &rb->rb_right;
124 else
125 p = &rb->rb_left;
126 }
127 rb_link_node(&vma->obj_node, rb, p);
128 rb_insert_color(&vma->obj_node, &obj->vma_tree);
129
130 return vma;
131 }
132
133 struct i915_vma *
134 i915_vma_create(struct drm_i915_gem_object *obj,
135 struct i915_address_space *vm,
136 const struct i915_ggtt_view *view)
137 {
138 lockdep_assert_held(&obj->base.dev->struct_mutex);
139 GEM_BUG_ON(view && !i915_is_ggtt(vm));
140 GEM_BUG_ON(i915_gem_obj_to_vma(obj, vm, view));
141
142 return __i915_vma_create(obj, vm, view);
143 }
144
145 /**
146 * i915_vma_bind - Sets up PTEs for an VMA in it's corresponding address space.
147 * @vma: VMA to map
148 * @cache_level: mapping cache level
149 * @flags: flags like global or local mapping
150 *
151 * DMA addresses are taken from the scatter-gather table of this object (or of
152 * this VMA in case of non-default GGTT views) and PTE entries set up.
153 * Note that DMA addresses are also the only part of the SG table we care about.
154 */
155 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,
156 u32 flags)
157 {
158 u32 bind_flags;
159 u32 vma_flags;
160 int ret;
161
162 if (WARN_ON(flags == 0))
163 return -EINVAL;
164
165 bind_flags = 0;
166 if (flags & PIN_GLOBAL)
167 bind_flags |= I915_VMA_GLOBAL_BIND;
168 if (flags & PIN_USER)
169 bind_flags |= I915_VMA_LOCAL_BIND;
170
171 vma_flags = vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND);
172 if (flags & PIN_UPDATE)
173 bind_flags |= vma_flags;
174 else
175 bind_flags &= ~vma_flags;
176 if (bind_flags == 0)
177 return 0;
178
> 179 if (GEM_WARN_ON(vma->node.start + vma->node.size < vma->node.start))
180 return -ENODEV;
181
182 if (GEM_WARN_ON(vma->node.start + vma->node.size > vma->vm->total))
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56838 bytes --]
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] drm/i915: introduce GEM_WARN_ON
@ 2016-12-13 16:00 Matthew Auld
2016-12-13 16:25 ` Joonas Lahtinen
0 siblings, 1 reply; 12+ messages in thread
From: Matthew Auld @ 2016-12-13 16:00 UTC (permalink / raw)
To: intel-gfx
In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, with the
simple goal of expressing warnings which are truly insane, and so are
only really useful for CI where we have some abusive tests.
v2:
- use BUILD_BUG_ON_INVALID for !DEBUG_GEM
- clarify commit message
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
---
drivers/gpu/drm/i915/i915_gem.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 8801a14a78a2..a585d47c420a 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -27,8 +27,10 @@
#ifdef CONFIG_DRM_I915_DEBUG_GEM
#define GEM_BUG_ON(expr) BUG_ON(expr)
+#define GEM_WARN_ON(expr) WARN_ON(expr)
#else
#define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
+#define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
#endif
#define I915_NUM_ENGINES 5
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON
2016-12-13 16:00 [PATCH 1/2] " Matthew Auld
@ 2016-12-13 16:25 ` Joonas Lahtinen
2016-12-13 16:52 ` Matthew Auld
0 siblings, 1 reply; 12+ messages in thread
From: Joonas Lahtinen @ 2016-12-13 16:25 UTC (permalink / raw)
To: Matthew Auld, intel-gfx
On ti, 2016-12-13 at 16:00 +0000, Matthew Auld wrote:
> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, with the
> simple goal of expressing warnings which are truly insane, and so are
> only really useful for CI where we have some abusive tests.
>
> v2:
> - use BUILD_BUG_ON_INVALID for !DEBUG_GEM
> - clarify commit message
>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
<SNIP>
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -27,8 +27,10 @@
>
> #ifdef CONFIG_DRM_I915_DEBUG_GEM
> #define GEM_BUG_ON(expr) BUG_ON(expr)
> +#define GEM_WARN_ON(expr) WARN_ON(expr)
> #else
> #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
> +#define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
+#define GEM_WARN_ON(expr) GEM_BUG_ON(expr)
Should be enough in the #else branch just like in linux/mmdebug.h
Regards, joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: introduce GEM_WARN_ON
2016-12-13 16:25 ` Joonas Lahtinen
@ 2016-12-13 16:52 ` Matthew Auld
0 siblings, 0 replies; 12+ messages in thread
From: Matthew Auld @ 2016-12-13 16:52 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: Intel Graphics Development, Matthew Auld
On 13 December 2016 at 16:25, Joonas Lahtinen
<joonas.lahtinen@linux.intel.com> wrote:
> On ti, 2016-12-13 at 16:00 +0000, Matthew Auld wrote:
>> In a similar spirit to GEM_BUG_ON we now also have GEM_WARN_ON, with the
>> simple goal of expressing warnings which are truly insane, and so are
>> only really useful for CI where we have some abusive tests.
>>
>> v2:
>> - use BUILD_BUG_ON_INVALID for !DEBUG_GEM
>> - clarify commit message
>>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>
> <SNIP>
>
>> +++ b/drivers/gpu/drm/i915/i915_gem.h
>> @@ -27,8 +27,10 @@
>>
>> #ifdef CONFIG_DRM_I915_DEBUG_GEM
>> #define GEM_BUG_ON(expr) BUG_ON(expr)
>> +#define GEM_WARN_ON(expr) WARN_ON(expr)
>> #else
>> #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
>> +#define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
>
> +#define GEM_WARN_ON(expr) GEM_BUG_ON(expr)
>
> Should be enough in the #else branch just like in linux/mmdebug.h
BUILD_BUG_INVALID(expr) will do a (void) cast to discard the result of
the sizeof operation, but then we don't end up ignoring said result if
we use it in the context of an if condition, which will make the
compiler very unhappy.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-12-13 16:52 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-02 13:23 [PATCH 1/2] drm/i915: introduce GEM_WARN_ON Matthew Auld
2016-12-02 13:23 ` [PATCH 2/2] drm/i915: move vma sanity checking into i915_vma_bind Matthew Auld
2016-12-03 17:53 ` kbuild test robot
2016-12-03 18:07 ` kbuild test robot
2016-12-02 13:34 ` [PATCH 1/2] drm/i915: introduce GEM_WARN_ON Ville Syrjälä
2016-12-02 13:54 ` Chris Wilson
2016-12-02 15:11 ` Matthew Auld
2016-12-02 15:25 ` Chris Wilson
2016-12-02 14:15 ` ✗ Fi.CI.BAT: warning for series starting with [1/2] " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2016-12-13 16:00 [PATCH 1/2] " Matthew Auld
2016-12-13 16:25 ` Joonas Lahtinen
2016-12-13 16:52 ` Matthew Auld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox