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

[Jesse Barnes]

On 06/30/2015 07:36 AM, Chris Wilson wrote:
> 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.

I think they serve as useful documentation at the very least, whether in
lockdep form, WARN form, or BUG form.  It's not really something we can
recover from either (maybe returning early before touching data?), so...