[Intel-gfx] [PATCH 6/8] drm/i915: add struct_mutex WARNs to i915_gem_stolen.c

[Chris Wilson]

On Tue, Jun 30, 2015 at 11:26:11AM -0300, Paulo Zanoni wrote:
> 2015-06-30 11:15 GMT-03:00 Chris Wilson <chris at chris-wilson.co.uk>:
> > On Tue, Jun 30, 2015 at 10:53:10AM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>
> >> Let's make sure the future Paulos don't forget that we need
> >> struct_mutex when touching dev_priv->mm.stolen.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_gem_stolen.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> index 793bcba..cac1bce 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> @@ -160,6 +160,8 @@ static int find_compression_threshold(struct drm_device *dev,
> >>       int compression_threshold = 1;
> >>       int ret;
> >>
> >> +     WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> >
> > I'm not a huge fan of vague mutex warnings that don't even check the owner.
> > I'm espcially not a fan of adding a WARN and not handling the error.
> 
> But then, what exactly is your proposal? What would you like to see here?
> 
> We can discard this patch if you want. But I hope you're not
> advocating for lockdep_assert_held(), because if I switch to lockdep,
> then Daniel is going to deny it again. Also, this type of WARN_ON is a
> common pattern on our codebase...

I'm just trying to convince Daniel that blindly using this pattern is
the wrong approach and encouraging a proliferation of unhandled WARN_ON
doesn't improve driver robustness.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre