Re: [PATCH v4 01/14] drm/ttm: Remap all page faults to per process dummy page.
On Wed, Jan 27, 2021 at 09:29:41AM -0500, Andrey Grodzovsky wrote: > Hey Daniel, just a ping. Was on vacations last week. > Andrey > > On 1/25/21 10:28 AM, Andrey Grodzovsky wrote: > > > > On 1/19/21 8:56 AM, Daniel Vetter wrote: > > > On Mon, Jan 18, 2021 at 04:01:10PM -0500, Andrey Grodzovsky wrote: > > > > On device removal reroute all CPU mappings to dummy page. > > > > > > > > v3: > > > > Remove loop to find DRM file and instead access it > > > > by vma->vm_file->private_data. Move dummy page installation > > > > into a separate function. > > > > > > > > v4: > > > > Map the entire BOs VA space into on demand allocated dummy page > > > > on the first fault for that BO. > > > > > > > > Signed-off-by: Andrey Grodzovsky > > > > --- > > > > drivers/gpu/drm/ttm/ttm_bo_vm.c | 82 > > > > - > > > > include/drm/ttm/ttm_bo_api.h | 2 + > > > > 2 files changed, 83 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c > > > > b/drivers/gpu/drm/ttm/ttm_bo_vm.c > > > > index 6dc96cf..ed89da3 100644 > > > > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > > > > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > > > > @@ -34,6 +34,8 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > +#include > > > > #include > > > > #include > > > > #include > > > > @@ -380,25 +382,103 @@ vm_fault_t > > > > ttm_bo_vm_fault_reserved(struct vm_fault *vmf, > > > > } > > > > EXPORT_SYMBOL(ttm_bo_vm_fault_reserved); > > > > +static void ttm_bo_release_dummy_page(struct drm_device *dev, void > > > > *res) > > > > +{ > > > > + struct page *dummy_page = (struct page *)res; > > > > + > > > > + __free_page(dummy_page); > > > > +} > > > > + > > > > +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot) > > > > +{ > > > > + struct vm_area_struct *vma = vmf->vma; > > > > + struct ttm_buffer_object *bo = vma->vm_private_data; > > > > + struct ttm_bo_device *bdev = bo->bdev; > > > > + struct drm_device *ddev = bo->base.dev; > > > > + vm_fault_t ret = VM_FAULT_NOPAGE; > > > > + unsigned long address = vma->vm_start; > > > > + unsigned long num_prefault = (vma->vm_end - vma->vm_start) >> > > > > PAGE_SHIFT; > > > > + unsigned long pfn; > > > > + struct page *page; > > > > + int i; > > > > + > > > > + /* > > > > + * Wait for buffer data in transit, due to a pipelined > > > > + * move. > > > > + */ > > > > + ret = ttm_bo_vm_fault_idle(bo, vmf); > > > > + if (unlikely(ret != 0)) > > > > + return ret; > > > > + > > > > + /* Allocate new dummy page to map all the VA range in this VMA to > > > > it*/ > > > > + page = alloc_page(GFP_KERNEL | __GFP_ZERO); > > > > + if (!page) > > > > + return VM_FAULT_OOM; > > > > + > > > > + pfn = page_to_pfn(page); > > > > + > > > > + /* > > > > + * Prefault the entire VMA range right away to avoid further faults > > > > + */ > > > > + for (i = 0; i < num_prefault; ++i) { > > > > + > > > > + if (unlikely(address >= vma->vm_end)) > > > > + break; > > > > + > > > > + if (vma->vm_flags & VM_MIXEDMAP) > > > > + ret = vmf_insert_mixed_prot(vma, address, > > > > + __pfn_to_pfn_t(pfn, PFN_DEV), > > > > + prot); > > > > + else > > > > + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); > > > > + > > > > + /* Never error on prefaulted PTEs */ > > > > + if (unlikely((ret & VM_FAULT_ERROR))) { > > > > + if (i == 0) > > > > + return VM_FAULT_NOPAGE; > > > > + else > > > > + break; > > > > + } > > > > + > > > > + address += PAGE_SIZE; > > > > + } > > > > + > > > > + /* Set the page to be freed using drmm release action */ > > > > + if (drmm_add_action_or_reset(ddev, ttm_bo_release_dummy_page, > > > > page)) > > > > + return VM_FAULT_OOM; > > > > + > > > > + return ret; > > > > +} > > > > +EXPORT_SYMBOL(ttm_bo_vm_dummy_page); > > > I think we can lift this entire thing (once the ttm_bo_vm_fault_idle is > > > gone) to the drm level, since nothing ttm specific in here. Probably stuff > > > it into drm_gem.c (but really it's not even gem specific, it's fully > > > generic "replace this vma with dummy pages pls" function. > > > > > > Once I started with this I noticed that drmm_add_action_or_reset depends > > on struct drm_device *ddev = bo->base.dev and bo is the private data > > we embed at the TTM level when setting up the mapping and so this forces > > to move drmm_add_action_or_reset out of this function to every client who > > uses > > this function, and then you separate the logic of page allocation from > > it's release. > > So I suggest we keep it as is. Uh disappointing. Thing is, ttm essentially means drm devices with gem, except for vmwgfx, which is a drm_device without gem.
Re: [PATCH v4 01/14] drm/ttm: Remap all page faults to per process dummy page.
Hey Daniel, just a ping. Andrey On 1/25/21 10:28 AM, Andrey Grodzovsky wrote: On 1/19/21 8:56 AM, Daniel Vetter wrote: On Mon, Jan 18, 2021 at 04:01:10PM -0500, Andrey Grodzovsky wrote: On device removal reroute all CPU mappings to dummy page. v3: Remove loop to find DRM file and instead access it by vma->vm_file->private_data. Move dummy page installation into a separate function. v4: Map the entire BOs VA space into on demand allocated dummy page on the first fault for that BO. Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/ttm/ttm_bo_vm.c | 82 - include/drm/ttm/ttm_bo_api.h | 2 + 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 6dc96cf..ed89da3 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -34,6 +34,8 @@ #include #include #include +#include +#include #include #include #include @@ -380,25 +382,103 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, } EXPORT_SYMBOL(ttm_bo_vm_fault_reserved); +static void ttm_bo_release_dummy_page(struct drm_device *dev, void *res) +{ + struct page *dummy_page = (struct page *)res; + + __free_page(dummy_page); +} + +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot) +{ + struct vm_area_struct *vma = vmf->vma; + struct ttm_buffer_object *bo = vma->vm_private_data; + struct ttm_bo_device *bdev = bo->bdev; + struct drm_device *ddev = bo->base.dev; + vm_fault_t ret = VM_FAULT_NOPAGE; + unsigned long address = vma->vm_start; + unsigned long num_prefault = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT; + unsigned long pfn; + struct page *page; + int i; + + /* + * Wait for buffer data in transit, due to a pipelined + * move. + */ + ret = ttm_bo_vm_fault_idle(bo, vmf); + if (unlikely(ret != 0)) + return ret; + + /* Allocate new dummy page to map all the VA range in this VMA to it*/ + page = alloc_page(GFP_KERNEL | __GFP_ZERO); + if (!page) + return VM_FAULT_OOM; + + pfn = page_to_pfn(page); + + /* + * Prefault the entire VMA range right away to avoid further faults + */ + for (i = 0; i < num_prefault; ++i) { + + if (unlikely(address >= vma->vm_end)) + break; + + if (vma->vm_flags & VM_MIXEDMAP) + ret = vmf_insert_mixed_prot(vma, address, + __pfn_to_pfn_t(pfn, PFN_DEV), + prot); + else + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); + + /* Never error on prefaulted PTEs */ + if (unlikely((ret & VM_FAULT_ERROR))) { + if (i == 0) + return VM_FAULT_NOPAGE; + else + break; + } + + address += PAGE_SIZE; + } + + /* Set the page to be freed using drmm release action */ + if (drmm_add_action_or_reset(ddev, ttm_bo_release_dummy_page, page)) + return VM_FAULT_OOM; + + return ret; +} +EXPORT_SYMBOL(ttm_bo_vm_dummy_page); I think we can lift this entire thing (once the ttm_bo_vm_fault_idle is gone) to the drm level, since nothing ttm specific in here. Probably stuff it into drm_gem.c (but really it's not even gem specific, it's fully generic "replace this vma with dummy pages pls" function. Once I started with this I noticed that drmm_add_action_or_reset depends on struct drm_device *ddev = bo->base.dev and bo is the private data we embed at the TTM level when setting up the mapping and so this forces to move drmm_add_action_or_reset out of this function to every client who uses this function, and then you separate the logic of page allocation from it's release. So I suggest we keep it as is. Andrey Aside from this nit I think the overall approach you have here is starting to look good. Lots of work, but imo we're getting there and can start landing stuff soon. -Daniel + vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) { struct vm_area_struct *vma = vmf->vma; pgprot_t prot; struct ttm_buffer_object *bo = vma->vm_private_data; + struct drm_device *ddev = bo->base.dev; vm_fault_t ret; + int idx; ret = ttm_bo_vm_reserve(bo, vmf); if (ret) return ret; prot = vma->vm_page_prot; - ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + if (drm_dev_enter(ddev, )) { + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + drm_dev_exit(idx); + } else { + ret = ttm_bo_vm_dummy_page(vmf, prot); + } if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) return ret; dma_resv_unlock(bo->base.resv); return ret; + + return ret; } EXPORT_SYMBOL(ttm_bo_vm_fault); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index e17be32..12fb240 100644
Re: [PATCH v4 01/14] drm/ttm: Remap all page faults to per process dummy page.
On 1/19/21 8:56 AM, Daniel Vetter wrote: On Mon, Jan 18, 2021 at 04:01:10PM -0500, Andrey Grodzovsky wrote: On device removal reroute all CPU mappings to dummy page. v3: Remove loop to find DRM file and instead access it by vma->vm_file->private_data. Move dummy page installation into a separate function. v4: Map the entire BOs VA space into on demand allocated dummy page on the first fault for that BO. Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/ttm/ttm_bo_vm.c | 82 - include/drm/ttm/ttm_bo_api.h| 2 + 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 6dc96cf..ed89da3 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -34,6 +34,8 @@ #include #include #include +#include +#include #include #include #include @@ -380,25 +382,103 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, } EXPORT_SYMBOL(ttm_bo_vm_fault_reserved); +static void ttm_bo_release_dummy_page(struct drm_device *dev, void *res) +{ + struct page *dummy_page = (struct page *)res; + + __free_page(dummy_page); +} + +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot) +{ + struct vm_area_struct *vma = vmf->vma; + struct ttm_buffer_object *bo = vma->vm_private_data; + struct ttm_bo_device *bdev = bo->bdev; + struct drm_device *ddev = bo->base.dev; + vm_fault_t ret = VM_FAULT_NOPAGE; + unsigned long address = vma->vm_start; + unsigned long num_prefault = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT; + unsigned long pfn; + struct page *page; + int i; + + /* +* Wait for buffer data in transit, due to a pipelined +* move. +*/ + ret = ttm_bo_vm_fault_idle(bo, vmf); + if (unlikely(ret != 0)) + return ret; + + /* Allocate new dummy page to map all the VA range in this VMA to it*/ + page = alloc_page(GFP_KERNEL | __GFP_ZERO); + if (!page) + return VM_FAULT_OOM; + + pfn = page_to_pfn(page); + + /* +* Prefault the entire VMA range right away to avoid further faults +*/ + for (i = 0; i < num_prefault; ++i) { + + if (unlikely(address >= vma->vm_end)) + break; + + if (vma->vm_flags & VM_MIXEDMAP) + ret = vmf_insert_mixed_prot(vma, address, + __pfn_to_pfn_t(pfn, PFN_DEV), + prot); + else + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); + + /* Never error on prefaulted PTEs */ + if (unlikely((ret & VM_FAULT_ERROR))) { + if (i == 0) + return VM_FAULT_NOPAGE; + else + break; + } + + address += PAGE_SIZE; + } + + /* Set the page to be freed using drmm release action */ + if (drmm_add_action_or_reset(ddev, ttm_bo_release_dummy_page, page)) + return VM_FAULT_OOM; + + return ret; +} +EXPORT_SYMBOL(ttm_bo_vm_dummy_page); I think we can lift this entire thing (once the ttm_bo_vm_fault_idle is gone) to the drm level, since nothing ttm specific in here. Probably stuff it into drm_gem.c (but really it's not even gem specific, it's fully generic "replace this vma with dummy pages pls" function. Once I started with this I noticed that drmm_add_action_or_reset depends on struct drm_device *ddev = bo->base.dev and bo is the private data we embed at the TTM level when setting up the mapping and so this forces to move drmm_add_action_or_reset out of this function to every client who uses this function, and then you separate the logic of page allocation from it's release. So I suggest we keep it as is. Andrey Aside from this nit I think the overall approach you have here is starting to look good. Lots of work, but imo we're getting there and can start landing stuff soon. -Daniel + vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) { struct vm_area_struct *vma = vmf->vma; pgprot_t prot; struct ttm_buffer_object *bo = vma->vm_private_data; + struct drm_device *ddev = bo->base.dev; vm_fault_t ret; + int idx; ret = ttm_bo_vm_reserve(bo, vmf); if (ret) return ret; prot = vma->vm_page_prot; - ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + if (drm_dev_enter(ddev, )) { + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + drm_dev_exit(idx); + } else { + ret = ttm_bo_vm_dummy_page(vmf, prot); + } if (ret == VM_FAULT_RETRY && !(vmf->flags &
Re: [PATCH v4 01/14] drm/ttm: Remap all page faults to per process dummy page.
On Mon, Jan 18, 2021 at 04:01:10PM -0500, Andrey Grodzovsky wrote: > On device removal reroute all CPU mappings to dummy page. > > v3: > Remove loop to find DRM file and instead access it > by vma->vm_file->private_data. Move dummy page installation > into a separate function. > > v4: > Map the entire BOs VA space into on demand allocated dummy page > on the first fault for that BO. > > Signed-off-by: Andrey Grodzovsky > --- > drivers/gpu/drm/ttm/ttm_bo_vm.c | 82 > - > include/drm/ttm/ttm_bo_api.h| 2 + > 2 files changed, 83 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index 6dc96cf..ed89da3 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -34,6 +34,8 @@ > #include > #include > #include > +#include > +#include > #include > #include > #include > @@ -380,25 +382,103 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault > *vmf, > } > EXPORT_SYMBOL(ttm_bo_vm_fault_reserved); > > +static void ttm_bo_release_dummy_page(struct drm_device *dev, void *res) > +{ > + struct page *dummy_page = (struct page *)res; > + > + __free_page(dummy_page); > +} > + > +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot) > +{ > + struct vm_area_struct *vma = vmf->vma; > + struct ttm_buffer_object *bo = vma->vm_private_data; > + struct ttm_bo_device *bdev = bo->bdev; > + struct drm_device *ddev = bo->base.dev; > + vm_fault_t ret = VM_FAULT_NOPAGE; > + unsigned long address = vma->vm_start; > + unsigned long num_prefault = (vma->vm_end - vma->vm_start) >> > PAGE_SHIFT; > + unsigned long pfn; > + struct page *page; > + int i; > + > + /* > + * Wait for buffer data in transit, due to a pipelined > + * move. > + */ > + ret = ttm_bo_vm_fault_idle(bo, vmf); > + if (unlikely(ret != 0)) > + return ret; > + > + /* Allocate new dummy page to map all the VA range in this VMA to it*/ > + page = alloc_page(GFP_KERNEL | __GFP_ZERO); > + if (!page) > + return VM_FAULT_OOM; > + > + pfn = page_to_pfn(page); > + > + /* > + * Prefault the entire VMA range right away to avoid further faults > + */ > + for (i = 0; i < num_prefault; ++i) { > + > + if (unlikely(address >= vma->vm_end)) > + break; > + > + if (vma->vm_flags & VM_MIXEDMAP) > + ret = vmf_insert_mixed_prot(vma, address, > + __pfn_to_pfn_t(pfn, > PFN_DEV), > + prot); > + else > + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); > + > + /* Never error on prefaulted PTEs */ > + if (unlikely((ret & VM_FAULT_ERROR))) { > + if (i == 0) > + return VM_FAULT_NOPAGE; > + else > + break; > + } > + > + address += PAGE_SIZE; > + } > + > + /* Set the page to be freed using drmm release action */ > + if (drmm_add_action_or_reset(ddev, ttm_bo_release_dummy_page, page)) > + return VM_FAULT_OOM; > + > + return ret; > +} > +EXPORT_SYMBOL(ttm_bo_vm_dummy_page); I think we can lift this entire thing (once the ttm_bo_vm_fault_idle is gone) to the drm level, since nothing ttm specific in here. Probably stuff it into drm_gem.c (but really it's not even gem specific, it's fully generic "replace this vma with dummy pages pls" function. Aside from this nit I think the overall approach you have here is starting to look good. Lots of work, but imo we're getting there and can start landing stuff soon. -Daniel > + > vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) > { > struct vm_area_struct *vma = vmf->vma; > pgprot_t prot; > struct ttm_buffer_object *bo = vma->vm_private_data; > + struct drm_device *ddev = bo->base.dev; > vm_fault_t ret; > + int idx; > > ret = ttm_bo_vm_reserve(bo, vmf); > if (ret) > return ret; > > prot = vma->vm_page_prot; > - ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); > + if (drm_dev_enter(ddev, )) { > + ret = ttm_bo_vm_fault_reserved(vmf, prot, > TTM_BO_VM_NUM_PREFAULT, 1); > + drm_dev_exit(idx); > + } else { > + ret = ttm_bo_vm_dummy_page(vmf, prot); > + } > if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) > return ret; > > dma_resv_unlock(bo->base.resv); > > return ret; > + > + return ret; > } > EXPORT_SYMBOL(ttm_bo_vm_fault); > > diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h > index e17be32..12fb240 100644 > --- a/include/drm/ttm/ttm_bo_api.h > +++
Re: [PATCH v4 01/14] drm/ttm: Remap all page faults to per process dummy page.
Am 18.01.21 um 22:01 schrieb Andrey Grodzovsky: On device removal reroute all CPU mappings to dummy page. v3: Remove loop to find DRM file and instead access it by vma->vm_file->private_data. Move dummy page installation into a separate function. v4: Map the entire BOs VA space into on demand allocated dummy page on the first fault for that BO. Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/ttm/ttm_bo_vm.c | 82 - include/drm/ttm/ttm_bo_api.h| 2 + 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 6dc96cf..ed89da3 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -34,6 +34,8 @@ #include #include #include +#include +#include #include #include #include @@ -380,25 +382,103 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, } EXPORT_SYMBOL(ttm_bo_vm_fault_reserved); +static void ttm_bo_release_dummy_page(struct drm_device *dev, void *res) +{ + struct page *dummy_page = (struct page *)res; + + __free_page(dummy_page); +} + +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot) +{ + struct vm_area_struct *vma = vmf->vma; + struct ttm_buffer_object *bo = vma->vm_private_data; + struct ttm_bo_device *bdev = bo->bdev; + struct drm_device *ddev = bo->base.dev; + vm_fault_t ret = VM_FAULT_NOPAGE; + unsigned long address = vma->vm_start; + unsigned long num_prefault = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT; + unsigned long pfn; + struct page *page; + int i; + + /* +* Wait for buffer data in transit, due to a pipelined +* move. +*/ + ret = ttm_bo_vm_fault_idle(bo, vmf); + if (unlikely(ret != 0)) + return ret; This is superfluous and probably quite harmful here because we wait for the hardware to do something. We map a dummy page instead of the real BO content to the whole range anyway, so no need to wait for the real BO content to show up. + + /* Allocate new dummy page to map all the VA range in this VMA to it*/ + page = alloc_page(GFP_KERNEL | __GFP_ZERO); + if (!page) + return VM_FAULT_OOM; + + pfn = page_to_pfn(page); + + /* +* Prefault the entire VMA range right away to avoid further faults +*/ + for (i = 0; i < num_prefault; ++i) { Maybe rename the variable to num_pages. I was confused for a moment why we still prefault. Alternative you can just drop i and do "for (addr = vma->vm_start; addr < vma->vm_end; addr += PAGE_SIZE)". + + if (unlikely(address >= vma->vm_end)) + break; + + if (vma->vm_flags & VM_MIXEDMAP) + ret = vmf_insert_mixed_prot(vma, address, + __pfn_to_pfn_t(pfn, PFN_DEV), + prot); + else + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); + + /* Never error on prefaulted PTEs */ + if (unlikely((ret & VM_FAULT_ERROR))) { + if (i == 0) + return VM_FAULT_NOPAGE; + else + break; This should probably be modified to either always return the error or always ignore it. Apart from that looks good to me. Christian. + } + + address += PAGE_SIZE; + } + + /* Set the page to be freed using drmm release action */ + if (drmm_add_action_or_reset(ddev, ttm_bo_release_dummy_page, page)) + return VM_FAULT_OOM; + + return ret; +} +EXPORT_SYMBOL(ttm_bo_vm_dummy_page); + vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) { struct vm_area_struct *vma = vmf->vma; pgprot_t prot; struct ttm_buffer_object *bo = vma->vm_private_data; + struct drm_device *ddev = bo->base.dev; vm_fault_t ret; + int idx; ret = ttm_bo_vm_reserve(bo, vmf); if (ret) return ret; prot = vma->vm_page_prot; - ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + if (drm_dev_enter(ddev, )) { + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + drm_dev_exit(idx); + } else { + ret = ttm_bo_vm_dummy_page(vmf, prot); + } if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) return ret; dma_resv_unlock(bo->base.resv); return ret; + + return ret; } EXPORT_SYMBOL(ttm_bo_vm_fault); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index e17be32..12fb240 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@
Re: [PATCH v4 01/14] drm/ttm: Remap all page faults to per process dummy page.
On Mon, Jan 18, 2021 at 4:02 PM Andrey Grodzovsky wrote: > > On device removal reroute all CPU mappings to dummy page. > > v3: > Remove loop to find DRM file and instead access it > by vma->vm_file->private_data. Move dummy page installation > into a separate function. > > v4: > Map the entire BOs VA space into on demand allocated dummy page > on the first fault for that BO. > > Signed-off-by: Andrey Grodzovsky > --- > drivers/gpu/drm/ttm/ttm_bo_vm.c | 82 > - > include/drm/ttm/ttm_bo_api.h| 2 + > 2 files changed, 83 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index 6dc96cf..ed89da3 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -34,6 +34,8 @@ > #include > #include > #include > +#include > +#include > #include > #include > #include > @@ -380,25 +382,103 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault > *vmf, > } > EXPORT_SYMBOL(ttm_bo_vm_fault_reserved); > > +static void ttm_bo_release_dummy_page(struct drm_device *dev, void *res) > +{ > + struct page *dummy_page = (struct page *)res; > + > + __free_page(dummy_page); > +} > + > +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot) > +{ > + struct vm_area_struct *vma = vmf->vma; > + struct ttm_buffer_object *bo = vma->vm_private_data; > + struct ttm_bo_device *bdev = bo->bdev; > + struct drm_device *ddev = bo->base.dev; > + vm_fault_t ret = VM_FAULT_NOPAGE; > + unsigned long address = vma->vm_start; > + unsigned long num_prefault = (vma->vm_end - vma->vm_start) >> > PAGE_SHIFT; > + unsigned long pfn; > + struct page *page; > + int i; > + > + /* > +* Wait for buffer data in transit, due to a pipelined > +* move. > +*/ > + ret = ttm_bo_vm_fault_idle(bo, vmf); > + if (unlikely(ret != 0)) > + return ret; > + > + /* Allocate new dummy page to map all the VA range in this VMA to it*/ > + page = alloc_page(GFP_KERNEL | __GFP_ZERO); > + if (!page) > + return VM_FAULT_OOM; > + > + pfn = page_to_pfn(page); > + > + /* > +* Prefault the entire VMA range right away to avoid further faults > +*/ > + for (i = 0; i < num_prefault; ++i) { > + > + if (unlikely(address >= vma->vm_end)) > + break; > + > + if (vma->vm_flags & VM_MIXEDMAP) > + ret = vmf_insert_mixed_prot(vma, address, > + __pfn_to_pfn_t(pfn, > PFN_DEV), > + prot); > + else > + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); > + > + /* Never error on prefaulted PTEs */ > + if (unlikely((ret & VM_FAULT_ERROR))) { > + if (i == 0) > + return VM_FAULT_NOPAGE; > + else > + break; > + } > + > + address += PAGE_SIZE; > + } > + > + /* Set the page to be freed using drmm release action */ > + if (drmm_add_action_or_reset(ddev, ttm_bo_release_dummy_page, page)) > + return VM_FAULT_OOM; > + > + return ret; > +} > +EXPORT_SYMBOL(ttm_bo_vm_dummy_page); > + > vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) > { > struct vm_area_struct *vma = vmf->vma; > pgprot_t prot; > struct ttm_buffer_object *bo = vma->vm_private_data; > + struct drm_device *ddev = bo->base.dev; > vm_fault_t ret; > + int idx; > > ret = ttm_bo_vm_reserve(bo, vmf); > if (ret) > return ret; > > prot = vma->vm_page_prot; > - ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); > + if (drm_dev_enter(ddev, )) { > + ret = ttm_bo_vm_fault_reserved(vmf, prot, > TTM_BO_VM_NUM_PREFAULT, 1); > + drm_dev_exit(idx); > + } else { > + ret = ttm_bo_vm_dummy_page(vmf, prot); > + } > if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) > return ret; > > dma_resv_unlock(bo->base.resv); > > return ret; > + > + return ret; Duplicate return here. Alex > } > EXPORT_SYMBOL(ttm_bo_vm_fault); > > diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h > index e17be32..12fb240 100644 > --- a/include/drm/ttm/ttm_bo_api.h > +++ b/include/drm/ttm/ttm_bo_api.h > @@ -643,4 +643,6 @@ void ttm_bo_vm_close(struct vm_area_struct *vma); > int ttm_bo_vm_access(struct vm_area_struct *vma, unsigned long addr, > void *buf, int len, int write); > > +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot); > + > #endif
[PATCH v4 01/14] drm/ttm: Remap all page faults to per process dummy page.
On device removal reroute all CPU mappings to dummy page. v3: Remove loop to find DRM file and instead access it by vma->vm_file->private_data. Move dummy page installation into a separate function. v4: Map the entire BOs VA space into on demand allocated dummy page on the first fault for that BO. Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/ttm/ttm_bo_vm.c | 82 - include/drm/ttm/ttm_bo_api.h| 2 + 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 6dc96cf..ed89da3 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -34,6 +34,8 @@ #include #include #include +#include +#include #include #include #include @@ -380,25 +382,103 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, } EXPORT_SYMBOL(ttm_bo_vm_fault_reserved); +static void ttm_bo_release_dummy_page(struct drm_device *dev, void *res) +{ + struct page *dummy_page = (struct page *)res; + + __free_page(dummy_page); +} + +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot) +{ + struct vm_area_struct *vma = vmf->vma; + struct ttm_buffer_object *bo = vma->vm_private_data; + struct ttm_bo_device *bdev = bo->bdev; + struct drm_device *ddev = bo->base.dev; + vm_fault_t ret = VM_FAULT_NOPAGE; + unsigned long address = vma->vm_start; + unsigned long num_prefault = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT; + unsigned long pfn; + struct page *page; + int i; + + /* +* Wait for buffer data in transit, due to a pipelined +* move. +*/ + ret = ttm_bo_vm_fault_idle(bo, vmf); + if (unlikely(ret != 0)) + return ret; + + /* Allocate new dummy page to map all the VA range in this VMA to it*/ + page = alloc_page(GFP_KERNEL | __GFP_ZERO); + if (!page) + return VM_FAULT_OOM; + + pfn = page_to_pfn(page); + + /* +* Prefault the entire VMA range right away to avoid further faults +*/ + for (i = 0; i < num_prefault; ++i) { + + if (unlikely(address >= vma->vm_end)) + break; + + if (vma->vm_flags & VM_MIXEDMAP) + ret = vmf_insert_mixed_prot(vma, address, + __pfn_to_pfn_t(pfn, PFN_DEV), + prot); + else + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); + + /* Never error on prefaulted PTEs */ + if (unlikely((ret & VM_FAULT_ERROR))) { + if (i == 0) + return VM_FAULT_NOPAGE; + else + break; + } + + address += PAGE_SIZE; + } + + /* Set the page to be freed using drmm release action */ + if (drmm_add_action_or_reset(ddev, ttm_bo_release_dummy_page, page)) + return VM_FAULT_OOM; + + return ret; +} +EXPORT_SYMBOL(ttm_bo_vm_dummy_page); + vm_fault_t ttm_bo_vm_fault(struct vm_fault *vmf) { struct vm_area_struct *vma = vmf->vma; pgprot_t prot; struct ttm_buffer_object *bo = vma->vm_private_data; + struct drm_device *ddev = bo->base.dev; vm_fault_t ret; + int idx; ret = ttm_bo_vm_reserve(bo, vmf); if (ret) return ret; prot = vma->vm_page_prot; - ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + if (drm_dev_enter(ddev, )) { + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + drm_dev_exit(idx); + } else { + ret = ttm_bo_vm_dummy_page(vmf, prot); + } if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) return ret; dma_resv_unlock(bo->base.resv); return ret; + + return ret; } EXPORT_SYMBOL(ttm_bo_vm_fault); diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index e17be32..12fb240 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -643,4 +643,6 @@ void ttm_bo_vm_close(struct vm_area_struct *vma); int ttm_bo_vm_access(struct vm_area_struct *vma, unsigned long addr, void *buf, int len, int write); +vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot); + #endif -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel