Re: [PATCH 3/5 v8] OMAP: McBSP: Introduce caching in register write operations

[Jarkko Nikula <jhnikula@gmail.com>]

On Tue, 15 Dec 2009 01:36:47 +0100
Janusz Krzysztofik <jkrzyszt@tis.icnet.pl> wrote:

> 3. In omap_mcbsp_free(), marking the port as free before deallocating the 
>    cache, could result in memory leak as well.
> 
> I hope all are addressed correctly. Since releasing the port with
> 
> 	mcbsp->free = 1;
> 
> after kzalloc() failure should be atomic, I have assumed this doesn't require 
> locking.
> 
Good catch. One comment below.

> @@ -471,6 +491,9 @@ void omap_mcbsp_free(unsigned int id)
>  		free_irq(mcbsp->tx_irq, (void *)mcbsp);
>  	}
>  
> +	kfree(mcbsp->reg_cache);
> +	mcbsp->reg_cache = NULL;
> +
>  	spin_lock(&mcbsp->lock);
>  	if (mcbsp->free) {
>  		dev_err(mcbsp->dev, "McBSP%d was not reserved\n",

This is fine in current form but to play safe, lets do the kfree
inside the spin_lock section where the mcbsp->free is set. Then there
is no risk if some future code would use the mcbsp->free as a guard for
cache etc. access.


-- 
Jarkko