Re: [PATCH 2/4] drm/i915: don't trylock in the gpu reset code

Łukasz Kuryło
2012-06-26T22:08:34Z

>
> -       if (!mutex_trylock(&dev->struct_mutex))
> -               return -EBUSY;
> +       mutex_lock(&dev->struct_mutex);
>
>        i915_gem_reset(dev);
>

But ... the original code:

Correct me if I'm wrong.
In every manual I've found mutex_trylock(...) returns 0 on success.
So

if(!mutex_trylock(&dev->struct_mutex))
return -EBUSY;

would actually execute when mutex was acquired as requested.
We then return without releasing it and possibly end up with deadlock.

On the other hand if mutex was already locked, by other or maybe even
the same thread, mutex_trylock returns error (some nonzero value),
nonzero means true ... if(!true) -> if(false) means do not execute the
"if" code. In this case in spite of not getting the mutex we would
proceed.

You've written:
"Simply failing to reset the gpu because someone else might still hold
the mutex isn't a great idea - I see reliable silent reset failures.
And gpu reset simply needs to be reliable and Just Work."

I believe that was not the case with the original code.
>From my understanding this procedure failed to reset gpu when the
mutex was unlocked (locking it and bailing out) and tried to reset it
if the mutex could not be acquired.