| Message ID | 20190912183854.28194-1-thomas_os@shipmail.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ttm: Restore ttm prefaulting | expand |
On Fri, Sep 13, 2019 at 12:09 AM Thomas Hellström (VMware) <thomas_os@shipmail.org> wrote: > > From: Thomas Hellstrom <thellstrom@vmware.com> > > Commit 4daa4fba3a38 ("gpu: drm: ttm: Adding new return type vm_fault_t") > broke TTM prefaulting. Since vmf_insert_mixed() typically always returns > VM_FAULT_NOPAGE, prefaulting stops after the second PTE. > > Restore (almost) the original behaviour. Unfortunately we can no longer > with the new vm_fault_t return type determine whether a prefaulting > PTE insertion hit an already populated PTE, and terminate the insertion > loop. Instead we continue with the pre-determined number of prefaults. > > Fixes: 4daa4fba3a38 ("gpu: drm: ttm: Adding new return type vm_fault_t") > Cc: Souptick Joarder <jrdr.linux@gmail.com> > Cc: Christian König <christian.koenig@amd.com> > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> This commit merged into 4.19. Need to Cc stable. Cc: stable@vger.kernel.org # v4.19+ > --- > drivers/gpu/drm/ttm/ttm_bo_vm.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index 5a580adeb9d1..aa18e8a53727 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -290,15 +290,13 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, > else > ret = vmf_insert_pfn(&cvma, address, pfn); > > - /* > - * Somebody beat us to this PTE or prefaulting to > - * an already populated PTE, or prefaulting error. > - */ > - > - if (unlikely((ret == VM_FAULT_NOPAGE && i > 0))) > - break; > - else if (unlikely(ret & VM_FAULT_ERROR)) > - goto out_io_unlock; > + /* Never error on prefaulted PTEs */ > + if (unlikely((ret & VM_FAULT_ERROR))) { > + if (i == 0) > + goto out_io_unlock; > + else > + break; > + } > > address += PAGE_SIZE; > if (unlikely(++page_offset >= page_last)) > -- > 2.20.1 >
Am 12.09.19 um 20:38 schrieb Thomas Hellström (VMware): > From: Thomas Hellstrom <thellstrom@vmware.com> > > Commit 4daa4fba3a38 ("gpu: drm: ttm: Adding new return type vm_fault_t") > broke TTM prefaulting. Since vmf_insert_mixed() typically always returns > VM_FAULT_NOPAGE, prefaulting stops after the second PTE. > > Restore (almost) the original behaviour. Unfortunately we can no longer > with the new vm_fault_t return type determine whether a prefaulting > PTE insertion hit an already populated PTE, and terminate the insertion > loop. Instead we continue with the pre-determined number of prefaults. > > Fixes: 4daa4fba3a38 ("gpu: drm: ttm: Adding new return type vm_fault_t") > Cc: Souptick Joarder <jrdr.linux@gmail.com> > Cc: Christian König <christian.koenig@amd.com> > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> Reviewed-by: Christian König <christian.koenig@amd.com> Going to pick that up when Alex rebases our upstream branch. Christian. > --- > drivers/gpu/drm/ttm/ttm_bo_vm.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index 5a580adeb9d1..aa18e8a53727 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -290,15 +290,13 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, > else > ret = vmf_insert_pfn(&cvma, address, pfn); > > - /* > - * Somebody beat us to this PTE or prefaulting to > - * an already populated PTE, or prefaulting error. > - */ > - > - if (unlikely((ret == VM_FAULT_NOPAGE && i > 0))) > - break; > - else if (unlikely(ret & VM_FAULT_ERROR)) > - goto out_io_unlock; > + /* Never error on prefaulted PTEs */ > + if (unlikely((ret & VM_FAULT_ERROR))) { > + if (i == 0) > + goto out_io_unlock; > + else > + break; > + } > > address += PAGE_SIZE; > if (unlikely(++page_offset >= page_last))
On 9/13/19 12:53 PM, Koenig, Christian wrote: > Am 12.09.19 um 20:38 schrieb Thomas Hellström (VMware): >> From: Thomas Hellstrom <thellstrom@vmware.com> >> >> Commit 4daa4fba3a38 ("gpu: drm: ttm: Adding new return type vm_fault_t") >> broke TTM prefaulting. Since vmf_insert_mixed() typically always returns >> VM_FAULT_NOPAGE, prefaulting stops after the second PTE. >> >> Restore (almost) the original behaviour. Unfortunately we can no longer >> with the new vm_fault_t return type determine whether a prefaulting >> PTE insertion hit an already populated PTE, and terminate the insertion >> loop. Instead we continue with the pre-determined number of prefaults. >> >> Fixes: 4daa4fba3a38 ("gpu: drm: ttm: Adding new return type vm_fault_t") >> Cc: Souptick Joarder <jrdr.linux@gmail.com> >> Cc: Christian König <christian.koenig@amd.com> >> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> > Reviewed-by: Christian König <christian.koenig@amd.com> > > Going to pick that up when Alex rebases our upstream branch. > > Christian. > >> --- >> drivers/gpu/drm/ttm/ttm_bo_vm.c | 16 +++++++--------- >> 1 file changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c >> index 5a580adeb9d1..aa18e8a53727 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c >> @@ -290,15 +290,13 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, >> else >> ret = vmf_insert_pfn(&cvma, address, pfn); >> >> - /* >> - * Somebody beat us to this PTE or prefaulting to >> - * an already populated PTE, or prefaulting error. >> - */ >> - >> - if (unlikely((ret == VM_FAULT_NOPAGE && i > 0))) >> - break; >> - else if (unlikely(ret & VM_FAULT_ERROR)) >> - goto out_io_unlock; >> + /* Never error on prefaulted PTEs */ >> + if (unlikely((ret & VM_FAULT_ERROR))) { >> + if (i == 0) >> + goto out_io_unlock; >> + else We could perhaps skip that else. Let me know if you want me to respin. /Thomas >> + break; >> + } >> >> address += PAGE_SIZE; >> if (unlikely(++page_offset >= page_last))
Am 13.09.19 um 13:07 schrieb Thomas Hellström (VMware): > On 9/13/19 12:53 PM, Koenig, Christian wrote: >> Am 12.09.19 um 20:38 schrieb Thomas Hellström (VMware): >>> From: Thomas Hellstrom <thellstrom@vmware.com> >>> >>> Commit 4daa4fba3a38 ("gpu: drm: ttm: Adding new return type >>> vm_fault_t") >>> broke TTM prefaulting. Since vmf_insert_mixed() typically always >>> returns >>> VM_FAULT_NOPAGE, prefaulting stops after the second PTE. >>> >>> Restore (almost) the original behaviour. Unfortunately we can no longer >>> with the new vm_fault_t return type determine whether a prefaulting >>> PTE insertion hit an already populated PTE, and terminate the insertion >>> loop. Instead we continue with the pre-determined number of prefaults. >>> >>> Fixes: 4daa4fba3a38 ("gpu: drm: ttm: Adding new return type >>> vm_fault_t") >>> Cc: Souptick Joarder <jrdr.linux@gmail.com> >>> Cc: Christian König <christian.koenig@amd.com> >>> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com> >> Reviewed-by: Christian König <christian.koenig@amd.com> >> >> Going to pick that up when Alex rebases our upstream branch. >> >> Christian. >> >>> --- >>> drivers/gpu/drm/ttm/ttm_bo_vm.c | 16 +++++++--------- >>> 1 file changed, 7 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c >>> b/drivers/gpu/drm/ttm/ttm_bo_vm.c >>> index 5a580adeb9d1..aa18e8a53727 100644 >>> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c >>> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c >>> @@ -290,15 +290,13 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct >>> vm_fault *vmf, >>> else >>> ret = vmf_insert_pfn(&cvma, address, pfn); >>> - /* >>> - * Somebody beat us to this PTE or prefaulting to >>> - * an already populated PTE, or prefaulting error. >>> - */ >>> - >>> - if (unlikely((ret == VM_FAULT_NOPAGE && i > 0))) >>> - break; >>> - else if (unlikely(ret & VM_FAULT_ERROR)) >>> - goto out_io_unlock; >>> + /* Never error on prefaulted PTEs */ >>> + if (unlikely((ret & VM_FAULT_ERROR))) { >>> + if (i == 0) >>> + goto out_io_unlock; >>> + else > > We could perhaps skip that else. Let me know if you want me to respin. I would rather like to keep it, when something goes wrong we should really stop faulting in more. Christian. > > /Thomas > > > >>> + break; >>> + } >>> address += PAGE_SIZE; >>> if (unlikely(++page_offset >= page_last)) > >
diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c index 5a580adeb9d1..aa18e8a53727 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c @@ -290,15 +290,13 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, else ret = vmf_insert_pfn(&cvma, address, pfn); - /* - * Somebody beat us to this PTE or prefaulting to - * an already populated PTE, or prefaulting error. - */ - - if (unlikely((ret == VM_FAULT_NOPAGE && i > 0))) - break; - else if (unlikely(ret & VM_FAULT_ERROR)) - goto out_io_unlock; + /* Never error on prefaulted PTEs */ + if (unlikely((ret & VM_FAULT_ERROR))) { + if (i == 0) + goto out_io_unlock; + else + break; + } address += PAGE_SIZE; if (unlikely(++page_offset >= page_last))