All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bo Shen <voice.shen@atmel.com>
To: Lars-Peter Clausen <lars@metafoo.de>
Cc: Linux-ALSA <alsa-devel@alsa-project.org>,
	Nicolas Ferre <nicolas.ferre@atmel.com>
Subject: Re: ASoC: atmel-pcm-dma: Leaking memory in atmel_pcm_hw_params
Date: Tue, 19 Mar 2013 09:18:43 +0800	[thread overview]
Message-ID: <5147BCF3.6060106@atmel.com> (raw)
In-Reply-To: <51473462.7030605@metafoo.de>

Hi Lars,

On 3/18/2013 23:36, Lars-Peter Clausen wrote:
> On 03/18/2013 04:59 AM, Bo Shen wrote:
>> Hi Lars,
>>
>> On 3/17/2013 22:39, Lars-Peter Clausen wrote:
>>> Hi,
>>>
>>> The dmaengine based pcm driver for atmel calls snd_dmaengine_pcm_open from
>>> it's hw_params callback. There is nothing preventing an application calling
>>> hw_params more than once. snd_dmaengine_pcm_open allocates a new prtd struct
>>> each time it gets called. So in case hw_params is called multiple times we
>>> leak memory here.
>>
>> I use alsa utils and don't meet this issue. Please point out which software
>> you use meet this issue.
>
> I found the issue by looking at the code, but I think any application using
> the oss compatibility layer of ALSA will trigger this.

Thanks for this information.

>
>>
>>> Is there any specific reason why snd_dmaengine_pcm_open() needs to be called
>>> from the hw_params callback and why it can't be called from the open
>>> callback?
>>
>> This is because dma data setting and usage.
>>
>> The filter function depends on the data returned by
>>> snd_soc_dai_get_dma_data(), but as far as I can see the DAI driver calls
>>> snd_soc_dai_set_dma_data() from its startup() callback, so the data is
>>> available in the PCM drivers open callback.
>>
>> Have you test this yet?
>
> I don't have the hardware. I just looked at the code again, and it looks
> like I must have misread it the first time. snd_soc_dai_set_dma_data() is
> called from the DAI drivers hwparams callback. But the data that gets passed
> to snd_soc_dai_set_dma_data() doesn't really depend on anything that's only
> available in the hwparams callback. So the snd_soc_dai_set_dma_data() could
> easily be move to the DAI driver startup() callback.

I will implement this, and send patch for this.

Thanks again.

> - Lars
>

Best Regards,
Bo Shen

      reply	other threads:[~2013-03-19  1:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-17 14:39 ASoC: atmel-pcm-dma: Leaking memory in atmel_pcm_hw_params Lars-Peter Clausen
2013-03-18  3:59 ` Bo Shen
2013-03-18 15:36   ` Lars-Peter Clausen
2013-03-19  1:18     ` Bo Shen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5147BCF3.6060106@atmel.com \
    --to=voice.shen@atmel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=lars@metafoo.de \
    --cc=nicolas.ferre@atmel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.