diff mbox

drm/i915: Always evaluate GEM_BUG_ON expression

Message ID 1478860845-29554-1-git-send-email-joonas.lahtinen@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joonas Lahtinen Nov. 11, 2016, 10:40 a.m. UTC
Evaluate GEM_BUG_ON expression similar to actual BUG_ON even if
DEBUG_GEM is disabled.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Chris Wilson Nov. 11, 2016, 11:05 a.m. UTC | #1
On Fri, Nov 11, 2016 at 12:40:45PM +0200, Joonas Lahtinen wrote:
> Evaluate GEM_BUG_ON expression similar to actual BUG_ON even if
> DEBUG_GEM is disabled.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index 51ec793..d141676 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -28,7 +28,7 @@
>  #ifdef CONFIG_DRM_I915_DEBUG_GEM
>  #define GEM_BUG_ON(expr) BUG_ON(expr)
>  #else
> -#define GEM_BUG_ON(expr) do { } while (0)
> +#define GEM_BUG_ON(expr) do { if (expr) {} } while(0)
>  #endif

Mentioned on irc, that I'm not that keen on having the side-effects
around. Suggestion was while (0) do { if (expr) {} } while (0)

But spot the dev who touched gem code without enabling the debug :-p
-Chris
Tvrtko Ursulin Nov. 11, 2016, 12:09 p.m. UTC | #2
On 11/11/2016 11:05, Chris Wilson wrote:
> On Fri, Nov 11, 2016 at 12:40:45PM +0200, Joonas Lahtinen wrote:
>> Evaluate GEM_BUG_ON expression similar to actual BUG_ON even if
>> DEBUG_GEM is disabled.
>>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_gem.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
>> index 51ec793..d141676 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.h
>> +++ b/drivers/gpu/drm/i915/i915_gem.h
>> @@ -28,7 +28,7 @@
>>  #ifdef CONFIG_DRM_I915_DEBUG_GEM
>>  #define GEM_BUG_ON(expr) BUG_ON(expr)
>>  #else
>> -#define GEM_BUG_ON(expr) do { } while (0)
>> +#define GEM_BUG_ON(expr) do { if (expr) {} } while(0)
>>  #endif
>
> Mentioned on irc, that I'm not that keen on having the side-effects
> around. Suggestion was while (0) do { if (expr) {} } while (0)

Sounds good to me.

Regards,

Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
index 51ec793..d141676 100644
--- a/drivers/gpu/drm/i915/i915_gem.h
+++ b/drivers/gpu/drm/i915/i915_gem.h
@@ -28,7 +28,7 @@ 
 #ifdef CONFIG_DRM_I915_DEBUG_GEM
 #define GEM_BUG_ON(expr) BUG_ON(expr)
 #else
-#define GEM_BUG_ON(expr) do { } while (0)
+#define GEM_BUG_ON(expr) do { if (expr) {} } while(0)
 #endif
 
 #define I915_NUM_ENGINES 5