> From: Jason Gunthorpe <jgg@xxxxxxxx>
> > > > > > +
> > > > > > + if (force_zero_offset) {
> > > > > > + while (ib_umem_dma_offset(umem, page_sz) && page_sz
> > > > > > + >
> > > > > > PAGE_SIZE)
> > > > > > + page_sz /= 2;
> > > > > > + if (ib_umem_dma_offset(umem, page_sz) != 0) {
> > > > > > + ibdev_dbg(&dev->ib_dev, "failed to find page
> > > > > > + size to
> > > > > > force zero offset.\n");
> > > > > > + return -ENOMEM;
> > > > > > + }
> > > > > > + }
> > > > > > +
> > >
> > > Yes this doesn't look quite right..
> > >
> > > It should flow from the HW capability, the helper you call should be
> > > tightly linked to what the HW can do.
> > >
> > > ib_umem_find_best_pgsz() is used for MRs that have the usual
> > > offset = IOVA % pgsz
> > >
> > > We've always created other helpers for other restrictions.
> > >
> > > So you should move your "force_zero_offset" into another helper and
> > > describe exactly how the HW works to support the calculation
> > >
> > > It is odd to have the offset loop and be using
> > > ib_umem_find_best_pgsz() with some iova, usually you'd use
> > > ib_umem_find_best_pgoff() in those cases, see the other callers.
> >
> > Hi Jason,
> > Thanks for the comments.
> >
> > To be honest, I do not understand how I could employ
> > ib_umem_find_best_pgoff for my purpose. As well as I do not see any
> mistake in the patch, and I think you neither.
>
> It does exactly the same thing, it is just intended to be used by things that
> are not doing the IOVA calculation. It is a matter of documentation.
>
> > I can make a special helper, but I do not think that it will be useful
> > to anyone. Plus, there is no better approach then halving the page
> > size, so the helper will end up with that loop under the hood. As I
> > see mlnx also uses a loop with halving page_sz, but for a different purpose,
> I do not see why our code cannot do the same without a special helper.
>
> Are you sure you don't need the length check too? You have a granular size
> but not a granular offset?
Yes, we do not have constraints on the size.
>
> In that case yes, a helper does not seem necessary
>
> However, you should still be calling ib_umem_find_best_pgoff() for the
> initialize sizing as a matter of clarity since this is not a MR and does not use
> IOVA addressing.
Thanks for the clarification! I agree that the use of ib_umem_find_best_pgoff()
will make the code more understandable, even though it will do the same computation.
I have already prepared the patch. I will send it next week after running tests.
>
> Jason
