From patchwork Sat Sep 6 00:00:00 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 4859781 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 62B11C0338 for ; Mon, 8 Sep 2014 01:21:17 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6C8C020114 for ; Mon, 8 Sep 2014 01:21:16 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id E3E402011E for ; Mon, 8 Sep 2014 01:21:14 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 845116E19D; Sun, 7 Sep 2014 18:21:12 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.11.231]) by gabe.freedesktop.org (Postfix) with ESMTP id 842996E059 for ; Fri, 5 Sep 2014 17:00:04 -0700 (PDT) Received: from smtp.codeaurora.org (localhost [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id 3A95513EC5C; Sat, 6 Sep 2014 00:00:02 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 486) id 2C18C13EC78; Sat, 6 Sep 2014 00:00:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from localhost (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: sboyd@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id C5E0413EC5C; Sat, 6 Sep 2014 00:00:01 +0000 (UTC) Date: Fri, 5 Sep 2014 17:00:00 -0700 From: Stephen Boyd To: Rob Clark Subject: Re: [PATCH v2] clk: Don't hold prepare_lock across debugfs creation Message-ID: <20140906000000.GB22593@codeaurora.org> References: <1409899069-13313-1-git-send-email-sboyd@codeaurora.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1409899069-13313-1-git-send-email-sboyd@codeaurora.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Virus-Scanned: ClamAV using ClamSMTP X-Mailman-Approved-At: Sun, 07 Sep 2014 18:21:08 -0700 Cc: Mike Turquette , linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP On 09/04, Stephen Boyd wrote: > > I don't understand why we need to hold the prepare lock around > the kref_put(), so I changed the flow so that we don't do this > when we unregister a clock. Ok we hold the prepare mutex to make sure get and put are serialized. Good. Here's the interdiff to move the debugfs unregistration before the prepare lock and detect double unregisters without holding the prepare lock. diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 3c04d0d69b96..8ca28189e4e9 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -338,10 +338,14 @@ unlock: static void clk_debug_unregister(struct clk *clk) { mutex_lock(&clk_debug_lock); - hlist_del_init(&clk->debug_node); - mutex_unlock(&clk_debug_lock); + if (!clk->dentry) + goto out; + hlist_del_init(&clk->debug_node); debugfs_remove_recursive(clk->dentry); + clk->dentry = NULL; +out: + mutex_unlock(&clk_debug_lock); } struct dentry *clk_debugfs_add_file(struct clk *clk, char *name, umode_t mode, @@ -2065,14 +2069,15 @@ void clk_unregister(struct clk *clk) { unsigned long flags; - if (!clk || WARN_ON_ONCE(IS_ERR(clk))) - return; + if (!clk || WARN_ON_ONCE(IS_ERR(clk))) + return; + + clk_debug_unregister(clk); clk_prepare_lock(); if (clk->ops == &clk_nodrv_ops) { pr_err("%s: unregistered clock: %s\n", __func__, clk->name); - clk_prepare_unlock(); return; } /* @@ -2097,11 +2102,9 @@ void clk_unregister(struct clk *clk) if (clk->prepare_count) pr_warn("%s: unregistering prepared clock: %s\n", __func__, clk->name); - clk_prepare_unlock(); - - clk_debug_unregister(clk); - kref_put(&clk->ref, __clk_release); + + clk_prepare_unlock(); } EXPORT_SYMBOL_GPL(clk_unregister);