2014-11-04 21:03:55

by Gregory Fong

[permalink] [raw]
Subject: CMA alignment question

Hi all,

The alignment in cma_alloc() is done w.r.t. the bitmap. This is a
problem when, for example:

- a device requires 16M (order 12) alignment
- the CMA region is not 16 M aligned

In such a case, can result with the CMA region starting at, say,
0x2f800000 but any allocation you make from there will be aligned from
there. Requesting an allocation of 32 M with 16 M alignment, will
result in an allocation from 0x2f800000 to 0x31800000, which doesn't
work very well if your strange device requires 16M alignment.

This doesn't have the behavior I would expect, which would be for the
allocation to be aligned w.r.t. the start of memory. I realize that
aligning the CMA region is an option, but don't see why cma_alloc()
aligns to the start of the CMA region. Is there a good reason for
having cma_alloc() alignment work this way?

Thanks and regards,
Gregory


2014-11-04 22:27:36

by Michal Nazarewicz

[permalink] [raw]
Subject: Re: CMA alignment question

On Tue, Nov 04 2014, Gregory Fong wrote:
> The alignment in cma_alloc() is done w.r.t. the bitmap. This is a
> problem when, for example:
>
> - a device requires 16M (order 12) alignment
> - the CMA region is not 16 M aligned
>
> In such a case, can result with the CMA region starting at, say,
> 0x2f800000 but any allocation you make from there will be aligned from
> there. Requesting an allocation of 32 M with 16 M alignment, will
> result in an allocation from 0x2f800000 to 0x31800000, which doesn't
> work very well if your strange device requires 16M alignment.
>
> This doesn't have the behavior I would expect, which would be for the
> allocation to be aligned w.r.t. the start of memory. I realize that
> aligning the CMA region is an option, but don't see why cma_alloc()
> aligns to the start of the CMA region. Is there a good reason for
> having cma_alloc() alignment work this way?

No, it's a bug. The alignment should indicate alignment of physical
address not position in CMA region.

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--

2014-11-05 04:19:30

by Gregory Fong

[permalink] [raw]
Subject: Re: CMA alignment question

On Tue, Nov 4, 2014 at 2:27 PM, Michal Nazarewicz <[email protected]> wrote:
> On Tue, Nov 04 2014, Gregory Fong wrote:
>> The alignment in cma_alloc() is done w.r.t. the bitmap. This is a
>> problem when, for example:
>>
>> - a device requires 16M (order 12) alignment
>> - the CMA region is not 16 M aligned
>>
>> In such a case, can result with the CMA region starting at, say,
>> 0x2f800000 but any allocation you make from there will be aligned from
>> there. Requesting an allocation of 32 M with 16 M alignment, will
>> result in an allocation from 0x2f800000 to 0x31800000, which doesn't
>> work very well if your strange device requires 16M alignment.
>>
>> This doesn't have the behavior I would expect, which would be for the
>> allocation to be aligned w.r.t. the start of memory. I realize that
>> aligning the CMA region is an option, but don't see why cma_alloc()
>> aligns to the start of the CMA region. Is there a good reason for
>> having cma_alloc() alignment work this way?
>
> No, it's a bug. The alignment should indicate alignment of physical
> address not position in CMA region.
>

Ah, now I see that Marek submitted this patch from you back in 2011
that would have allowed the bitmap lib to support an alignment offset:
http://thread.gmane.org/gmane.linux.kernel/1121103/focus=1121100

Any idea why this didn't make it into the later changesets? If not,
I'll resubmit it and to use it to fix this bug.

Thanks,
Gregory

2014-11-05 07:20:15

by Weijie Yang

[permalink] [raw]
Subject: Re: CMA alignment question

On Wed, Nov 5, 2014 at 12:18 PM, Gregory Fong <[email protected]> wrote:
> On Tue, Nov 4, 2014 at 2:27 PM, Michal Nazarewicz <[email protected]> wrote:
>> On Tue, Nov 04 2014, Gregory Fong wrote:
>>> The alignment in cma_alloc() is done w.r.t. the bitmap. This is a
>>> problem when, for example:
>>>
>>> - a device requires 16M (order 12) alignment
>>> - the CMA region is not 16 M aligned

I think the device driver should ensure that situation could not occur,
by assign suitable alignment parameter in cma_declare_contiguous().

>>> In such a case, can result with the CMA region starting at, say,
>>> 0x2f800000 but any allocation you make from there will be aligned from
>>> there. Requesting an allocation of 32 M with 16 M alignment, will
>>> result in an allocation from 0x2f800000 to 0x31800000, which doesn't
>>> work very well if your strange device requires 16M alignment.
>>>
>>> This doesn't have the behavior I would expect, which would be for the
>>> allocation to be aligned w.r.t. the start of memory. I realize that
>>> aligning the CMA region is an option, but don't see why cma_alloc()
>>> aligns to the start of the CMA region. Is there a good reason for
>>> having cma_alloc() alignment work this way?
>>
>> No, it's a bug. The alignment should indicate alignment of physical
>> address not position in CMA region.
>>
>
> Ah, now I see that Marek submitted this patch from you back in 2011
> that would have allowed the bitmap lib to support an alignment offset:
> http://thread.gmane.org/gmane.linux.kernel/1121103/focus=1121100
>
> Any idea why this didn't make it into the later changesets? If not,
> I'll resubmit it and to use it to fix this bug.
>
> Thanks,
> Gregory
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2014-11-05 22:01:56

by Michal Nazarewicz

[permalink] [raw]
Subject: Re: CMA alignment question

> On Tue, Nov 04 2014, Gregory Fong wrote:
>> The alignment in cma_alloc() is done w.r.t. the bitmap. This is a
>> problem when, for example:
>>
>> - a device requires 16M (order 12) alignment
>> - the CMA region is not 16 M aligned

On Wed, Nov 05 2014, Weijie Yang wrote:
> I think the device driver should ensure that situation could not occur,
> by assign suitable alignment parameter in cma_declare_contiguous().

What about default CMA area? Besides, I think principle of least
surprise applies here and alignment should be physical.

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--

2014-11-06 01:00:35

by Weijie Yang

[permalink] [raw]
Subject: Re: CMA alignment question

On Thu, Nov 6, 2014 at 6:01 AM, Michal Nazarewicz <[email protected]> wrote:
>> On Tue, Nov 04 2014, Gregory Fong wrote:
>>> The alignment in cma_alloc() is done w.r.t. the bitmap. This is a
>>> problem when, for example:
>>>
>>> - a device requires 16M (order 12) alignment
>>> - the CMA region is not 16 M aligned
>
> On Wed, Nov 05 2014, Weijie Yang wrote:
>> I think the device driver should ensure that situation could not occur,
>> by assign suitable alignment parameter in cma_declare_contiguous().
>
> What about default CMA area? Besides, I think principle of least
> surprise applies here and alignment should be physical.

I agree the current code doesn't handle this issue properly.
However, I prefer to add specific usage to CMA interface rather than
modify the cma code, Because the latter hide the issue and could waste
memory.

> --
> Best regards, _ _
> .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
> ..o | Computer Science, Michał “mina86” Nazarewicz (o o)
> ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--

2014-11-06 12:29:51

by Michal Nazarewicz

[permalink] [raw]
Subject: Re: CMA alignment question

On Thu, Nov 06 2014, Weijie Yang <[email protected]> wrote:
> I agree the current code doesn't handle this issue properly.
> However, I prefer to add specific usage to CMA interface rather than
> modify the cma code, Because the latter hide the issue and could waste
> memory.

cma_alloc should handle whatever alignment caller uses. Sure, if CMA
area has smaller alignment this may lead to wasted memory, but so can
allocation with small alignment followed by allocation with big
alignment.

If you're saying that platform should try to get the CMA area aligned
such that no alignment offset happens I agree. If you're saying that
cma_alloc should fail (to properly align) an allocation request,
I disagree.

--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<[email protected]>--<xmpp:[email protected]>--ooO--(_)--Ooo--