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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx