Re: [PATCH 08/17] drm/i915: Drop spinlocks around adding to the client request list

Chris Wilson
2016-09-02T13:38:51Z

On Fri, Sep 02, 2016 at 02:20:16PM +0100, John Harrison wrote:
> On 02/09/2016 12:02, Chris Wilson wrote:
> >On Fri, Sep 02, 2016 at 11:59:12AM +0100, Chris Wilson wrote:
> >>On Fri, Sep 02, 2016 at 11:30:18AM +0100, John Harrison wrote:
> >>>>+ if (request->file_priv) {
> >>>Why check for request->file_priv again? The block above will exit if
> >>>it is null. There surely can't be a race with remove_from_client
> >>>being called concurrently with add_to_client? Especially as
> >>>add_to_client no longer takes the spin_lock anyway.
> >>We can however allow i915_gem_release() to be called concurrently. It
> >>doesn't require struct_mutex.
> >That's not correct. setting file_priv = NULL is serialised by the
> >struct_mutex, not the file_priv->mm.lock. That spin_lock there is
> >misleading.
> >-Chris
> >
>
> The spinlock is to protect the list rather than the request, I thought.

Hmm. We hold a ref to drm_file / drm_i915_file_private and the write
side to the list is serialised by struct_mutex, and we have RCU on the
list itself, so we could just swap the spinlock for an rcu list walk.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre