linux-kernel - Re: [PATCH E 11/14] OMAP clock: track child clocks

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20090129195211.GI18233@n2100.arm.linux.org.uk>
Date:	Thu, 29 Jan 2009 19:52:11 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Paul Walmsley <paul@...an.com>
Cc:	linux-arm-kernel@...ts.arm.linux.org.uk,
	linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org,
	Tony Lindgren <tony@...mide.com>
Subject: Re: [PATCH E 11/14] OMAP clock: track child clocks

On Wed, Jan 28, 2009 at 12:27:59PM -0700, Paul Walmsley wrote:
> +static int omap_clk_for_each_child(struct clk *clk, unsigned long parent_rate,
> +		   u8 rate_storage, int (*cb)(struct clk *, unsigned long, u8))
> +{
> +	struct clk_child *child;
> +	int ret;
> +
> +	list_for_each_entry(child, &clk->children, node) {
> +		ret = (*cb)(child->clk, parent_rate, rate_storage);
> +		if (ret)
> +			break;
> +	}
> +
> +	return ret;
> +}

> +static int _do_propagate_rate(struct clk *clk, unsigned long parent_rate,
> +			      u8 rate_storage)
> +{
> +	if (clk->recalc)
> +		clk->recalc(clk, parent_rate, rate_storage);
> +	if (omap_clk_has_children(clk))
> +		propagate_rate(clk, rate_storage);
> +	return 0;
> +}

>  /* Propagate rate to children */
>  void propagate_rate(struct clk *tclk, u8 rate_storage)
>  {
>  	unsigned long parent_rate = 0;
>  
>  	if (tclk == NULL || IS_ERR(tclk))
>  		return;
>  
> +	if (rate_storage == CURRENT_RATE)
> +		parent_rate = tclk->rate;
> +	else if (rate_storage == TEMP_RATE)
> +		parent_rate = tclk->temp_rate;
>  
> +	omap_clk_for_each_child(tclk, parent_rate, rate_storage,
> +				_do_propagate_rate);
>  }

This worries me.  Calling this puts onto the stack:

- a frame for propagate_rate()
- a frame for omap_clk_for_each_child
- a frame for _do_propagate_rate

for every level of children.  How close we get to overflowing the kernels
depends on how much each of those functions puts on the kernel stack.
However, since this is recursive, minimising the number of stack frames
is a good idea.

That's why I have in my patch:

 [ARM] omap: move propagate_rate() calls into generic omap clock code

I've arranged for there to be the minimum of function nesting here.
I suggest keeping this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/