From patchwork Fri Mar 1 00:43:10 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 2201561 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 63FEA3FCA4 for ; Fri, 1 Mar 2013 12:44:10 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 57A72E6400 for ; Fri, 1 Mar 2013 04:44:10 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pb0-f53.google.com (mail-pb0-f53.google.com [209.85.160.53]) by gabe.freedesktop.org (Postfix) with ESMTP id 6C43AE60F3 for ; Thu, 28 Feb 2013 16:44:01 -0800 (PST) Received: by mail-pb0-f53.google.com with SMTP id un1so1380311pbc.26 for ; Thu, 28 Feb 2013 16:44:01 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:x-gm-message-state; bh=weix2x5juI4VuZXf3a17p+WTIHjK9wgdvHctutIKUk0=; b=OLBKzvL3aAEWO80asYN7u9R1AGnIl0SJUsM1Drb3TyQdzlCAlye9GBqlNk6AvHMdyK zhvBfmWxqFu8HLmXdJlBt40L0Du9WkaseSHuB/XOujbHphUJKDuco6Crdh/XyHeG1jEb qTRTdpvfuqAKXfj97MJBMaO3QVkneNiX2TdM/kqZMWKkFrMtH/7mEo9lH+B1Qg1UL6pq WMutC3Bg31Gwd2wyNcV8osH/vsNvJs/jeMvJzKJJ+Egxd1qwj+R14I15TXNiOAujaWY7 njvNml0lbDDd6xeP1FacLeVe1JLTKMFZ5m5kbadwG6wqcINbyU9Uv0oXltb3OtQcDc9+ s9lA== X-Received: by 10.68.197.168 with SMTP id iv8mr11735434pbc.114.1362098641337; Thu, 28 Feb 2013 16:44:01 -0800 (PST) Received: from localhost.localdomain (c-24-21-54-107.hsd1.or.comcast.net. [24.21.54.107]) by mx.google.com with ESMTPS id dx17sm10914892pac.17.2013.02.28.16.43.59 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 28 Feb 2013 16:44:00 -0800 (PST) From: John Stultz To: lkml Subject: [PATCH 14/30] staging: sync: Add internal refcounting to fences Date: Thu, 28 Feb 2013 16:43:10 -0800 Message-Id: <1362098606-26469-15-git-send-email-john.stultz@linaro.org> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1362098606-26469-1-git-send-email-john.stultz@linaro.org> References: <1362098606-26469-1-git-send-email-john.stultz@linaro.org> X-Gm-Message-State: ALoCoQnhASsjfro9FIm+3kA8Wv1VrPrXscBWKc5+KZrb1ro7eHEjES1DGVGaOFwWU/Tf0fz9A5Ps X-Mailman-Approved-At: Fri, 01 Mar 2013 04:21:18 -0800 Cc: Daniel Vetter , dri-devel@lists.freedesktop.org, Rob Clark , John Stultz , Greg KH , Android Kernel Team X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org From: Erik Gilling If a fence is released while a timeline that one of it's pts is on is being signaled, it is possible for that fence to be deleted before it is signaled. This patch adds a refcount for internal references such as signaled pt processing. Cc: Maarten Lankhorst Cc: Erik Gilling Cc: Daniel Vetter Cc: Rob Clark Cc: Sumit Semwal Cc: Greg KH Cc: dri-devel@lists.freedesktop.org Cc: Android Kernel Team Signed-off-by: Erik Gilling Signed-off-by: John Stultz --- drivers/staging/android/sync.c | 54 ++++++++++++++++++++++++++++++++++------ drivers/staging/android/sync.h | 5 ++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index 6cb7c88..7d4e9aa 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -30,6 +30,7 @@ static void sync_fence_signal_pt(struct sync_pt *pt); static int _sync_pt_has_signaled(struct sync_pt *pt); +static void sync_fence_free(struct kref *kref); static LIST_HEAD(sync_timeline_list_head); static DEFINE_SPINLOCK(sync_timeline_list_lock); @@ -113,7 +114,7 @@ static void sync_timeline_remove_pt(struct sync_pt *pt) { struct sync_timeline *obj = pt->parent; unsigned long flags; - bool needs_freeing; + bool needs_freeing = false; spin_lock_irqsave(&obj->active_list_lock, flags); if (!list_empty(&pt->active_list)) @@ -121,8 +122,11 @@ static void sync_timeline_remove_pt(struct sync_pt *pt) spin_unlock_irqrestore(&obj->active_list_lock, flags); spin_lock_irqsave(&obj->child_list_lock, flags); - list_del(&pt->child_list); - needs_freeing = obj->destroyed && list_empty(&obj->child_list_head); + if (!list_empty(&pt->child_list)) { + list_del_init(&pt->child_list); + needs_freeing = obj->destroyed && + list_empty(&obj->child_list_head); + } spin_unlock_irqrestore(&obj->child_list_lock, flags); if (needs_freeing) @@ -141,18 +145,22 @@ void sync_timeline_signal(struct sync_timeline *obj) struct sync_pt *pt = container_of(pos, struct sync_pt, active_list); - if (_sync_pt_has_signaled(pt)) - list_move(pos, &signaled_pts); + if (_sync_pt_has_signaled(pt)) { + list_del_init(pos); + list_add(&pt->signaled_list, &signaled_pts); + kref_get(&pt->fence->kref); + } } spin_unlock_irqrestore(&obj->active_list_lock, flags); list_for_each_safe(pos, n, &signaled_pts) { struct sync_pt *pt = - container_of(pos, struct sync_pt, active_list); + container_of(pos, struct sync_pt, signaled_list); list_del_init(pos); sync_fence_signal_pt(pt); + kref_put(&pt->fence->kref, sync_fence_free); } } EXPORT_SYMBOL(sync_timeline_signal); @@ -253,6 +261,7 @@ static struct sync_fence *sync_fence_alloc(const char *name) if (fence->file == NULL) goto err; + kref_init(&fence->kref); strlcpy(fence->name, name, sizeof(fence->name)); INIT_LIST_HEAD(&fence->pt_list_head); @@ -362,6 +371,16 @@ static int sync_fence_merge_pts(struct sync_fence *dst, struct sync_fence *src) return 0; } +static void sync_fence_detach_pts(struct sync_fence *fence) +{ + struct list_head *pos, *n; + + list_for_each_safe(pos, n, &fence->pt_list_head) { + struct sync_pt *pt = container_of(pos, struct sync_pt, pt_list); + sync_timeline_remove_pt(pt); + } +} + static void sync_fence_free_pts(struct sync_fence *fence) { struct list_head *pos, *n; @@ -565,18 +584,37 @@ int sync_fence_wait(struct sync_fence *fence, long timeout) } EXPORT_SYMBOL(sync_fence_wait); +static void sync_fence_free(struct kref *kref) +{ + struct sync_fence *fence = container_of(kref, struct sync_fence, kref); + + sync_fence_free_pts(fence); + + kfree(fence); +} + static int sync_fence_release(struct inode *inode, struct file *file) { struct sync_fence *fence = file->private_data; unsigned long flags; + /* + * We need to remove all ways to access this fence before droping + * our ref. + * + * start with its membership in the global fence list + */ spin_lock_irqsave(&sync_fence_list_lock, flags); list_del(&fence->sync_fence_list); spin_unlock_irqrestore(&sync_fence_list_lock, flags); - sync_fence_free_pts(fence); + /* + * remove its pts from their parents so that sync_timeline_signal() + * can't reference the fence. + */ + sync_fence_detach_pts(fence); - kfree(fence); + kref_put(&fence->kref, sync_fence_free); return 0; } diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h index 943f414..00c9bae 100644 --- a/drivers/staging/android/sync.h +++ b/drivers/staging/android/sync.h @@ -16,6 +16,7 @@ #include #ifdef __KERNEL__ +#include #include #include #include @@ -109,6 +110,7 @@ struct sync_timeline { * @parent: sync_timeline to which this sync_pt belongs * @child_list: membership in sync_timeline.child_list_head * @active_list: membership in sync_timeline.active_list_head + * @signaled_list: membership in temorary signaled_list on stack * @fence: sync_fence to which the sync_pt belongs * @pt_list: membership in sync_fence.pt_list_head * @status: 1: signaled, 0:active, <0: error @@ -120,6 +122,7 @@ struct sync_pt { struct list_head child_list; struct list_head active_list; + struct list_head signaled_list; struct sync_fence *fence; struct list_head pt_list; @@ -133,6 +136,7 @@ struct sync_pt { /** * struct sync_fence - sync fence * @file: file representing this fence + * @kref: referenace count on fence. * @name: name of sync_fence. Useful for debugging * @pt_list_head: list of sync_pts in ths fence. immutable once fence * is created @@ -145,6 +149,7 @@ struct sync_pt { */ struct sync_fence { struct file *file; + struct kref kref; char name[32]; /* this list is immutable once the fence is created */