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@xxxxxxxx> 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
