linux-kernel - Re: [PATCH v2 1/6] clk: Remove recursion in clk_core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <155181177527.20095.15680753964583935841@swboyd.mtv.corp.google.com>
Date:   Tue, 05 Mar 2019 10:49:35 -0800
From:   Stephen Boyd <sboyd@...nel.org>
To:     Derek Basehore <dbasehore@...omium.org>,
        linux-kernel@...r.kernel.org
Cc:     linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-rockchip@...ts.infradead.org, linux-doc@...r.kernel.org,
        mturquette@...libre.com, heiko@...ech.de, aisheng.dong@....com,
        mchehab+samsung@...nel.org, corbet@....net, jbrunet@...libre.com,
        Stephen Boyd <sboyd@...eaurora.org>,
        Derek Basehore <dbasehore@...omium.org>
Subject: Re: [PATCH v2 1/6] clk: Remove recursion in clk_core_{prepare,enable}()

Quoting Derek Basehore (2019-03-04 20:49:31)
> From: Stephen Boyd <sboyd@...eaurora.org>
> 
> 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 <jbrunet@...libre.com>
> Signed-off-by: Stephen Boyd <sboyd@...eaurora.org>
> Signed-off-by: Derek Basehore <dbasehore@...omium.org>
> ---

>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?