This is the alternative approach for solving a deadlock situation with
array-fences.
Currently with fence-array, we have a potential deadlock situation. If we
fence_add_callback() on an array-fence, the array-fence's lock is aquired
first, and in it's ->enable_signaling() callback, it will install cb's on
it's array-member fences, so the array-member's lock is acquired second.
But in the signal path, the array-member's lock is acquired first, and the
array-fence's lock acquired second.
To solve that, punt signaling the array-fence to a worker.
lockdep splat:
======================================================
[ INFO: possible circular locking dependency detected ]
4.7.0-rc7+ #489 Not tainted
-------------------------------------------------------
surfaceflinger/2034 is trying to acquire lock:
(&(&array->lock)->rlock){......}, at: [<ffff00000858cddc>]
fence_signal+0x5c/0xf8
but task is already holding lock:
(&(&obj->child_list_lock)->rlock){......}, at: [<ffff0000085902f8>]
sw_sync_ioctl+0x228/0x3b0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&(&obj->child_list_lock)->rlock){......}:
[<ffff000008108924>] __lock_acquire+0x173c/0x18d8
[<ffff000008108e0c>] lock_acquire+0x4c/0x68
[<ffff000008ac6a6c>] _raw_spin_lock_irqsave+0x54/0x70
[<ffff00000858d05c>] fence_add_callback+0x3c/0x100
[<ffff00000858f100>] fence_array_enable_signaling+0x80/0x170
[<ffff00000858d0d8>] fence_add_callback+0xb8/0x100
[<ffff00000858f504>] sync_file_poll+0xd4/0xf0
[<ffff0000081fd3a0>] do_sys_poll+0x220/0x438
[<ffff0000081fd8d0>] SyS_ppoll+0x1b0/0x1d8
[<ffff000008084f30>] el0_svc_naked+0x24/0x28
-> #0 (&(&array->lock)->rlock){......}:
[<ffff000008104768>] print_circular_bug+0x80/0x2e0
[<ffff0000081089ac>] __lock_acquire+0x17c4/0x18d8
[<ffff000008108e0c>] lock_acquire+0x4c/0x68
[<ffff000008ac6a6c>] _raw_spin_lock_irqsave+0x54/0x70
[<ffff00000858cddc>] fence_signal+0x5c/0xf8
[<ffff00000858f268>] fence_array_cb_func+0x78/0x88
[<ffff00000858cb28>] fence_signal_locked+0x80/0xe0
[<ffff0000085903c8>] sw_sync_ioctl+0x2f8/0x3b0
[<ffff0000081faf6c>] do_vfs_ioctl+0xa4/0x790
[<ffff0000081fb6e4>] SyS_ioctl+0x8c/0xa0
[<ffff000008084f30>] el0_svc_naked+0x24/0x28
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&(&obj->child_list_lock)->rlock);
lock(&(&array->lock)->rlock);
lock(&(&obj->child_list_lock)->rlock);
lock(&(&array->lock)->rlock);
*** DEADLOCK ***
1 lock held by surfaceflinger/2034:
#0: (&(&obj->child_list_lock)->rlock){......}, at: [<ffff0000085902f8>]
sw_sync_ioctl+0x228/0x3b0
---
drivers/dma-buf/fence-array.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/fence-array.c b/drivers/dma-buf/fence-array.c
index f1989fc..446dfc2 100644
--- a/drivers/dma-buf/fence-array.c
+++ b/drivers/dma-buf/fence-array.c
@@ -19,8 +19,11 @@
#include <linux/export.h>
#include <linux/slab.h>
+#include <linux/module.h>
#include <linux/fence-array.h>
+static struct workqueue_struct *fence_array_wq;
+
static void fence_array_cb_func(struct fence *f, struct fence_cb *cb);
static const char *fence_array_get_driver_name(struct fence *fence)
@@ -33,6 +36,13 @@ static const char *fence_array_get_timeline_name(struct
fence *fence)
return "unbound";
}
+static void signal_worker(struct work_struct *w)
+{
+ struct fence_array *array =
+ container_of(w, struct fence_array, signal_worker);
+ fence_signal(&array->base);
+}
+
static void fence_array_cb_func(struct fence *f, struct fence_cb *cb)
{
struct fence_array_cb *array_cb =
@@ -40,7 +50,7 @@ static void fence_array_cb_func(struct fence *f, struct
fence_cb *cb)
struct fence_array *array = array_cb->array;
if (atomic_dec_and_test(&array->num_pending))
- fence_signal(&array->base);
+ queue_work(fence_array_wq, &array->signal_worker);
fence_put(&array->base);
}
@@ -140,6 +150,25 @@ struct fence_array *fence_array_create(int num_fences,
struct fence **fences,
atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences);
array->fences = fences;
+ INIT_WORK(&array->signal_worker, signal_worker);
+
return array;
}
EXPORT_SYMBOL(fence_array_create);
+
+static int __init fence_array_init(void)
+{
+ fence_array_wq = alloc_ordered_workqueue("fence-array", 0);
+ if (!fence_array_wq)
+ return -ENOMEM;
+ return 0;
+}
+
+static void __exit fence_array_exit(void)
+{
+ flush_workqueue(fence_array_wq);
+ destroy_workqueue(fence_array_wq);
+}
+
+module_init(fence_array_init);
+module_exit(fence_array_exit);
--
2.7.4
