On Mon, Apr 11, 2016 at 11:12:48AM +0300, Joonas Lahtinen wrote: > On la, 2016-04-09 at 12:19 +0100, Chris Wilson wrote: > > Currently there is a #define to enable extra BUG_ON for debugging > > requests and associated activities. I want to expand its use to cover > > all of GEM internals (so that we can saturate the code with asserts). > > We can add a Kconfig option to make it easier to enable - with the usual > > caveats of not enabling unless explicitly requested. > > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > > A few comments below, still: > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > > --- > > drivers/gpu/drm/i915/Kconfig.debug | 12 ++++++++++++ > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/i915_gem.c | 12 +++++------- > > drivers/gpu/drm/i915/i915_gem.h | 34 ++++++++++++++++++++++++++++++++++ > > 4 files changed, 52 insertions(+), 7 deletions(-) > > create mode 100644 drivers/gpu/drm/i915/i915_gem.h > > > > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug > > index 61485c8ba3a8..8f404103341d 100644 > > --- a/drivers/gpu/drm/i915/Kconfig.debug > > +++ b/drivers/gpu/drm/i915/Kconfig.debug > > @@ -27,3 +27,15 @@ config DRM_I915_DEBUG > > > > If in doubt, say "N". > > > > +config DRM_I915_DEBUG_GEM > > + bool "Insert extra checks into the GEM internals" > > + default n > > + depends on DRM_I915_WERROR > > Not sure if this needs to be explicit. I was using it to hide the option unless in "developer" mode. When we hit these BUG_ON we can often limp on without any noticeable effect (yes, they are precursors of infinite loops, memory corruption and whatnot but not always ;) > > +#ifdef CONFIG_DRM_I915_DEBUG_GEM > > +#define GEM_BUG_ON(expr) BUG_ON(expr) > > +#else > > +#define GEM_BUG_ON(expr) > > There seems to be no consistent way of doing this within kernel so > guess this is fin, s/(expr)//g would do, too. The choice is whether we want side-effects from expr to be evaluated. I voted for no, as that allows us to write complicated expr such that only the BUG_ON() need to jump through 29 pointers and function calls to sanity check calling conditions. And partly why I want Werror enabled so that I have to fix any bugs in the BUG_ON() for the code to compile! -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
