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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d632598e-0896-fa10-9148-73794a9a49d7@amazon.com>
Date:   Mon, 29 Jul 2019 16:53:38 +0300
From:   Gal Pressman <galpress@...zon.com>
To:     Michal Kalderon <mkalderon@...vell.com>,
        Jason Gunthorpe <jgg@...pe.ca>
CC:     Ariel Elior <aelior@...vell.com>,
        "dledford@...hat.com" <dledford@...hat.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v6 rdma-next 1/6] RDMA/core: Create mmap database and
 cookie helper functions

On 29/07/2019 15:58, Michal Kalderon wrote:
>> From: linux-rdma-owner@...r.kernel.org <linux-rdma-
>> owner@...r.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.