Quoting Jani Nikula (2018-03-19 22:21:31) > On Mon, 19 Mar 2018, Matthew Auld <matthew.william.auld@xxxxxxxxx> wrote: > > On 19 March 2018 at 18:17, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
