| Message ID | 20180404111904.2768-1-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Matthew Auld (2018-04-04 14:19:04) > GEM_WARN_ON is intended to be used as: > > if (GEM_WARN_ON(expr)) > ... > > However this isn't clear since we just wrap WARN_ON, therefore throw a > warning if the result is ignored. For !DRM_I915_DEBUG_GEM builds this > should already be the case. > > Suggested-by: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> I think this would be a worthwhile improvement. Regards, Joonas > --- > drivers/gpu/drm/i915/i915_gem.h | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h > index 8922344fc21b..760671119130 100644 > --- a/drivers/gpu/drm/i915/i915_gem.h > +++ b/drivers/gpu/drm/i915/i915_gem.h > @@ -36,7 +36,14 @@ > BUG(); \ > } \ > } while(0) > -#define GEM_WARN_ON(expr) WARN_ON(expr) > + > +static __always_inline bool __must_check __gem_warn_on(bool v) > +{ > + return WARN_ON(v); > +} > + > +/* To be used as: if (GEM_WARN_ON(expr)) */ > +#define GEM_WARN_ON(expr) __gem_warn_on(expr) > > #define GEM_DEBUG_DECL(var) var > #define GEM_DEBUG_EXEC(expr) expr > -- > 2.14.3 >
Quoting Joonas Lahtinen (2018-04-06 12:29:01) > Quoting Matthew Auld (2018-04-04 14:19:04) > > GEM_WARN_ON is intended to be used as: > > > > if (GEM_WARN_ON(expr)) > > ... > > > > However this isn't clear since we just wrap WARN_ON, therefore throw a > > warning if the result is ignored. For !DRM_I915_DEBUG_GEM builds this > > should already be the case. > > > > Suggested-by: Jani Nikula <jani.nikula@linux.intel.com> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > I think this would be a worthwhile improvement. At the very least may I beg for the GEM_TRACE output and #expr? #expr kind of defeats the use of an inline... Or at least requires more macro magic. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h index 8922344fc21b..760671119130 100644 --- a/drivers/gpu/drm/i915/i915_gem.h +++ b/drivers/gpu/drm/i915/i915_gem.h @@ -36,7 +36,14 @@ BUG(); \ } \ } while(0) -#define GEM_WARN_ON(expr) WARN_ON(expr) + +static __always_inline bool __must_check __gem_warn_on(bool v) +{ + return WARN_ON(v); +} + +/* To be used as: if (GEM_WARN_ON(expr)) */ +#define GEM_WARN_ON(expr) __gem_warn_on(expr) #define GEM_DEBUG_DECL(var) var #define GEM_DEBUG_EXEC(expr) expr
GEM_WARN_ON is intended to be used as: if (GEM_WARN_ON(expr)) ... However this isn't clear since we just wrap WARN_ON, therefore throw a warning if the result is ignored. For !DRM_I915_DEBUG_GEM builds this should already be the case. Suggested-by: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)