[Intel-gfx] [PATCH] drm/i915: make GEM_WARN_ON less terrible

[Joonas Lahtinen]

Quoting Jani Nikula (2018-03-19 22:21:31)
> On Mon, 19 Mar 2018, Matthew Auld <matthew.william.auld at gmail.com> wrote:
> > On 19 March 2018 at 18:17, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >> Quoting Matthew Auld (2018-03-19 18:08:54)
> >>> GEM_WARN_ON() was originally intended to be used only as:
> >>>
> >>>    if (GEM_WARN_ON(expr))
> >>>         ...
> >>>
> >>> but it just so happens to also work as simply:
> >>>
> >>>    GEM_WARN_ON(expr);
> >>>
> >>> since it just wraps WARN_ON, which is a little misleading since for
> >>> !DRM_I915_DEBUG_GEM builds the second case will actually break the
> >>> build. Given that there are some patches floating around which seem to
> >>> miss this, it probably makes sense to just make it work for both cases.
> >>
> >> That really was quite intentional. The only time to use GEM_WARN_ON() is
> >> inside an if, otherwise what's the point?
> >
> > Why wouldn't we want it to behave like WARN_ON? That seems to be what
> > people expect, since it does wrap WARN_ON, and we don't always use
> > WARN_ON in an if...
> 
> Looking at this, I'm more baffled by GEM_WARN_ON() evaluating to expr on
> CONFIG_DRM_I915_DEBUG_GEM=y and 0 otherwise. That's the seriously
> misleading part here.
> 
> Are you sure all those if (GEM_WARN_ON(expr)) are to be ignored? I'm no
> expert on gem code, but I could be easily persuaded to believe not.
> 
> 
> BR,
> Jani.
> 
> PS. On the original question, if you design GEM_WARN_ON() to be used
> within if conditions only, I think you better squeeze in an inline
> function with __must_check.

Did somebody write a patch for this?

Regards, Joonas

> 
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx