On Thu, 17 Mar 2011 13:54:45 -0300, Herton Ronaldo Krzesinski <herton.krzesinski at canonical.com> wrote: > On Thu, Mar 17, 2011 at 01:46:34PM +0000, Chris Wilson wrote: > > 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(). > > Ok. I just still have one doubt though, if in i915_add_request > file/file_priv is NULL, wouldn't be possible to have an oops also in > i915_gem_release without the check? As in this case, > request->client_list wouldn't have mm.request_list added to it, and if > an error occurs and i915_reset is called, which ends up calling > i915_gem_release, we would try to do a list_del on request->client_list > without items.
If the file_priv is NULL, then the request is not added to the client mm.request_list and so it is not seen during i915_gem_release. The list is file_priv->mm.request_list, the nodes within that are request->client_list. > If the check really isn't needed in i915_gem_release, then please > consider this patch: Done, thanks, -Chris -- Chris Wilson, Intel Open Source Technology Centre
