Re: [PATCH v2 1/6] clk: Remove recursion in clk_core_{prepare,enable}()

From: Stephen Boyd
Date: Tue Mar 05 2019 - 13:49:58 EST


Quoting Derek Basehore (2019-03-04 20:49:31)
> From: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxx>
> Signed-off-by: Stephen Boyd <sboyd@xxxxxxxxxxxxxx>
> Signed-off-by: Derek Basehore <dbasehore@xxxxxxxxxxxx>
> ---

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