On Mon, Aug 29, 2016 at 05:08:46 -0700, Yuval Shaia wrote:
> On Wed, Aug 03, 2016 at 04:27:35PM -0700, Adit Ranadive wrote:
> > This patch adds helper functions to store guest page addresses in a
> > page directory structure. The page directory pointer is passed down to
> > the backend which then maps the entire memory for the RDMA object by
> > traversing the directory. We add some more helper functions for
> > converting to/from RDMA stack address handles from/to PVRDMA ones.
> >
> > Reviewed-by: Jorgen Hansen <jhansen@xxxxxxxxxx>
> > Reviewed-by: George Zhang <georgezhang@xxxxxxxxxx>
> > Reviewed-by: Aditya Sarwade <asarwade@xxxxxxxxxx>
> > Reviewed-by: Bryan Tan <bryantan@xxxxxxxxxx>
> > Signed-off-by: Adit Ranadive <aditr@xxxxxxxxxx>
> > ---
> > drivers/infiniband/hw/pvrdma/pvrdma_misc.c | 309
> > +++++++++++++++++++++++++++++
> > 1 file changed, 309 insertions(+)
> > create mode 100644 drivers/infiniband/hw/pvrdma/pvrdma_misc.c
> >
...
> > +int pvrdma_page_dir_init(struct pvrdma_dev *dev, struct
> pvrdma_page_dir *pdir,
> > + u64 npages, bool alloc_pages)
> > +{
> > + u64 i;
> > +
> > + if (npages > PVRDMA_PAGE_DIR_MAX_PAGES)
> > + return -EINVAL;
> > +
> > + memset(pdir, 0, sizeof(*pdir));
> > +
> > + pdir->dir = dma_alloc_coherent(&dev->pdev->dev, PAGE_SIZE,
> > + &pdir->dir_dma, GFP_KERNEL);
> > + if (!pdir->dir)
> > + goto err;
> > +
> > + pdir->ntables = PVRDMA_PAGE_DIR_TABLE(npages - 1) + 1;
> > + pdir->tables = kcalloc(pdir->ntables, sizeof(*pdir->tables),
> > + GFP_KERNEL);
> > + if (!pdir->tables)
> > + goto err;
> > +
> > + for (i = 0; i < pdir->ntables; i++) {
> > + u64 *table;
> > + dma_addr_t table_base;
> > +
> > + table = dma_alloc_coherent(&dev->pdev->dev, PAGE_SIZE,
> > + &table_base, GFP_KERNEL);
> > + if (!table)
> > + goto err;
> > +
> > + pdir->tables[i] = table;
> > + pdir->dir[i] = table_base;
>
> Suggesting:
> pdir->tables[i] = dma_alloc_coherent(&dev->pdev->dev,
> PAGE_SIZE,
> &pdir->dir[i],
> GFP_KERNEL);
> if (!pdir->tables[i])
> goto err;
>
> And get rid of the scope variables 'table' and table_base.
Done. Thanks for the suggestion.
> > + }
> > +
> > + pdir->npages = npages;
> > +
> > + if (alloc_pages) {
> > + pdir->pages = kcalloc(npages, sizeof(*pdir->pages),
> > + GFP_KERNEL);
> > + if (!pdir->pages)
> > + goto err;
> > +
> > + for (i = 0; i < pdir->npages; i++) {
> > + void *page;
> > + dma_addr_t page_dma;
> > +
> > + page = dma_alloc_coherent(&dev->pdev->dev,
> PAGE_SIZE,
> > + &page_dma, GFP_KERNEL);
> > + if (!page)
> > + goto err;
> > +
> > + pdir->pages[i] = page;
>
> Ditto.
>
> > + pvrdma_page_dir_insert_dma(pdir, i, page_dma);
> > + }
> > + }
> > +
> > + return 0;
> > +
> > +err:
> > + pvrdma_page_dir_cleanup(dev, pdir);
> > +
> > + return -ENOMEM;
> > +}
> > +
> > +static u64 *pvrdma_page_dir_table(struct pvrdma_page_dir *pdir, u64
> > +idx) {
> > + return pdir->tables[PVRDMA_PAGE_DIR_TABLE(idx)];
> > +}
> > +
> > +dma_addr_t pvrdma_page_dir_get_dma(struct pvrdma_page_dir *pdir,
> u64
> > +idx) {
> > + return pvrdma_page_dir_table(pdir,
> idx)[PVRDMA_PAGE_DIR_PAGE(idx)];
> > +}
> > +
> > +static void pvrdma_page_dir_cleanup_pages(struct pvrdma_dev *dev,
> > + struct pvrdma_page_dir *pdir)
> > +{
> > + if (pdir->pages) {
> > + u64 i;
> > +
> > + for (i = 0; i < pdir->npages && pdir->pages[i]; i++) {
> > + dma_addr_t page_dma =
> pvrdma_page_dir_get_dma(pdir, i);
> > +
> > + dma_free_coherent(&dev->pdev->dev, PAGE_SIZE,
> > + pdir->pages[i], page_dma);
> > + }
> > +
> > + kfree(pdir->pages);
> > + }
> > +}
> > +
> > +static void pvrdma_page_dir_cleanup_tables(struct pvrdma_dev *dev,
> > + struct pvrdma_page_dir *pdir) {
> > + if (pdir->tables) {
> > + int i;
> > +
> > + pvrdma_page_dir_cleanup_pages(dev, pdir);
> > +
> > + for (i = 0; i < pdir->ntables; i++) {
> > + u64 *table = pdir->tables[i];
> > +
> > + if (table)
> > + dma_free_coherent(&dev->pdev->dev,
> PAGE_SIZE,
> > + table, pdir->dir[i]);
> > + }
> > +
> > + kfree(pdir->tables);
> > + }
> > +}
> > +
> > +void pvrdma_page_dir_cleanup(struct pvrdma_dev *dev,
> > + struct pvrdma_page_dir *pdir) {
> > + if (pdir->dir) {
> > + pvrdma_page_dir_cleanup_tables(dev, pdir);
> > + dma_free_coherent(&dev->pdev->dev, PAGE_SIZE,
> > + pdir->dir, pdir->dir_dma);
> > + }
> > +}
> > +
> > +int pvrdma_page_dir_insert_dma(struct pvrdma_page_dir *pdir, u64 idx,
> > + dma_addr_t daddr)
> > +{
> > + u64 *table;
> > +
> > + if (idx >= pdir->npages)
> > + return -EINVAL;
> > +
> > + table = pvrdma_page_dir_table(pdir, idx);
> > + table[PVRDMA_PAGE_DIR_PAGE(idx)] = daddr;
>
> Do you think it worth to unmap what is there before?
I don't think so. These structures are used underneath a QP, CQ, MR to keep
track of already mapped application pages. Instead of passing the entire set of
pages representing these RDMA constructs we create a directory containing the
page addresses and pass only a pointer to the directory to the device. The directory
would be unmapped when the destroy verb is called.
> > +
> > + return 0;
> > +}
> > +
> > +int pvrdma_page_dir_insert_umem(struct pvrdma_page_dir *pdir,
> > + struct ib_umem *umem, u64 offset) {
> > + u64 i = offset;
> > + int j, entry;
> > + int ret = 0, len = 0;
> > + struct scatterlist *sg;
> > +
> > + if (offset >= pdir->npages)
> > + return -EINVAL;
> > +
> > + for_each_sg(umem->sg_head.sgl, sg, umem->nmap, entry) {
> > + len = sg_dma_len(sg) >> PAGE_SHIFT;
> > + for (j = 0; j < len; j++) {
> > + dma_addr_t addr = sg_dma_address(sg) +
> > + umem->page_size * j;
> > +
> > + ret = pvrdma_page_dir_insert_dma(pdir, i, addr);
> > + if (ret)
> > + goto exit;
> > +
> > + i++;
> > + }
> > + }
> > +
> > +exit:
> > + return ret;
> > +}
> > +
> > +int pvrdma_page_dir_insert_page_list(struct pvrdma_page_dir *pdir,
> > + u64 *page_list,
> > + int num_pages)
> > +{
> > + int i;
> > + int ret;
> > +
> > + if (num_pages > pdir->npages)
> > + return -EINVAL;
>
> How about calling pvrdma_page_dir_cleanup_pages to release current
> allocation?
> This is not needed if you accept the comment for
> pvrdma_page_dir_insert_dma.
Same as above.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
