> From: Gal Pressman <galpr...@amazon.com>
> Sent: Monday, July 29, 2019 4:54 PM
>
> On 29/07/2019 15:58, Michal Kalderon wrote:
> >> From: linux-rdma-ow...@vger.kernel.org <linux-rdma-
> >> ow...@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.
Thanks, so Jason, if we're now freeing the objects, can we simply us
xa_alloc_cyclic instead?
Thanks,
Michal
