On Tue, Jul 16, 2013 at 09:12:09AM +0200, Daniel Vetter wrote:
The gem flink name holds a reference onto the object itself, and this self-reference would prevent an flink'ed object from every being freed. To break that loop we remove the flink name when the last userspace handle disappears, i.e. when obj->handle_count reaches 0.
Now in gem_open we drop the dev->object_name_lock between the flink name lookup and actually adding the handle. This means a concurrent gem_close of the last handle could result in the flink name getting reaped right inbetween, i.e.
Thread 1 Thread 2 gem_open gem_close
flink -> obj lookup handle_count drops to 0 remove flink name create_handle handle_count++
If someone now flinks this object again, we'll get a new flink name.
We can close this race by removing the lock dropping and making the entire lookup+handle_create sequence atomic. Unfortunately to still be able to share the handle_create logic this requires a handle_create_tail function which drops the lock - we can't hold the object_name_lock while calling into a driver's ->gem_open callback.
Note that for flink fixing this race isn't really important, since racing gem_open against gem_close is clearly a userspace bug. And no matter how the race ends, we won't leak any references.
But with dma-buf where the userspace dma-buf fd itself is refcounted this is a valid sequence and hence we should fix it. Therefore this patch here is just a warm-up exercise (and for consistency between flink buffer sharing and dma-buf buffer sharing with self-imports).
Also note that this extension of the critical section in gem_open protected by dev->object_name_lock only works because it's now a mutex: A spinlock would conflict with the potential memory allocation in idr_preload().
Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch
The gem_flink_race/flink_name subtest exercise this race here. Like explained in the commit message strictly we don't need to care here, but having the same logic for dma-buf and flink names is imo worthwile.
But now I've spotted a little bug in the the dma-buf import code, so gotta write some dma-buf multithreaded testcases for that race now ;-) -Daniel