New subject: [Intel-gfx] [PATCH] drm/i915/eb: Fix pagefault disabling in the first slowpath

18 Jun 2021

In
commit ebc0808fa2da0548a78e715858024cb81cd732bc
Author: Chris Wilson chris@chris-wilson.co.uk
Date:   Tue Oct 18 13:02:51 2016 +0100
drm/i915: Restrict pagefault disabling to just around copy_from_user()
we entirely missed that there's a slow path call to eb_relocate_entry
(or i915_gem_execbuffer_relocate_entry as it was called back then)
which was left fully wrapped by pagefault_disable/enable() calls.
Previously any issues with blocking calls where handled by the
following code:
/* we can't wait for rendering with pagefaults disabled */
    if (pagefault_disabled() && !object_is_idle(obj))
    	return -EFAULT;
Now at this point the prefaulting was still around, which means in
normal applications it was very hard to hit this bug. No idea why the
regressions in igts weren't caught.
Now this all changed big time with 2 patches merged closely together.
First
commit 2889caa9232109afc8881f29a2205abeb5709d0c
Author: Chris Wilson chris@chris-wilson.co.uk
Date:   Fri Jun 16 15:05:19 2017 +0100
drm/i915: Eliminate lots of iterations over the execobjects array
removes the prefaulting from the first relocation path, pushing it into
the first slowpath (of which this patch added a total of 3 escalation
levels). This would have really quickly uncovered the above bug, were
it not for immediate adding a duct-tape on top with
commit 7dd4f6729f9243bd7046c6f04c107a456bda38eb
Author: Chris Wilson chris@chris-wilson.co.uk
Date:   Fri Jun 16 15:05:24 2017 +0100
drm/i915: Async GPU relocation processing
by pushing all all the relocation patching to the gpu if the buffer
was busy, which avoided all the possible blocking calls.
The entire slowpath was then furthermore ditched in
commit 7dc8f1143778a35b190f9413f228b3cf28f67f8d
Author: Chris Wilson chris@chris-wilson.co.uk
Date:   Wed Mar 11 16:03:10 2020 +0000
drm/i915/gem: Drop relocation slowpath
and resurrected in
commit fd1500fcd4420eee06e2c7f3aa6067b78ac05871
Author: Maarten Lankhorst maarten.lankhorst@linux.intel.com
Date:   Wed Aug 19 16:08:43 2020 +0200
Revert "drm/i915/gem: Drop relocation slowpath".
but this did not further impact what's going on.
Since pagefault_disable/enable is an atomic section, any sleeping in
there is prohibited, and we definitely do that without gpu relocations
since we have to wait for the gpu usage to finish before we can patch
up the relocations.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Cc: Jon Bloomfield jon.bloomfield@intel.com
Cc: Chris Wilson chris@chris-wilson.co.uk
Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com
Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com
Cc: Daniel Vetter daniel.vetter@ffwll.ch
Cc: "Thomas Hellström" thomas.hellstrom@linux.intel.com
Cc: Matthew Auld matthew.auld@intel.com
Cc: Lionel Landwerlin lionel.g.landwerlin@intel.com
Cc: Dave Airlie airlied@redhat.com
Cc: Jason Ekstrand jason@jlekstrand.net
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 6539b82dda54..7ff2fc3c0b2c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2082,9 +2082,7 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb,
list_for_each_entry(ev, &eb->relocs, reloc_link) {
    	if (!have_copy) {
-			pagefault_disable();
    		err = eb_relocate_vma(eb, ev);
-			pagefault_enable();
    		if (err)
    			break;
    	} else {
-- 
2.32.0.rc2


    