Re: [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Tvrtko Ursulin (2019-10-04 10:15:20)
> 
> On 03/10/2019 22:01, Chris Wilson wrote:
> > A few callers need to serialise the destruction of their drm_mm_node and
> > ensure it is removed from the drm_mm before freeing. However, to be
> > completely sure that any access from another thread is complete before
> > we free the struct, we require the RELEASE semantics of
> > clear_bit_unlock().
> > 
> > This allows the conditional locking such as
> > 
> > Thread A                              Thread B
> >      mutex_lock(mm_lock);                if (drm_mm_node_allocated(node)) {
> >      drm_mm_node_remove(node);               mutex_lock(mm_lock);
> >      mutex_unlock(mm_lock);                  drm_mm_node_remove(node);
> >                                              mutex_unlock(mm_lock);
> >                                           }
> >                                           kfree(node);
> 
> My understanding is that release semantics on node allocated mean 1 -> 0 
> transition is guaranteed to be seen only when thread A 
> drm_mm_node_remove is fully done with the removal. But if it is in the 
> middle of removal, node is still seen as allocated outside and thread B 
> can enter the if-body, wait for the lock, and then drm_mm_node_remove 
> will attempt a double removal. So I think another drm_mm_node_allocated 
> under the lock is needed.

Yes. Check after the lock is indeed required in this scenario. And
drm_mm_node_remove() insists the caller doesn't try a double remove.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx