From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: Oops at i915_gem_retire_requests_ring Date: Thu, 17 Mar 2011 13:46:34 +0000 Message-ID: References: <20110317123906.GA1857@herton-IdeaPad-Y430> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20110317123906.GA1857@herton-IdeaPad-Y430> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Herton Ronaldo Krzesinski , dri-devel@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Thu, 17 Mar 2011 09:39:07 -0300, Herton Ronaldo Krzesinski wrote: > I don't know if it's the most correct fix, but perhaps the simple fix > is needed in the code. It's against latest Linus tree. We may have an > already removed client_list, or we didn't add any item to client_list > (file_priv NULL in i915_add_request) > > Signed-off-by: Herton Ronaldo Krzesinski > --- > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 36e66cc..6077c0d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1749,8 +1749,10 @@ i915_gem_request_remove_from_client(struct drm_i915_gem_request *request) > return; > > spin_lock(&file_priv->mm.lock); > - list_del(&request->client_list); > - request->file_priv = NULL; > + if (request->file_priv) { > + list_del(&request->client_list); > + request->file_priv = NULL; > + } > spin_unlock(&file_priv->mm.lock); > } This is the single chunk required. I had thought that the actual insertion/deletion was serialised under the struct mutex and the intention of the spinlock was to protect the unlocked list traversal during throttling. However, I missed that i915_gem_release() is also called without struct mutex and so we do need the double check for i915_gem_request_remove_from_client(). -Chris -- Chris Wilson, Intel Open Source Technology Centre