Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and cookie helper functions

[Gal Pressman <galpress@amazon.com>]

On 29/07/2019 15:58, Michal Kalderon wrote:
>> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
>> owner@vger.kernel.org> On Behalf Of Jason Gunthorpe
>>
>>> +	xa_lock(&ucontext->mmap_xa);
>>> +	if (check_add_overflow(ucontext->mmap_xa_page,
>>> +			       (u32)(length >> PAGE_SHIFT),
>>> +			       &next_mmap_page))
>>> +		goto err_unlock;
>>
>> I still don't like that this algorithm latches into a permanent failure when the
>> xa_page wraps.
>>
>> It seems worth spending a bit more time here to tidy this.. Keep using the
>> mmap_xa_page scheme, but instead do something like
>>
>> alloc_cyclic_range():
>>
>> while () {
>>    // Find first empty element in a cyclic way
>>    xa_page_first = mmap_xa_page;
>>    xa_find(xa, &xa_page_first, U32_MAX, XA_FREE_MARK)
>>
>>    // Is there a enough room to have the range?
>>    if (check_add_overflow(xa_page_first, npages, &xa_page_end)) {
>>       mmap_xa_page = 0;
>>       continue;
>>    }
>>
>>    // See if the element before intersects
>>    elm = xa_find(xa, &zero, xa_page_end, 0);
>>    if (elm && intersects(xa_page_first, xa_page_last, elm->first, elm->last)) {
>>       mmap_xa_page = elm->last + 1;
>>       continue
>>    }
>>
>>    // xa_page_first -> xa_page_end should now be free
>>    xa_insert(xa, xa_page_start, entry);
>>    mmap_xa_page = xa_page_end + 1;
>>    return xa_page_start;
>> }
>>
>> Approximately, please check it.
> Gal & Jason, 
> 
> Coming back to the mmap_xa_page algorithm. I couldn't find some background on this. 
> Why do you need the length to be represented in the mmap_xa_page ?  
> Why not simply use xa_alloc_cyclic ( like in siw ) 
> This is simply a key to a mmap object... 

The intention was that the entry would "occupy" number of xarray elements
according to its size (in pages). It wasn't initially like this, but IIRC this
was preferred by Jason.