On Wed, Feb 13, 2019 at 11:58:24PM +0530, Devesh Sharma wrote:
> On Wed, Feb 13, 2019 at 10:58 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> >
> > On Wed, Feb 13, 2019 at 10:44:47PM +0530, Devesh Sharma wrote:
> > > > @@ -116,12 +116,9 @@ static int __alloc_pbl(struct pci_dev *pdev, struct bnxt_qplib_pbl *pbl,
> > > > } else {
> > > > i = 0;
> > > > is_umem = true;
> > > > - for_each_sg(sghead, sg, pages, i) {
> > > > - pbl->pg_map_arr[i] = sg_dma_address(sg);
> > > > - pbl->pg_arr[i] = sg_virt(sg);
> > > > - if (!pbl->pg_arr[i])
> > > > - goto fail;
> > > > -
> > > > + for_each_sg_dma_page(sghead, &sg_iter, pages, 0) {
> > > > + pbl->pg_map_arr[i] = sg_page_iter_dma_address(&sg_iter);
> > > > + i++;
> > > > pbl->pg_count++;
> > > The thing is we need the virtual address for future reference in this
> > > path. If we do not have virt-addr populated it breaks!
> >
> > Drivers should not use both DMA and struct page for umems. One other
> > other.
> >
> > Where is the pg_arr read in the umem case, and what is it for?
> Right there at the caller of __alloc_pbl().
What I see is a confusing mess where sometimes the pg_arr points to
dma_alloc_coherent value that needs to be freed, and sometimes points
to a struct page pointing into user memory.
All the cases that touch pg_arr seem to want to touch the
dma_alloc_coherent memory, not the user pages, so setting it to NULL
for the usage pages seems like the right thing to do?
Jason
