On Wed, Apr 22, 2020 at 09:39:11AM -0300, Jason Gunthorpe wrote:
> > Nice callchain from hell.. Unfortunately such "code listings" tend to
> > get out of date very quickly, so I'm not sure it is worth keeping in
> > the code. What would be really worthile is consolidating the two
> > different sets of defines (NVIF_VMM_PFNMAP_V0_ vs NVKM_VMM_PFN_)
> > to make the code a little easier to follow.
>
> I was mainly concerned that this function is using hmm properly,
> becuase it sure looks like it is just forming the CPU physical address
> into a HW specific data. But it turns out it is just an internal data
> for some other code and the dma_map is impossibly far away
>
> It took forever to find, I figured I'd leave a hint for the next poor
> soul that has to look at this..
>
> Also, I think it shows there is no 'performance' argument here, if
> this path needs more performance the above should be cleaned
> before we abuse hmm_range_fault.
>
> Put it in the commit message instead?
Yes, the graph itself sounds reasonable for the commit log as a point
of time information.
> > > npages = (range->end - range->start) >> PAGE_SHIFT;
> > > for (i = 0; i < npages; ++i) {
> > > struct page *page;
> > >
> > > + if (!(range->hmm_pfns[i] & HMM_PFN_VALID)) {
> > > + ioctl_addr[i] = 0;
> > > continue;
> > > + }
> >
> > Can't we rely on the caller pre-zeroing the array?
>
> This ends up as args.phys in nouveau_svm_fault - I didn't see a
> zeroing?
>
> I think it makes sense that this routine fully sets the output array
> and does not assume pre-initialize
Ok.
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
