nouveau/hmm: map pages after migration
Subsystems (5)
PATCH v2 1/1 nouveau/hmm: map pages after migration
When memory is migrated to the GPU, it is likely to be accessed by GPU code soon afterwards. Instead of waiting for a GPU fault, map the migrated memory into the GPU page tables with the same access permissions as the source CPU page table entries. This preserves copy on write semantics.
---
Originally this patch was targeted for Jason's rdma tree since other HMM related changes were queued there. Now that those have been merged, this patch just contains changes to nouveau so it could go through any tree. I guess Ben Skeggs' tree would be appropriate.
Changes since v1:
Rebase to linux-5.6.0-rc4
Address Christoph Hellwig's comments
drivers/gpu/drm/nouveau/nouveau_dmem.c | 44 ++++++++----- drivers/gpu/drm/nouveau/nouveau_svm.c | 85 ++++++++++++++++++++++++++ drivers/gpu/drm/nouveau/nouveau_svm.h | 5 ++ 3 files changed, 118 insertions(+), 16 deletions(-)
drivers/gpu/drm/nouveau/nouveau_dmem.c +28 -16
@@ -25,11 +25,13 @@#include "nouveau_dma.h" #include "nouveau_mem.h" #include "nouveau_bo.h" +#include "nouveau_svm.h" #include <nvif/class.h> #include <nvif/object.h> #include <nvif/if500b.h> #include <nvif/if900b.h> +#include <nvif/if000c.h> #include <linux/sched/mm.h> #include <linux/hmm.h>
@@ -558,10 +560,11 @@ nouveau_dmem_init(struct nouveau_drm *drm) }static unsigned long nouveau_dmem_migrate_copy_one (struct nouveau_drm *drm, -unsigned long src,dma_addr_t *dma_addr) +unsigned long src,dma_addr_t *dma_addr,u64 *pfn) {struct device *dev = drm->dev->dev;struct page *dpage, *spage; +unsigned long paddr; spage =migrate_pfn_to_page (src);if (!spage || !(src &MIGRATE_PFN_MIGRATE ))
@@ -569,17 +572,21 @@ static unsigned long nouveau_dmem_migrate_copy_one(struct nouveau_drm *drm, dpage =nouveau_dmem_page_alloc_locked (drm);if (!dpage) -return 0 ; + goto out; *dma_addr =dma_map_page (dev, spage,0 ,PAGE_SIZE ,DMA_BIDIRECTIONAL );if (dma_mapping_error (dev, *dma_addr)) goto out_free_page; + paddr =nouveau_dmem_page_addr (dpage);if (drm->dmem->migrate.copy_func (drm,1 ,NOUVEAU_APER_VRAM , -nouveau_dmem_page_addr (dpage),NOUVEAU_APER_HOST , - *dma_addr)) + paddr,NOUVEAU_APER_HOST , *dma_addr)) goto out_dma_unmap; + *pfn =NVIF_VMM_PFNMAP_V0_V |NVIF_VMM_PFNMAP_V0_VRAM | + ((paddr >>PAGE_SHIFT ) <<NVIF_VMM_PFNMAP_V0_ADDR_SHIFT ); +if (src &MIGRATE_PFN_WRITE ) + *pfn |=NVIF_VMM_PFNMAP_V0_W ;return migrate_pfn (page_to_pfn (dpage)) |MIGRATE_PFN_LOCKED ; out_dma_unmap:
@@ -587,18 +594,19 @@ static unsigned long nouveau_dmem_migrate_copy_one(struct nouveau_drm *drm, out_free_page:nouveau_dmem_page_free_locked (drm, dpage); out: + *pfn =NVIF_VMM_PFNMAP_V0_NONE ;return 0 ; }static void nouveau_dmem_migrate_chunk (struct nouveau_drm *drm, -struct migrate_vma *args,dma_addr_t *dma_addrs) +struct migrate_vma *args,dma_addr_t *dma_addrs,u64 *pfns) {struct nouveau_fence *fence;unsigned long addr = args->start, nr_dma =0 , i;for (i =0 ; addr < args->end; i++) { args->dst[i] =nouveau_dmem_migrate_copy_one (drm, args->src[i], - dma_addrs + nr_dma); + dma_addrs + nr_dma, pfns + i);if (args->dst[i]) nr_dma++; addr +=PAGE_SIZE ;
@@ -607,15 +615,12 @@ static void nouveau_dmem_migrate_chunk(struct nouveau_drm *drm,nouveau_fence_new (drm->dmem->migrate.chan, false, &fence);migrate_vma_pages (args);nouveau_dmem_fence_done (&fence); +nouveau_pfns_map (drm, args->vma->vm_mm, args->start, pfns, i);while (nr_dma--) {dma_unmap_page (drm->dev->dev, dma_addrs[nr_dma],PAGE_SIZE ,DMA_BIDIRECTIONAL ); } -/* -* FIXME optimization: update GPU page table to point to newly migrated -* memory. -*/ migrate_vma_finalize (args); }
@@ -632,7 +637,8 @@ nouveau_dmem_migrate_vma(struct nouveau_drm *drm, .vma = vma, .start = start, }; -unsigned long c, i; +unsigned long i; +u64 *pfns;int ret = -ENOMEM ; args.src =kcalloc (max,sizeof (*args.src),GFP_KERNEL );
@@ -646,19 +652,25 @@ nouveau_dmem_migrate_vma(struct nouveau_drm *drm,if (!dma_addrs) goto out_free_dst; -for (i =0 ; i < npages; i += c) { - c =min (SG_MAX_SINGLE_ALLOC , npages); - args.end = start + (c <<PAGE_SHIFT ); + pfns =nouveau_pfns_alloc (max); +if (!pfns) + goto out_free_dma; + +for (i =0 ; i < npages; i += max) { + args.end = start + (max <<PAGE_SHIFT ); ret =migrate_vma_setup (&args);if (ret) - goto out_free_dma; + goto out_free_pfns;if (args.cpages) -nouveau_dmem_migrate_chunk (drm, &args, dma_addrs); +nouveau_dmem_migrate_chunk (drm, &args, dma_addrs, + pfns); args.start = args.end; } ret =0 ; +out_free_pfns: +nouveau_pfns_free (pfns); out_free_dma:kfree (dma_addrs); out_free_dst:
drivers/gpu/drm/nouveau/nouveau_svm.c +85 -0
97 lines not on this page · raw message
drivers/gpu/drm/nouveau/nouveau_svm.h +5 -0
@@ -18,6 +18,11 @@ void nouveau_svmm_fini(struct nouveau_svmm **);int nouveau_svmm_join (struct nouveau_svmm *,u64 inst);void nouveau_svmm_part (struct nouveau_svmm *,u64 inst);int nouveau_svmm_bind (struct drm_device *,void *,struct drm_file *); + +u64 *nouveau_pfns_alloc (unsigned long npages); +void nouveau_pfns_free (u64 *pfns); +void nouveau_pfns_map (struct nouveau_drm *drm,struct mm_struct *mm, +unsigned long addr,u64 *pfns,unsigned long npages);#else /* IS_ENABLED(CONFIG_DRM_NOUVEAU_SVM) */ static inline void nouveau_svm_init (struct nouveau_drm *drm) {}static inline void nouveau_svm_fini (struct nouveau_drm *drm) {}
Signature · 1 line
2.20.1
Quoted 11 lines · Ralph Campbell
When memory is migrated to the GPU, it is likely to be accessed by GPU code soon afterwards. Instead of waiting for a GPU fault, map the migrated memory into the GPU page tables with the same access permissions as the source CPU page table entries. This preserves copy on write semantics.
Signed-off-by Ralph Campbell <[email protected]>Cc Christoph Hellwig <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc "Jérôme Glisse" <[email protected]>Cc Ben Skeggs <[email protected]>
Quoted 6 lines
---
Originally this patch was targeted for Jason's rdma tree since other HMM related changes were queued there. Now that those have been merged, this patch just contains changes to nouveau so it could go through any tree. I guess Ben Skeggs' tree would be appropriate.
Yep
drivers/gpu/drm/nouveau/nouveau_svm.c nouveau_svm_fault(struct nvif_notify *notify) · line 792
Quoted 2 lines
drivers/gpu/drm/nouveau/nouveau_svm.c +2
-0+static inline struct nouveau_pfnmap_args * +nouveau_pfns_to_args (void *pfns)
don't use static inline inside C files
drivers/gpu/drm/nouveau/nouveau_svm.c nouveau_svm_fault(struct nvif_notify *notify) · line 797
Quoted 5 lines
drivers/gpu/drm/nouveau/nouveau_svm.c +5
-0+{ +struct nvif_vmm_pfnmap_v0 *p = + container_of(pfns, struct nvif_vmm_pfnmap_v0, phys); + + return container_of(p, struct nouveau_pfnmap_args, p);
And this should just be
return container_of(pfns, struct nouveau_pfnmap_args, p.phys);
drivers/gpu/drm/nouveau/nouveau_svm.c nouveau_svm_fault(struct nvif_notify *notify) · line 834
Quoted 11 lines
drivers/gpu/drm/nouveau/nouveau_svm.c +11
-0+static struct nouveau_svmm * +nouveau_find_svmm (struct nouveau_svm *svm,struct mm_struct *mm) +{ +struct nouveau_ivmm *ivmm; + +list_for_each_entry (ivmm, &svm->inst, head) { +if (ivmm->svmm->notifier.mm == mm) +return ivmm->svmm; + } +return NULL ; +}
Is this re-implementing mmu_notifier_get() ?
Jason
Quoted 19 lines · Jason Gunthorpe
Quoted 11 lines · Ralph Campbell
When memory is migrated to the GPU, it is likely to be accessed by GPU code soon afterwards. Instead of waiting for a GPU fault, map the migrated memory into the GPU page tables with the same access permissions as the source CPU page table entries. This preserves copy on write semantics.
Signed-off-by Ralph Campbell <[email protected]>Cc Christoph Hellwig <[email protected]>Cc Jason Gunthorpe <[email protected]>Cc "Jérôme Glisse" <[email protected]>Cc Ben Skeggs <[email protected]>Quoted 6 lines
---
Originally this patch was targeted for Jason's rdma tree since other HMM related changes were queued there. Now that those have been merged, this patch just contains changes to nouveau so it could go through any tree. I guess Ben Skeggs' tree would be appropriate.
Quoted 6 lines
Yep
drivers/gpu/drm/nouveau/nouveau_svm.c nouveau_svm_fault(struct nvif_notify *notify) · line 792
Quoted 2 lines
drivers/gpu/drm/nouveau/nouveau_svm.c +2
-0+static inline struct nouveau_pfnmap_args * +nouveau_pfns_to_args (void *pfns)don't use static inline inside C files
OK.
Quoted 9 lines
drivers/gpu/drm/nouveau/nouveau_svm.c nouveau_svm_fault(struct nvif_notify *notify) · line 797
Quoted 5 lines
drivers/gpu/drm/nouveau/nouveau_svm.c +5
-0+{ +struct nvif_vmm_pfnmap_v0 *p = + container_of(pfns, struct nvif_vmm_pfnmap_v0, phys); + + return container_of(p, struct nouveau_pfnmap_args, p);And this should just be
return container_of(pfns, struct nouveau_pfnmap_args, p.phys);
Much simpler, thanks.
Quoted 14 lines
drivers/gpu/drm/nouveau/nouveau_svm.c nouveau_svm_fault(struct nvif_notify *notify) · line 834
Quoted 11 lines
drivers/gpu/drm/nouveau/nouveau_svm.c +11
-0+static struct nouveau_svmm * +nouveau_find_svmm (struct nouveau_svm *svm,struct mm_struct *mm) +{ +struct nouveau_ivmm *ivmm; + +list_for_each_entry (ivmm, &svm->inst, head) { +if (ivmm->svmm->notifier.mm == mm) +return ivmm->svmm; + } +return NULL ; +}Is this re-implementing
mmu_notifier_get()?
Quoted 1 line
Jason
Not quite. This is being called from an ioctl() call on the GPU device file which calls nouveau_svmm_bind() which locks mmap_sem for reading, walks the vmas for the address range given in the ioctl() data, and migrates the pages to the GPU memory.mmu_notifier_get() would try to lock mmap_sem for writing so that would deadlock. But it is similar in that the GPU specific process context (nouveau_svmm) needs to be found for the given ioctl caller.
If find_get_mmu_notifier() was exported, I think that could work. Now that I look at this again, there is an easier way to find the svmm and I see some other bugs that need fixing. I'll post a v3 as soon as I get those written and tested.
Thanks for the review.
Quoted 15 lines · Ralph Campbell
Quoted 4 lines
drivers/gpu/drm/nouveau/nouveau_svm.c nouveau_svm_fault(struct nvif_notify *notify) · line 792
Quoted 2 lines
drivers/gpu/drm/nouveau/nouveau_svm.c +2
-0+static inline struct nouveau_pfnmap_args * +nouveau_pfns_to_args (void *pfns)don't use static inline inside C files
OK.
Quoted 8 lines
drivers/gpu/drm/nouveau/nouveau_svm.c nouveau_svm_fault(struct nvif_notify *notify) · line 797
Quoted 5 lines
drivers/gpu/drm/nouveau/nouveau_svm.c +5
-0+{ +struct nvif_vmm_pfnmap_v0 *p = + container_of(pfns, struct nvif_vmm_pfnmap_v0, phys); + + return container_of(p, struct nouveau_pfnmap_args, p);And this should just be
Quoted 3 lines
Quoted 1 line
return container_of(pfns, struct nouveau_pfnmap_args, p.phys);Much simpler, thanks.
Btw, for the case where we just have an container_of wrapper I strongly disagree with avoiding the inline - not inlining this would be stupid, but unfortunately compilers often behave stupidly. It also is a very clear marker.