From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6280FC43381 for ; Tue, 5 Mar 2019 18:49:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 319B52133F for ; Tue, 5 Mar 2019 18:49:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1551811779; bh=k+Iq3xwBIbt9ZPAisBgeg2pj+qY/jWn5gCD7h7gjDnA=; h=In-Reply-To:References:From:Subject:Cc:To:Date:List-ID:From; b=vMywXozFI1SOPHNjFy3rCQ3lYSTnM5PqdH7EjMI6/IhooGCfu5qv0BEzutWwobZQD rSE9fIl5jOcSfCnstGrR1+oz/ZtN6mhr2U26uNtpCjfWlve8gCj5qLveZzxZSt+qWD z1lz5XYIhv14s7hTsivej2S1mOFg9exdfzNvU5fI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727686AbfCESth (ORCPT ); Tue, 5 Mar 2019 13:49:37 -0500 Received: from mail.kernel.org ([198.145.29.99]:44788 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726190AbfCEStg (ORCPT ); Tue, 5 Mar 2019 13:49:36 -0500 Received: from localhost (unknown [104.132.0.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 15CBF208E4; Tue, 5 Mar 2019 18:49:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1551811776; bh=k+Iq3xwBIbt9ZPAisBgeg2pj+qY/jWn5gCD7h7gjDnA=; h=In-Reply-To:References:From:Subject:Cc:To:Date:From; b=mC6lKazT6lIrm9i+G1AcAWAwRkMzT8/Ne7BEWwXlNXjNPYIOYPwlaerFACuAP2N03 X9CvpctEsfCQmrSSWm6IOqDTjv7d6VVZ4oGqXbf0ZrgJOd/XLwzYm3joGozHT8cp9A gYfeXENWzH1wYz6K9nVJbSL2DeYeMr5Zxvx68/Eo= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20190305044936.22267-2-dbasehore@chromium.org> References: <20190305044936.22267-1-dbasehore@chromium.org> <20190305044936.22267-2-dbasehore@chromium.org> From: Stephen Boyd Subject: Re: [PATCH v2 1/6] clk: Remove recursion in clk_core_{prepare,enable}() Cc: linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, linux-doc@vger.kernel.org, mturquette@baylibre.com, heiko@sntech.de, aisheng.dong@nxp.com, mchehab+samsung@kernel.org, corbet@lwn.net, jbrunet@baylibre.com, Stephen Boyd , Derek Basehore To: Derek Basehore , linux-kernel@vger.kernel.org Message-ID: <155181177527.20095.15680753964583935841@swboyd.mtv.corp.google.com> User-Agent: alot/0.8 Date: Tue, 05 Mar 2019 10:49:35 -0800 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Quoting Derek Basehore (2019-03-04 20:49:31) > From: Stephen Boyd >=20 > Enabling and preparing clocks can be written quite naturally with > recursion. We start at some point in the tree and recurse up the > tree to find the oldest parent clk that needs to be enabled or > prepared. Then we enable/prepare and return to the caller, going > back to the clk we started at and enabling/preparing along the > way. This also unroll the recursion in unprepare,disable which can > just be done in the order of walking up the clk tree. >=20 > The problem is recursion isn't great for kernel code where we > have a limited stack size. Furthermore, we may be calling this > code inside clk_set_rate() which also has recursion in it, so > we're really not looking good if we encounter a tall clk tree. >=20 > Let's create a stack instead by looping over the parent chain and > collecting clks of interest. Then the enable/prepare becomes as > simple as iterating over that list and calling enable. >=20 > Modified verison of https://lore.kernel.org/patchwork/patch/814369/ > -Fixed kernel warning > -unrolled recursion in unprepare/disable too >=20 > Cc: Jerome Brunet > Signed-off-by: Stephen Boyd > Signed-off-by: Derek Basehore > --- >From the original post: "I have some vague fear that this may not work if a clk op is framework=20 reentrant and attemps to call consumer clk APIs from within the clk ops. If the reentrant call tries to add a clk that's already in the list then we'll corrupt the list. Ugh." Do we have this sort of problem here? Or are you certain that we don't have clks that prepare or enable something that is already in the process of being prepared or enabled? From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH v2 1/6] clk: Remove recursion in clk_core_{prepare, enable}() Date: Tue, 05 Mar 2019 10:49:35 -0800 Message-ID: <155181177527.20095.15680753964583935841@swboyd.mtv.corp.google.com> References: <20190305044936.22267-1-dbasehore@chromium.org> <20190305044936.22267-2-dbasehore@chromium.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190305044936.22267-2-dbasehore@chromium.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: linux-kernel@vger.kernel.org Cc: aisheng.dong@nxp.com, Derek Basehore , heiko@sntech.de, linux-doc@vger.kernel.org, mturquette@baylibre.com, corbet@lwn.net, Stephen Boyd , linux-rockchip@lists.infradead.org, mchehab+samsung@kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, jbrunet@baylibre.com List-Id: linux-rockchip.vger.kernel.org Quoting Derek Basehore (2019-03-04 20:49:31) > From: Stephen Boyd > > Enabling and preparing clocks can be written quite naturally with > recursion. We start at some point in the tree and recurse up the > tree to find the oldest parent clk that needs to be enabled or > prepared. Then we enable/prepare and return to the caller, going > back to the clk we started at and enabling/preparing along the > way. This also unroll the recursion in unprepare,disable which can > just be done in the order of walking up the clk tree. > > The problem is recursion isn't great for kernel code where we > have a limited stack size. Furthermore, we may be calling this > code inside clk_set_rate() which also has recursion in it, so > we're really not looking good if we encounter a tall clk tree. > > Let's create a stack instead by looping over the parent chain and > collecting clks of interest. Then the enable/prepare becomes as > simple as iterating over that list and calling enable. > > Modified verison of https://lore.kernel.org/patchwork/patch/814369/ > -Fixed kernel warning > -unrolled recursion in unprepare/disable too > > Cc: Jerome Brunet > Signed-off-by: Stephen Boyd > Signed-off-by: Derek Basehore > --- >From the original post: "I have some vague fear that this may not work if a clk op is framework reentrant and attemps to call consumer clk APIs from within the clk ops. If the reentrant call tries to add a clk that's already in the list then we'll corrupt the list. Ugh." Do we have this sort of problem here? Or are you certain that we don't have clks that prepare or enable something that is already in the process of being prepared or enabled? From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 991AAC43381 for ; Tue, 5 Mar 2019 18:49:51 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6AB50208E4 for ; Tue, 5 Mar 2019 18:49:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Qmcoxbwq"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="mC6lKazT" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6AB50208E4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Date:Message-ID:To:Subject:From: References:In-Reply-To:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3vSJ9SINkGx9b8kBd/pPKNDHmuGyRcRlr5U+/xpV6KQ=; b=QmcoxbwqFq/clm LFeHYNyhBKFmEO9VBtVp8zhjKkXer/XPUiXutDLelM7sP/yCWZcAfJar7mrkOndnpZUTm/V0dUL5g FHmjyWgbKYmAu8jxfIjpkGJ5oUpcjhobW/YnJDPummK3CE2CoKLSgPgzaT5bQ/9g7XkkGQ+8CrUFC yK3lMZvlclEDbpwHjeTOfqYZP9DVBZLvZbJVkJBGvSCrMolDedrK57wYHblWHJMpGuSrpSU4PfpYp caotgOG/KoEu8ErnmO6Iu7+Q8ZkQ58ph2feNqNFa4cD9ETCm+cveK195tlGpSXsLx5/X/2sluLNDN bc7Qo/ifmiPRtmSP14qA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h1F8H-0000NP-2u; Tue, 05 Mar 2019 18:49:41 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h1F8D-0000Mj-3l; Tue, 05 Mar 2019 18:49:38 +0000 Received: from localhost (unknown [104.132.0.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 15CBF208E4; Tue, 5 Mar 2019 18:49:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1551811776; bh=k+Iq3xwBIbt9ZPAisBgeg2pj+qY/jWn5gCD7h7gjDnA=; h=In-Reply-To:References:From:Subject:Cc:To:Date:From; b=mC6lKazT6lIrm9i+G1AcAWAwRkMzT8/Ne7BEWwXlNXjNPYIOYPwlaerFACuAP2N03 X9CvpctEsfCQmrSSWm6IOqDTjv7d6VVZ4oGqXbf0ZrgJOd/XLwzYm3joGozHT8cp9A gYfeXENWzH1wYz6K9nVJbSL2DeYeMr5Zxvx68/Eo= MIME-Version: 1.0 In-Reply-To: <20190305044936.22267-2-dbasehore@chromium.org> References: <20190305044936.22267-1-dbasehore@chromium.org> <20190305044936.22267-2-dbasehore@chromium.org> From: Stephen Boyd Subject: Re: [PATCH v2 1/6] clk: Remove recursion in clk_core_{prepare, enable}() To: Derek Basehore , linux-kernel@vger.kernel.org Message-ID: <155181177527.20095.15680753964583935841@swboyd.mtv.corp.google.com> User-Agent: alot/0.8 Date: Tue, 05 Mar 2019 10:49:35 -0800 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190305_104937_176325_A3921F4B X-CRM114-Status: GOOD ( 14.64 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: aisheng.dong@nxp.com, Derek Basehore , heiko@sntech.de, linux-doc@vger.kernel.org, mturquette@baylibre.com, corbet@lwn.net, Stephen Boyd , linux-rockchip@lists.infradead.org, mchehab+samsung@kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, jbrunet@baylibre.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Quoting Derek Basehore (2019-03-04 20:49:31) > From: Stephen Boyd > > Enabling and preparing clocks can be written quite naturally with > recursion. We start at some point in the tree and recurse up the > tree to find the oldest parent clk that needs to be enabled or > prepared. Then we enable/prepare and return to the caller, going > back to the clk we started at and enabling/preparing along the > way. This also unroll the recursion in unprepare,disable which can > just be done in the order of walking up the clk tree. > > The problem is recursion isn't great for kernel code where we > have a limited stack size. Furthermore, we may be calling this > code inside clk_set_rate() which also has recursion in it, so > we're really not looking good if we encounter a tall clk tree. > > Let's create a stack instead by looping over the parent chain and > collecting clks of interest. Then the enable/prepare becomes as > simple as iterating over that list and calling enable. > > Modified verison of https://lore.kernel.org/patchwork/patch/814369/ > -Fixed kernel warning > -unrolled recursion in unprepare/disable too > > Cc: Jerome Brunet > Signed-off-by: Stephen Boyd > Signed-off-by: Derek Basehore > --- >From the original post: "I have some vague fear that this may not work if a clk op is framework reentrant and attemps to call consumer clk APIs from within the clk ops. If the reentrant call tries to add a clk that's already in the list then we'll corrupt the list. Ugh." Do we have this sort of problem here? Or are you certain that we don't have clks that prepare or enable something that is already in the process of being prepared or enabled? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel