All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org, gustavoars@kernel.org,
	linux-kernel@vger.kernel.org, shengjiu.wang@nxp.com,
	pierre-louis.bossart@linux.intel.com, tiwai@suse.com,
	xiang@kernel.org, Robin Gong <yibin.gong@nxp.com>,
	akpm@linux-foundation.org
Subject: Re: [PATCH v1 ] ALSA: core: memalloc: add page alignment for iram
Date: Thu, 17 Dec 2020 11:59:23 +0100	[thread overview]
Message-ID: <70074f62-954a-9b40-ab4a-cb438925060c@metafoo.de> (raw)
In-Reply-To: <s5h5z50n4dd.wl-tiwai@suse.de>

On 12/17/20 10:55 AM, Takashi Iwai wrote:
> On Thu, 17 Dec 2020 10:43:45 +0100,
> Lars-Peter Clausen wrote:
>> On 12/17/20 5:15 PM, Robin Gong wrote:
>>> Since mmap for userspace is based on page alignment, add page alignment
>>> for iram alloc from pool, otherwise, some good data located in the same
>>> page of dmab->area maybe touched wrongly by userspace like pulseaudio.
>>>
>> I wonder, do we also have to align size to be a multiple of PAGE_SIZE
>> to avoid leaking unrelated data?
> Hm, a good question.  Basically the PCM buffer size itself shouldn't
> be influenced by that (i.e. no hw-constraint or such is needed), but
> the padding should be cleared indeed.  I somehow left those to the
> allocator side, but maybe it's safer to clear the whole buffer in
> sound/core/memalloc.c commonly.

What I meant was that most of the APIs that we use to allocate memory 
work on a PAGE_SIZE granularity. I.e. if you request a buffer that where 
the size is not a multiple of PAGE_SIZE internally they will still 
allocate a buffer that is a multiple of PAGE_SIZE and mark the unused 
bytes as reserved.

But I believe that is not the case gen_pool_dma_alloc(). It will happily 
allocate those extra bytes to some other allocation request.

That we need to zero out the reserved bytes even for those other APIs is 
a very good additional point!

I looked at this a few years ago and I'm pretty sure that we cleared out 
the allocated area, but I can't find that anymore in the current code. 
Which is not so great I guess.


WARNING: multiple messages have this Message-ID (diff)
From: Lars-Peter Clausen <lars@metafoo.de>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org, gustavoars@kernel.org,
	linux-kernel@vger.kernel.org, shengjiu.wang@nxp.com,
	tiwai@suse.com, pierre-louis.bossart@linux.intel.com,
	xiang@kernel.org, Robin Gong <yibin.gong@nxp.com>,
	akpm@linux-foundation.org
Subject: Re: [PATCH v1 ] ALSA: core: memalloc: add page alignment for iram
Date: Thu, 17 Dec 2020 11:59:23 +0100	[thread overview]
Message-ID: <70074f62-954a-9b40-ab4a-cb438925060c@metafoo.de> (raw)
In-Reply-To: <s5h5z50n4dd.wl-tiwai@suse.de>

On 12/17/20 10:55 AM, Takashi Iwai wrote:
> On Thu, 17 Dec 2020 10:43:45 +0100,
> Lars-Peter Clausen wrote:
>> On 12/17/20 5:15 PM, Robin Gong wrote:
>>> Since mmap for userspace is based on page alignment, add page alignment
>>> for iram alloc from pool, otherwise, some good data located in the same
>>> page of dmab->area maybe touched wrongly by userspace like pulseaudio.
>>>
>> I wonder, do we also have to align size to be a multiple of PAGE_SIZE
>> to avoid leaking unrelated data?
> Hm, a good question.  Basically the PCM buffer size itself shouldn't
> be influenced by that (i.e. no hw-constraint or such is needed), but
> the padding should be cleared indeed.  I somehow left those to the
> allocator side, but maybe it's safer to clear the whole buffer in
> sound/core/memalloc.c commonly.

What I meant was that most of the APIs that we use to allocate memory 
work on a PAGE_SIZE granularity. I.e. if you request a buffer that where 
the size is not a multiple of PAGE_SIZE internally they will still 
allocate a buffer that is a multiple of PAGE_SIZE and mark the unused 
bytes as reserved.

But I believe that is not the case gen_pool_dma_alloc(). It will happily 
allocate those extra bytes to some other allocation request.

That we need to zero out the reserved bytes even for those other APIs is 
a very good additional point!

I looked at this a few years ago and I'm pretty sure that we cleared out 
the allocated area, but I can't find that anymore in the current code. 
Which is not so great I guess.


  parent reply	other threads:[~2020-12-17 11:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-17 16:15 [PATCH v1 ] ALSA: core: memalloc: add page alignment for iram Robin Gong
2020-12-17  9:38 ` Takashi Iwai
2020-12-17  9:38   ` Takashi Iwai
2020-12-17  9:43 ` Lars-Peter Clausen
2020-12-17  9:55   ` Takashi Iwai
2020-12-17  9:55     ` Takashi Iwai
2020-12-17 10:14     ` Takashi Iwai
2020-12-17 10:14       ` Takashi Iwai
2020-12-17 10:44       ` Lars-Peter Clausen
2020-12-17 10:44         ` Lars-Peter Clausen
2020-12-17 10:59     ` Lars-Peter Clausen [this message]
2020-12-17 10:59       ` Lars-Peter Clausen
2020-12-17 11:06       ` Takashi Iwai
2020-12-17 11:06         ` Takashi Iwai
2020-12-17 13:16         ` Lars-Peter Clausen
2020-12-17 14:24           ` Takashi Iwai
2020-12-17 14:57             ` Lars-Peter Clausen
2020-12-17 15:18               ` Takashi Iwai
2020-12-17 15:38                 ` Lars-Peter Clausen
2020-12-17 16:28                   ` Takashi Iwai

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=70074f62-954a-9b40-ab4a-cb438925060c@metafoo.de \
    --to=lars@metafoo.de \
    --cc=akpm@linux-foundation.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=gustavoars@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=shengjiu.wang@nxp.com \
    --cc=tiwai@suse.com \
    --cc=tiwai@suse.de \
    --cc=xiang@kernel.org \
    --cc=yibin.gong@nxp.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.