> > > > > +
> > > > > + 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?
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.
Jason
