lib/genalloc

lib/genalloc

[Alexey Skidanov]

Hi,

I use gen_pool_first_fit_align() as pool allocation algorithm allocating
buffers with requested alignment. But if a chunk base address is not
aligned to the requested alignment(from some reason), the returned
address is not aligned too.

The reason is the allocation algorithm works on bitmap, omitting the
base address. Is this behavior by design?

Thanks,
Alexey

Re: lib/genalloc

[Stephen Bates]

>    I use gen_pool_first_fit_align() as pool allocation algorithm allocating
>    buffers with requested alignment. But if a chunk base address is not
>    aligned to the requested alignment(from some reason), the returned
>    address is not aligned too.
    
Alexey

Can you try using gen_pool_first_fit_order_align()? Will that give you the alignment you need?

Stephen
    

Re: lib/genalloc

[Alexey Skidanov]

On 11/1/18 18:48, Stephen  Bates wrote:
>>    I use gen_pool_first_fit_align() as pool allocation algorithm allocating
>>    buffers with requested alignment. But if a chunk base address is not
>>    aligned to the requested alignment(from some reason), the returned
>>    address is not aligned too.
>     
> Alexey
> 
> Can you try using gen_pool_first_fit_order_align()? Will that give you the alignment you need?
> 
> Stephen
>     
> 
I think it will not help me. Let's assume that the chunk base address is
0x2F400000 and I want to allocate 16MB aligned buffer. I get back the
0x2F400000. I think it happens because of this string in the
gen_pool_alloc_algo():

addr = chunk->start_addr + ((unsigned long)start_bit << order);

and the gen_pool_first_fit_align() implementation that doesn't take into
account the "incorrect" chunk base alignment.

It might be easily fixed, by rounding the start address up (0x2F400000
-> 0x30000000) and start looking from this, aligned, address.

Thanks,
Alexey

Re: lib/genalloc

[Daniel Mentz]

On Thu, Nov 1, 2018 at 10:07 AM Alexey Skidanov
<alexey.skidanov@intel.com> wrote:
> On 11/1/18 18:48, Stephen  Bates wrote:
> >>    I use gen_pool_first_fit_align() as pool allocation algorithm allocating
> >>    buffers with requested alignment. But if a chunk base address is not
> >>    aligned to the requested alignment(from some reason), the returned
> >>    address is not aligned too.
> >
> > Alexey
> >
> > Can you try using gen_pool_first_fit_order_align()? Will that give you the alignment you need?
> >
> > Stephen
> >
> >
> I think it will not help me. Let's assume that the chunk base address is
> 0x2F400000 and I want to allocate 16MB aligned buffer. I get back the
> 0x2F400000. I think it happens because of this string in the
> gen_pool_alloc_algo():
>
> addr = chunk->start_addr + ((unsigned long)start_bit << order);
>
> and the gen_pool_first_fit_align() implementation that doesn't take into
> account the "incorrect" chunk base alignment.

gen_pool_first_fit_align() has no information about the chunk base
alignment. Hence, it can't take it into account.

How do you request the alignment in your code?

I agree with your analysis that gen_pool_first_fit_align() performs
alignment only with respect to the start of the chunk not the memory
address that gen_pool_alloc_algo() returns. I guess a solution would
be to only add chunks that satisfy all your alignment requirements. In
your case, you must only add chunks that are 16MB aligned.
I am unsure whether this is by design, but I believe it's the way that
the code currently works.

Re: lib/genalloc

[Alexey Skidanov]

On 11/2/18 9:17 PM, Daniel Mentz wrote:
> On Thu, Nov 1, 2018 at 10:07 AM Alexey Skidanov
> <alexey.skidanov@intel.com> wrote:
>> On 11/1/18 18:48, Stephen  Bates wrote:
>>>>    I use gen_pool_first_fit_align() as pool allocation algorithm allocating
>>>>    buffers with requested alignment. But if a chunk base address is not
>>>>    aligned to the requested alignment(from some reason), the returned
>>>>    address is not aligned too.
>>>
>>> Alexey
>>>
>>> Can you try using gen_pool_first_fit_order_align()? Will that give you the alignment you need?
>>>
>>> Stephen
>>>
>>>
>> I think it will not help me. Let's assume that the chunk base address is
>> 0x2F400000 and I want to allocate 16MB aligned buffer. I get back the
>> 0x2F400000. I think it happens because of this string in the
>> gen_pool_alloc_algo():
>>
>> addr = chunk->start_addr + ((unsigned long)start_bit << order);
>>
>> and the gen_pool_first_fit_align() implementation that doesn't take into
>> account the "incorrect" chunk base alignment.
> 
> gen_pool_first_fit_align() has no information about the chunk base
> alignment. Hence, it can't take it into account.
> 
> How do you request the alignment in your code?
> 
> I agree with your analysis that gen_pool_first_fit_align() performs
> alignment only with respect to the start of the chunk not the memory
> address that gen_pool_alloc_algo() returns. I guess a solution would
> be to only add chunks that satisfy all your alignment requirements. In
> your case, you must only add chunks that are 16MB aligned.
> I am unsure whether this is by design, but I believe it's the way that
> the code currently works.
> 

Daniel,

I think the better solution is to use bitmap_find_next_zero_area_off()
that receives the bit offset (CMA allocator uses it to solve the same
issue). Of course, we need to pass the chunk base address to the
gen_pool_first_fit_align().

What do you think?

Thanks,
Alexey

Re: lib/genalloc

[Daniel Mentz]

On Fri, Nov 2, 2018 at 1:55 PM Alexey Skidanov
<alexey.skidanov@intel.com> wrote:
>
>
>
> On 11/2/18 9:17 PM, Daniel Mentz wrote:
> > On Thu, Nov 1, 2018 at 10:07 AM Alexey Skidanov
> > <alexey.skidanov@intel.com> wrote:
> >> On 11/1/18 18:48, Stephen  Bates wrote:
> >>>>    I use gen_pool_first_fit_align() as pool allocation algorithm allocating
> >>>>    buffers with requested alignment. But if a chunk base address is not
> >>>>    aligned to the requested alignment(from some reason), the returned
> >>>>    address is not aligned too.
> >>>
> >>> Alexey
> >>>
> >>> Can you try using gen_pool_first_fit_order_align()? Will that give you the alignment you need?
> >>>
> >>> Stephen
> >>>
> >>>
> >> I think it will not help me. Let's assume that the chunk base address is
> >> 0x2F400000 and I want to allocate 16MB aligned buffer. I get back the
> >> 0x2F400000. I think it happens because of this string in the
> >> gen_pool_alloc_algo():
> >>
> >> addr = chunk->start_addr + ((unsigned long)start_bit << order);
> >>
> >> and the gen_pool_first_fit_align() implementation that doesn't take into
> >> account the "incorrect" chunk base alignment.
> >
> > gen_pool_first_fit_align() has no information about the chunk base
> > alignment. Hence, it can't take it into account.
> >
> > How do you request the alignment in your code?
> >
> > I agree with your analysis that gen_pool_first_fit_align() performs
> > alignment only with respect to the start of the chunk not the memory
> > address that gen_pool_alloc_algo() returns. I guess a solution would
> > be to only add chunks that satisfy all your alignment requirements. In
> > your case, you must only add chunks that are 16MB aligned.
> > I am unsure whether this is by design, but I believe it's the way that
> > the code currently works.
> >
>
> Daniel,
>
> I think the better solution is to use bitmap_find_next_zero_area_off()
> that receives the bit offset (CMA allocator uses it to solve the same
> issue). Of course, we need to pass the chunk base address to the
> gen_pool_first_fit_align().
>
> What do you think?

Yeah, I guess you could extend genpool_algo_t to include the
information you need i.e. the offset and then provide a modified
version of gen_pool_first_fit_align() that does take your offset into
account. I wouldn't change gen_pool_first_fit_align(), though, because
existing users might depend on the current behavior.

Re: lib/genalloc

[Alexey Skidanov]

On 11/2/18 11:16 PM, Daniel Mentz wrote:
> On Fri, Nov 2, 2018 at 1:55 PM Alexey Skidanov
> <alexey.skidanov@intel.com> wrote:
>>
>>
>>
>> On 11/2/18 9:17 PM, Daniel Mentz wrote:
>>> On Thu, Nov 1, 2018 at 10:07 AM Alexey Skidanov
>>> <alexey.skidanov@intel.com> wrote:
>>>> On 11/1/18 18:48, Stephen  Bates wrote:
>>>>>>    I use gen_pool_first_fit_align() as pool allocation algorithm allocating
>>>>>>    buffers with requested alignment. But if a chunk base address is not
>>>>>>    aligned to the requested alignment(from some reason), the returned
>>>>>>    address is not aligned too.
>>>>>
>>>>> Alexey
>>>>>
>>>>> Can you try using gen_pool_first_fit_order_align()? Will that give you the alignment you need?
>>>>>
>>>>> Stephen
>>>>>
>>>>>
>>>> I think it will not help me. Let's assume that the chunk base address is
>>>> 0x2F400000 and I want to allocate 16MB aligned buffer. I get back the
>>>> 0x2F400000. I think it happens because of this string in the
>>>> gen_pool_alloc_algo():
>>>>
>>>> addr = chunk->start_addr + ((unsigned long)start_bit << order);
>>>>
>>>> and the gen_pool_first_fit_align() implementation that doesn't take into
>>>> account the "incorrect" chunk base alignment.
>>>
>>> gen_pool_first_fit_align() has no information about the chunk base
>>> alignment. Hence, it can't take it into account.
>>>
>>> How do you request the alignment in your code?
>>>
>>> I agree with your analysis that gen_pool_first_fit_align() performs
>>> alignment only with respect to the start of the chunk not the memory
>>> address that gen_pool_alloc_algo() returns. I guess a solution would
>>> be to only add chunks that satisfy all your alignment requirements. In
>>> your case, you must only add chunks that are 16MB aligned.
>>> I am unsure whether this is by design, but I believe it's the way that
>>> the code currently works.
>>>
>>
>> Daniel,
>>
>> I think the better solution is to use bitmap_find_next_zero_area_off()
>> that receives the bit offset (CMA allocator uses it to solve the same
>> issue). Of course, we need to pass the chunk base address to the
>> gen_pool_first_fit_align().
>>
>> What do you think?
> 
> Yeah, I guess you could extend genpool_algo_t to include the
> information you need i.e. the offset and then provide a modified
> version of gen_pool_first_fit_align() that does take your offset into
> account. I wouldn't change gen_pool_first_fit_align(), though, because
> existing users might depend on the current behavior.
> 
I think that the "fixed" version of gen_pool_first_fit_align() is less
restrictive with respect to chunk base address - it will work correctly
with arbitrary aligned chunks.

Thanks,
Alexey