| Message ID | 7-v2-270386b7e60b+28f4-umem_1_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | RDMA: Improve use of umem in DMA drivers | expand |
On 05/09/2020 1:41, Jason Gunthorpe wrote: > If ib_umem_find_best_pgsz() returns > PAGE_SIZE then the equation here is > not correct. 'start' should be 'virt'. Change it to use the core code for > page_num and the canonical calculation of page_shift. Should I submit a fix for stable changing start to virt? > Fixes: 40ddb3f02083 ("RDMA/efa: Use API to get contiguous memory blocks aligned to device supported page size") > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/infiniband/hw/efa/efa_verbs.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c > index d85c63a5021a70..72da0faa7ebf97 100644 > --- a/drivers/infiniband/hw/efa/efa_verbs.c > +++ b/drivers/infiniband/hw/efa/efa_verbs.c > @@ -4,6 +4,7 @@ > */ > > #include <linux/vmalloc.h> > +#include <linux/log2.h> > > #include <rdma/ib_addr.h> > #include <rdma/ib_umem.h> > @@ -1538,9 +1539,8 @@ struct ib_mr *efa_reg_mr(struct ib_pd *ibpd, u64 start, u64 length, > goto err_unmap; > } > > - params.page_shift = __ffs(pg_sz); > - params.page_num = DIV_ROUND_UP(length + (start & (pg_sz - 1)), > - pg_sz); > + params.page_shift = order_base_2(pg_sz); Not related to this patch, but indeed looks better :). > + params.page_num = ib_umem_num_dma_blocks(mr->umem, pg_sz); Thanks, Tested-by: Gal Pressman <galpress@amazon.com> Acked-by: Gal Pressman <galpress@amazon.com>
On Mon, Sep 07, 2020 at 03:19:54PM +0300, Gal Pressman wrote: > On 05/09/2020 1:41, Jason Gunthorpe wrote: > > If ib_umem_find_best_pgsz() returns > PAGE_SIZE then the equation here is > > not correct. 'start' should be 'virt'. Change it to use the core code for > > page_num and the canonical calculation of page_shift. > > Should I submit a fix for stable changing start to virt? I suspect EFA users never use ibv_reg_mr_iova() so won't have an actual bug? Thanks, Jason
On 08/09/2020 16:48, Jason Gunthorpe wrote: > On Mon, Sep 07, 2020 at 03:19:54PM +0300, Gal Pressman wrote: >> On 05/09/2020 1:41, Jason Gunthorpe wrote: >>> If ib_umem_find_best_pgsz() returns > PAGE_SIZE then the equation here is >>> not correct. 'start' should be 'virt'. Change it to use the core code for >>> page_num and the canonical calculation of page_shift. >> >> Should I submit a fix for stable changing start to virt? > > I suspect EFA users never use ibv_reg_mr_iova() so won't have an > actual bug? That's still a driver bug though, regardless of the userspace so I'd rather fix it. Should I submit a patch to for-rc? It would conflict with the for-next one.
On Wed, Sep 09, 2020 at 11:18:49AM +0300, Gal Pressman wrote: > On 08/09/2020 16:48, Jason Gunthorpe wrote: > > On Mon, Sep 07, 2020 at 03:19:54PM +0300, Gal Pressman wrote: > >> On 05/09/2020 1:41, Jason Gunthorpe wrote: > >>> If ib_umem_find_best_pgsz() returns > PAGE_SIZE then the equation here is > >>> not correct. 'start' should be 'virt'. Change it to use the core code for > >>> page_num and the canonical calculation of page_shift. > >> > >> Should I submit a fix for stable changing start to virt? > > > > I suspect EFA users never use ibv_reg_mr_iova() so won't have an > > actual bug? > > That's still a driver bug though, regardless of the userspace so I'd rather fix it. > Should I submit a patch to for-rc? It would conflict with the for-next one. If you care enough then propose the parts of this series for backporting to stable once they are merged to Linus's tree Jason
diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c index d85c63a5021a70..72da0faa7ebf97 100644 --- a/drivers/infiniband/hw/efa/efa_verbs.c +++ b/drivers/infiniband/hw/efa/efa_verbs.c @@ -4,6 +4,7 @@ */ #include <linux/vmalloc.h> +#include <linux/log2.h> #include <rdma/ib_addr.h> #include <rdma/ib_umem.h> @@ -1538,9 +1539,8 @@ struct ib_mr *efa_reg_mr(struct ib_pd *ibpd, u64 start, u64 length, goto err_unmap; } - params.page_shift = __ffs(pg_sz); - params.page_num = DIV_ROUND_UP(length + (start & (pg_sz - 1)), - pg_sz); + params.page_shift = order_base_2(pg_sz); + params.page_num = ib_umem_num_dma_blocks(mr->umem, pg_sz); ibdev_dbg(&dev->ibdev, "start %#llx length %#llx params.page_shift %u params.page_num %u\n",
If ib_umem_find_best_pgsz() returns > PAGE_SIZE then the equation here is not correct. 'start' should be 'virt'. Change it to use the core code for page_num and the canonical calculation of page_shift. Fixes: 40ddb3f02083 ("RDMA/efa: Use API to get contiguous memory blocks aligned to device supported page size") Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/infiniband/hw/efa/efa_verbs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)