Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: lib/genalloc
To:     Daniel Mentz <danielmentz@google.com>
Cc:     Stephen Bates <sbates@raithlin.com>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.co>,
        lkml <linux-kernel@vger.kernel.org>, labbott@redhat.com
References: <a170cf65-6884-3592-1de9-4c235888cc8a@intel.com>
 <4DECD467-AD45-419D-8F0F-1456863274FD@raithlin.com>
 <d127f28f-71fe-c175-c0e4-a368ff1d01a1@intel.com>
 <CAE2F3rBifp4y=WQFC6CTcYtPUFyvLD4ZZMrZSzu4P+Fzot_f0A@mail.gmail.com>
 <de2e1756-329b-7df2-559e-a603a7edc2ff@intel.com>
 <CAE2F3rDm1X-UuWHoJf9wAz3HJkC_LQP+UMq81Opu6te7KH704Q@mail.gmail.com>
From:   Alexey Skidanov <alexey.skidanov@intel.com>
Message-ID: <7a3eb4d7-2a93-8a39-c696-675053e53c53@intel.com>
Date:   Sat, 3 Nov 2018 00:18:22 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.2.1
MIME-Version: 1.0
In-Reply-To: <CAE2F3rDm1X-UuWHoJf9wAz3HJkC_LQP+UMq81Opu6te7KH704Q@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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