From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?ISO-8859-2?Q?=A3ukasz_Kury=B3o?= <lukasz.kurylo@gmail.com>
Subject: Re: [PATCH 2/4] drm/i915: don't trylock in the gpu
	reset code
Date: Wed, 27 Jun 2012 00:08:33 +0200
Message-ID: <CA+jNtFqOrdES3kQfQANqjzAysB7Smp2RW_9EdvfPiDDPvqG4CA@mail.gmail.com>
References: <1340744933-11835-1-git-send-email-daniel.vetter@ffwll.ch>
	<1340744933-11835-2-git-send-email-daniel.vetter@ffwll.ch>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from mail-vc0-f177.google.com (mail-vc0-f177.google.com
	[209.85.220.177])
	by gabe.freedesktop.org (Postfix) with ESMTP id A1B139E814
	for <intel-gfx@lists.freedesktop.org>;
	Tue, 26 Jun 2012 15:08:34 -0700 (PDT)
Received: by vcbf13 with SMTP id f13so317661vcb.36
	for <intel-gfx@lists.freedesktop.org>;
	Tue, 26 Jun 2012 15:08:34 -0700 (PDT)
In-Reply-To: <1340744933-11835-2-git-send-email-daniel.vetter@ffwll.ch>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
List-Id: intel-gfx@lists.freedesktop.org

>
> - =A0 =A0 =A0 if (!mutex_trylock(&dev->struct_mutex))
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;
> + =A0 =A0 =A0 mutex_lock(&dev->struct_mutex);
>
> =A0 =A0 =A0 =A0i915_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.