Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965261AbbLORRn (ORCPT ); Tue, 15 Dec 2015 12:17:43 -0500 Received: from mail-ig0-f174.google.com ([209.85.213.174]:38837 "EHLO mail-ig0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965147AbbLORRk (ORCPT ); Tue, 15 Dec 2015 12:17:40 -0500 MIME-Version: 1.0 In-Reply-To: <20151215092601.GI3189@phenom.ffwll.local> References: <20151215012955.GA28277@dtor-ws> <20151215092601.GI3189@phenom.ffwll.local> Date: Tue, 15 Dec 2015 09:17:37 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] android: fix warning when releasing active sync point From: Dmitry Torokhov To: Dmitry Torokhov , Greg Kroah-Hartman , devel@driverdev.osuosl.org, Andrew Bresticker , =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= , dri-devel@lists.freedesktop.org, "linux-kernel@vger.kernel.org" , Riley Andrews , linux-media@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3652 Lines: 77 On Tue, Dec 15, 2015 at 1:26 AM, Daniel Vetter wrote: > On Mon, Dec 14, 2015 at 05:29:55PM -0800, Dmitry Torokhov wrote: >> Userspace can close the sync device while there are still active fence >> points, in which case kernel produces the following warning: >> >> [ 43.853176] ------------[ cut here ]------------ >> [ 43.857834] WARNING: CPU: 0 PID: 892 at /mnt/host/source/src/third_party/kernel/v3.18/drivers/staging/android/sync.c:439 android_fence_release+0x88/0x104() >> [ 43.871741] CPU: 0 PID: 892 Comm: Binder_5 Tainted: G U 3.18.0-07661-g0550ce9 #1 >> [ 43.880176] Hardware name: Google Tegra210 Smaug Rev 1+ (DT) >> [ 43.885834] Call trace: >> [ 43.888294] [] dump_backtrace+0x0/0x10c >> [ 43.893697] [] show_stack+0x10/0x1c >> [ 43.898756] [] dump_stack+0x74/0xb8 >> [ 43.903814] [] warn_slowpath_common+0x84/0xb0 >> [ 43.909736] [] warn_slowpath_null+0x14/0x20 >> [ 43.915482] [] android_fence_release+0x84/0x104 >> [ 43.921582] [] fence_release+0x104/0x134 >> [ 43.927066] [] sync_fence_free+0x74/0x9c >> [ 43.932552] [] sync_fence_release+0x34/0x48 >> [ 43.938304] [] __fput+0x100/0x1b8 >> [ 43.943185] [] ____fput+0x8/0x14 >> [ 43.947982] [] task_work_run+0xb0/0xe4 >> [ 43.953297] [] do_notify_resume+0x44/0x5c >> [ 43.958867] ---[ end trace 5a2aa4027cc5d171 ]--- >> >> Let's fix it by introducing a new optional callback (disable_signaling) >> to fence operations so that drivers can do proper clean ups when we >> remove last callback for given fence. >> >> Reviewed-by: Andrew Bresticker >> Signed-off-by: Dmitry Torokhov >> --- >> drivers/dma-buf/fence.c | 6 +++++- >> drivers/staging/android/sync.c | 8 ++++++++ >> include/linux/fence.h | 2 ++ >> 3 files changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c >> index 7b05dbe..0ed73ad 100644 >> --- a/drivers/dma-buf/fence.c >> +++ b/drivers/dma-buf/fence.c >> @@ -304,8 +304,12 @@ fence_remove_callback(struct fence *fence, struct fence_cb *cb) >> spin_lock_irqsave(fence->lock, flags); >> >> ret = !list_empty(&cb->node); >> - if (ret) >> + if (ret) { >> list_del_init(&cb->node); >> + if (list_empty(&fence->cb_list)) >> + if (fence->ops->disable_signaling) >> + fence->ops->disable_signaling(fence); > > What exactly is the bug here? A fence with no callbacks registered any > more shouldn't have any problem. Why exactly does this blow up? > > I guess I don't really understand the bug ... we do seem to remove the > callback already. > The issue is that when enabling signalling in sync driver we put fence on an internal list in the driver and there is no way of taking it off this list, except when it is signalled. The driver, when destroying the fence, checks if the fence is not on this list (as a sanity measure) and that produces the backtrace in the commit log. IOW for some drivers we need an "undo" for enable_signaling() callback so that drivers can maintain consistent internal state. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/