Re: [PATCH rdma-rc 3/3] IB/core: Fix ODP with IB_ACCESS_HUGETLB handling

[Yishai Hadas]

On 1/8/2020 2:56 PM, Geert Uytterhoeven wrote:
>  ÂÂÂÂHi Leon,
>
> On Thu, 19 Dec 2019, Leon Romanovsky wrote:
> > From: Yishai Hadas <yishaih@xxxxxxxxxxxx>
> >
> > As VMAs for a given range might not be available as part of the
> > registration phase in ODP, IB_ACCESS_HUGETLB/page_shift must be checked
> > as part of the page fault flow.
> >
> > If the application didn't mmap the backed memory with huge pages or
> > released part of that hugepage area, an error will be set as part of the
> > page fault flow once be detected.
> >
> > Fixes: 0008b84ea9af ("IB/umem: Add support to huge ODP")
> > Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxxxx>
> > Reviewed-by: Artemy Kovalyov <artemyko@xxxxxxxxxxxx>
> > Reviewed-by: Aviad Yehezkel <aviadye@xxxxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/infiniband/core/umem_odp.c
> > +++ b/drivers/infiniband/core/umem_odp.c
> > @@ -241,22 +241,10 @@ struct ib_umem_odp *ib_umem_odp_get(struct 
> > ib_udata *udata, unsigned long addr,
> > ÂÂÂÂumem_odp->umem.owning_mm = mm = current->mm;
> > ÂÂÂÂumem_odp->notifier.ops = ops;
> >
> > -ÂÂÂ umem_odp->page_shift = PAGE_SHIFT;
> > -ÂÂÂ if (access & IB_ACCESS_HUGETLB) {
> > -ÂÂÂÂÂÂÂ struct vm_area_struct *vma;
> > -ÂÂÂÂÂÂÂ struct hstate *h;
> > -
> > -ÂÂÂÂÂÂÂ down_read(&mm->mmap_sem);
> > -ÂÂÂÂÂÂÂ vma = find_vma(mm, ib_umem_start(umem_odp));
> > -ÂÂÂÂÂÂÂ if (!vma || !is_vm_hugetlb_page(vma)) {
> > -ÂÂÂÂÂÂÂÂÂÂÂ up_read(&mm->mmap_sem);
> > -ÂÂÂÂÂÂÂÂÂÂÂ ret = -EINVAL;
> > -ÂÂÂÂÂÂÂÂÂÂÂ goto err_free;
> > -ÂÂÂÂÂÂÂ }
> > -ÂÂÂÂÂÂÂ h = hstate_vma(vma);
> > -ÂÂÂÂÂÂÂ umem_odp->page_shift = huge_page_shift(h);
> > -ÂÂÂÂÂÂÂ up_read(&mm->mmap_sem);
> > -ÂÂÂ }
> > +ÂÂÂ if (access & IB_ACCESS_HUGETLB)
> > +ÂÂÂÂÂÂÂ umem_odp->page_shift = HPAGE_SHIFT;
> > +ÂÂÂ else
> > +ÂÂÂÂÂÂÂ umem_odp->page_shift = PAGE_SHIFT;
> >
> > ÂÂÂÂumem_odp->tgid = get_task_pid(current->group_leader, PIDTYPE_PID);
> > ÂÂÂÂret = ib_init_umem_odp(umem_odp, ops);
>
> noreply@xxxxxxxxxxxxxx reports for linux-next/m68k-allmodconfig/m68k:
>
>  ÂÂÂ drivers/infiniband/core/umem_odp.c:245:26: error: 'HPAGE_SHIFT' 
> undeclared (first use in this function); did you mean 'PAGE_SHIFT'?
>
> Should this depend on some HUGETLBFS option?

Thanks for pointing on,
We would expect to use #ifdef CONFIG_HUGETLB_PAGE as done in below 
kernel code [1] that also used HPAGE_SHIFT.

I'll send some patch to 'for-next' to handle it.

[1] 
https://elixir.bootlin.com/linux/v5.3-rc7/source/drivers/misc/sgi-gru/grufault.c#L183