> From: Gal Pressman <galpress@xxxxxxxxxx>
> Sent: Sunday, August 25, 2019 1:45 PM
>
> On 25/08/2019 11:41, Michal Kalderon wrote:
> >>> @@ -515,46 +408,55 @@ static int qp_mmap_entries_setup(struct
> efa_qp
> >> *qp,
> >>> struct efa_com_create_qp_params
> >> *params,
> >>> struct efa_ibv_create_qp_resp *resp) {
> >>> + u64 address;
> >>> + u64 length;
> >>> +
> >>> /*
> >>> * Once an entry is inserted it might be mmapped, hence cannot be
> >>> * cleaned up until dealloc_ucontext.
> >>> */
> >>> resp->sq_db_mmap_key =
> >>
> >> Not a big deal, but now it makes more sense to assign qp-
> >>> sq_db_mmap_key and assign the response later on.
> > ok
> >>
> >>> - mmap_entry_insert(dev, ucontext, qp,
> >>> - dev->db_bar_addr + resp->sq_db_offset,
> >>> - PAGE_SIZE, EFA_MMAP_IO_NC);
> >>> - if (resp->sq_db_mmap_key == EFA_MMAP_INVALID)
> >>> + rdma_user_mmap_entry_insert(&ucontext->ibucontext, qp,
> >>> + dev->db_bar_addr +
> >>> + resp->sq_db_offset,
> >>> + PAGE_SIZE, EFA_MMAP_IO_NC);
> >>> + if (resp->sq_db_mmap_key == RDMA_USER_MMAP_INVALID)
> >>> return -ENOMEM;
> >>> -
> >>> + qp->sq_db_mmap_key = resp->sq_db_mmap_key;
> >>> resp->sq_db_offset &= ~PAGE_MASK;
> >>>
> >>> + address = dev->mem_bar_addr + resp->llq_desc_offset;
> >>> + length = PAGE_ALIGN(params->sq_ring_size_in_bytes +
> >>> + (resp->llq_desc_offset & ~PAGE_MASK));
> >>> resp->llq_desc_mmap_key =
> >>> - mmap_entry_insert(dev, ucontext, qp,
> >>> - dev->mem_bar_addr + resp-
> >>> llq_desc_offset,
> >>> - PAGE_ALIGN(params-
> >>> sq_ring_size_in_bytes +
> >>> - (resp->llq_desc_offset &
> >> ~PAGE_MASK)),
> >>> - EFA_MMAP_IO_WC);
> >>> - if (resp->llq_desc_mmap_key == EFA_MMAP_INVALID)
> >>> + rdma_user_mmap_entry_insert(&ucontext->ibucontext, qp,
> >>> + address,
> >>> + length,
> >>> + EFA_MMAP_IO_WC);
> >>> + if (resp->llq_desc_mmap_key == RDMA_USER_MMAP_INVALID)
> >>> return -ENOMEM;
> >>> -
> >>> + qp->llq_desc_mmap_key = resp->llq_desc_mmap_key;
> >>> resp->llq_desc_offset &= ~PAGE_MASK;
> >>>
> >>> if (qp->rq_size) {
> >>> + address = dev->db_bar_addr + resp->rq_db_offset;
> >>> resp->rq_db_mmap_key =
> >>> - mmap_entry_insert(dev, ucontext, qp,
> >>> - dev->db_bar_addr + resp-
> >>> rq_db_offset,
> >>> - PAGE_SIZE, EFA_MMAP_IO_NC);
> >>> - if (resp->rq_db_mmap_key == EFA_MMAP_INVALID)
> >>> + rdma_user_mmap_entry_insert(&ucontext-
> >>> ibucontext, qp,
> >>> + address, PAGE_SIZE,
> >>> + EFA_MMAP_IO_NC);
> >>> + if (resp->rq_db_mmap_key ==
> >> RDMA_USER_MMAP_INVALID)
> >>> return -ENOMEM;
> >>> -
> >>> + qp->rq_db_mmap_key = resp->rq_db_mmap_key;
> >>> resp->rq_db_offset &= ~PAGE_MASK;
> >>>
> >>> + address = virt_to_phys(qp->rq_cpu_addr);
> >>> resp->rq_mmap_key =
> >>> - mmap_entry_insert(dev, ucontext, qp,
> >>> - virt_to_phys(qp->rq_cpu_addr),
> >>> - qp->rq_size,
> >> EFA_MMAP_DMA_PAGE);
> >>> - if (resp->rq_mmap_key == EFA_MMAP_INVALID)
> >>> + rdma_user_mmap_entry_insert(&ucontext-
> >>> ibucontext, qp,
> >>> + address, qp->rq_size,
> >>> + EFA_MMAP_DMA_PAGE);
> >>> + if (resp->rq_mmap_key == RDMA_USER_MMAP_INVALID)
> >>> return -ENOMEM;
> >>> + qp->rq_mmap_key = resp->rq_mmap_key;
> >>>
> >>> resp->rq_mmap_size = qp->rq_size;
> >>> }
> >>> @@ -775,6 +677,9 @@ struct ib_qp *efa_create_qp(struct ib_pd *ibpd,
> >>> DMA_TO_DEVICE);
> >>> if (!rq_entry_inserted)
> >>
> >> Now that we store the keys on the QP object we can remove the
> >> rq_entry_inserted variable and test for !qp->rq_mmap_key.
> > ok
> >>
> >>> free_pages_exact(qp->rq_cpu_addr, qp->rq_size);
> >>> + else
> >>> + rdma_user_mmap_entry_remove(&ucontext-
> >>> ibucontext,
> >>> + qp->rq_mmap_key);
> >>
> >> Other entries need to be removed as well, otherwise the refcount
> >> won't reach zero. This error flow should now be similar to
> >> efa_destroy_qp. I think that means losing the free_pages_exact too.
> > Not sure I understand, how can we loose the free_pages_exact ? if the
> > entry wasn’t Inserted into the mmap_xa what flow will free the pages ?
>
> You're right.
> Still need to remove other entries though.
sure, thanks
