From patchwork Thu Nov 3 20:31:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rob Clark X-Patchwork-Id: 9411427 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A41C66022E for ; Thu, 3 Nov 2016 20:32:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 973342AF2F for ; Thu, 3 Nov 2016 20:32:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 880132AF39; Thu, 3 Nov 2016 20:32:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.6 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6C3A72AF2F for ; Thu, 3 Nov 2016 20:32:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 769F46E80E; Thu, 3 Nov 2016 20:32:19 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-qt0-x241.google.com (mail-qt0-x241.google.com [IPv6:2607:f8b0:400d:c0d::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id 849CE6E80E for ; Thu, 3 Nov 2016 20:32:17 +0000 (UTC) Received: by mail-qt0-x241.google.com with SMTP id l20so2114503qta.1 for ; Thu, 03 Nov 2016 13:32:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=f8VdMF9drbTP4owyGqx4nJ9btSFaMVXAZawUVkk2NwU=; b=hKdxa07yDGBbN2rlR+J23+/rtWxCCkWS21fnHvHM7GzR8wleZyBAAsy4oEFxlQgKs3 JsxE552BNSHITESHqJYU5PmdIMsUYxZ8dxoozr9oY3PEyhlqMh7cwmzHLYd2962D79DY x3yOVqCPHQ96sPDcqPc/GNw1/F4JTOL5Savo91TuuZaeXHQl94YVNOQyijMsjcIIr5FS tQUhJL/Uf3ZnYg1etBnjBbVKFYjytyfARd6dnY1YffQmm3lf8EFsJ7PcIPFczJHdXjAO P25ZATKbcHdEMEy96cUDZQNf6HVuLrIvUW+6XlDQWWepVz3pCWx559+sDyAiA7zPJF7M g68A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=f8VdMF9drbTP4owyGqx4nJ9btSFaMVXAZawUVkk2NwU=; b=IkmdGaYaiBRQJwMccWyxjcKgUo+YHOP+peAcLmik+RbKmONSgOT97x/xQgRePmP6da ahpr25k/TC4PGQfRFaeyoP8ulJVfm0+KHqGHxycYtY8VRLu8KcjfXr5/rfqu7OFJnaf7 mi5SXyz8atLajFawuD2Bd+Zt8YqMvCJ81J3bXffngthiPT4cR3Jrr7iSn+gUCaYkDwDK 6QKOOfry8PG9KHy4ziuvL7K4L9XoyFHCBqCFEoxVByiCVapebNi+xUvxNbfnR3E7drFR NqQJmRkCz52z8zLtdaJDIGijDk7F9ea1Euz6ISaS8Ny1nj82TeHyYx9NDWYWMj4IUneI alwg== X-Gm-Message-State: ABUngveIPntdRVI25KEQxFeocqLAN224ns6CP4u/udRzoBeDTmB1MPBPdUL2NnlfVw2dJw== X-Received: by 10.237.48.164 with SMTP id 33mr10261921qtf.95.1478205136549; Thu, 03 Nov 2016 13:32:16 -0700 (PDT) Received: from localhost (nat-pool-bos-t.redhat.com. [66.187.233.206]) by smtp.gmail.com with ESMTPSA id 16sm5478773qtt.38.2016.11.03.13.32.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 03 Nov 2016 13:32:16 -0700 (PDT) From: Rob Clark To: dri-devel@lists.freedesktop.org Subject: [PATCH] dma-buf/fence-array: enable_signaling from wq Date: Thu, 3 Nov 2016 16:31:14 -0400 Message-Id: <1478205074-8956-1-git-send-email-robdclark@gmail.com> X-Mailer: git-send-email 2.7.4 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 adding the callbacks on the array member fences 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: [] fence_signal+0x5c/0xf8 but task is already holding lock: (&(&obj->child_list_lock)->rlock){......}, at: [] 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){......}: [] __lock_acquire+0x173c/0x18d8 [] lock_acquire+0x4c/0x68 [] _raw_spin_lock_irqsave+0x54/0x70 [] fence_add_callback+0x3c/0x100 [] fence_array_enable_signaling+0x80/0x170 [] fence_add_callback+0xb8/0x100 [] sync_file_poll+0xd4/0xf0 [] do_sys_poll+0x220/0x438 [] SyS_ppoll+0x1b0/0x1d8 [] el0_svc_naked+0x24/0x28 -> #0 (&(&array->lock)->rlock){......}: [] print_circular_bug+0x80/0x2e0 [] __lock_acquire+0x17c4/0x18d8 [] lock_acquire+0x4c/0x68 [] _raw_spin_lock_irqsave+0x54/0x70 [] fence_signal+0x5c/0xf8 [] fence_array_cb_func+0x78/0x88 [] fence_signal_locked+0x80/0xe0 [] sw_sync_ioctl+0x2f8/0x3b0 [] do_vfs_ioctl+0xa4/0x790 [] SyS_ioctl+0x8c/0xa0 [] 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: [] sw_sync_ioctl+0x228/0x3b0 Signed-off-by: Rob Clark --- drivers/dma-buf/dma-fence-array.c | 18 ++++++++++++++---- include/linux/dma-fence-array.h | 4 ++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 67eb7c8..274bbb5 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -43,9 +43,10 @@ static void dma_fence_array_cb_func(struct dma_fence *f, dma_fence_put(&array->base); } -static bool dma_fence_array_enable_signaling(struct dma_fence *fence) +static void enable_signaling_worker(struct work_struct *w) { - struct dma_fence_array *array = to_dma_fence_array(fence); + struct dma_fence_array *array = + container_of(w, struct dma_fence_array, enable_signaling_worker); struct dma_fence_array_cb *cb = (void *)(&array[1]); unsigned i; @@ -63,11 +64,18 @@ static bool dma_fence_array_enable_signaling(struct dma_fence *fence) if (dma_fence_add_callback(array->fences[i], &cb[i].cb, dma_fence_array_cb_func)) { dma_fence_put(&array->base); - if (atomic_dec_and_test(&array->num_pending)) - return false; + if (atomic_dec_and_test(&array->num_pending)) { + dma_fence_signal(&array->base); + return; + } } } +} +static bool dma_fence_array_enable_signaling(struct dma_fence *fence) +{ + struct dma_fence_array *array = to_dma_fence_array(fence); + queue_work(system_unbound_wq, &array->enable_signaling_worker); return true; } @@ -141,6 +149,8 @@ struct dma_fence_array *dma_fence_array_create(int num_fences, atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences); array->fences = fences; + INIT_WORK(&array->enable_signaling_worker, enable_signaling_worker); + return array; } EXPORT_SYMBOL(dma_fence_array_create); diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h index 5900945..f48e8f4 100644 --- a/include/linux/dma-fence-array.h +++ b/include/linux/dma-fence-array.h @@ -35,6 +35,8 @@ struct dma_fence_array_cb { /** * struct dma_fence_array - fence to represent an array of fences * @base: fence base class + * @enable_signaling_worker: &work_struct for deferring enable_signaling + * to context not holding fence->lock * @lock: spinlock for fence handling * @num_fences: number of fences in the array * @num_pending: fences in the array still pending @@ -43,6 +45,8 @@ struct dma_fence_array_cb { struct dma_fence_array { struct dma_fence base; + struct work_struct enable_signaling_worker; + spinlock_t lock; unsigned num_fences; atomic_t num_pending;