[PATCH] drm/i915: Retry gtt fault when out of fence register

[PATCH] drm/i915: Retry gtt fault when out of fence register

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If we can't find a free fence register to handle a fault in the GMADR
range just return VM_FAULT_NOPAGE without populating the PTE so that
userspace will retry the access and trigger another fault. Eventually
we should find a free fence and the fault will get properly handled.

A further improvement idea might be to reserve a fence (or one per CPU?)
for the express purpose of handling faults without having to retry. But
that would require some additional work.

Looks like this may have gotten broken originally by
commit 39965b376601 ("drm/i915: don't trash the gtt when running out of fences")
as that changed the errno to -EDEADLK which wasn't handle by the gtt
fault code either. But later in commit 2feeb52859fc ("drm/i915/gt: Fix
-EDEADLK handling regression") I changed it again to -ENOBUFS as -EDEADLK
was now getting used for the ww mutex dance. So this fix only makes
sense after that last commit.

Cc: stable@vger.kernel.org
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9479
Fixes: 2feeb52859fc ("drm/i915/gt: Fix -EDEADLK handling regression")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index aa4d842d4c5a..310654542b42 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -235,6 +235,7 @@ static vm_fault_t i915_error_to_vmf_fault(int err)
 	case 0:
 	case -EAGAIN:
 	case -ENOSPC: /* transient failure to evict? */
+	case -ENOBUFS: /* temporarily out of fences? */
 	case -ERESTARTSYS:
 	case -EINTR:
 	case -EBUSY:
-- 
2.41.0

Re: [PATCH] drm/i915: Retry gtt fault when out of fence register

[Greg KH]

On Thu, Oct 12, 2023 at 04:28:01PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If we can't find a free fence register to handle a fault in the GMADR
> range just return VM_FAULT_NOPAGE without populating the PTE so that
> userspace will retry the access and trigger another fault. Eventually
> we should find a free fence and the fault will get properly handled.
> 
> A further improvement idea might be to reserve a fence (or one per CPU?)
> for the express purpose of handling faults without having to retry. But
> that would require some additional work.
> 
> Looks like this may have gotten broken originally by
> commit 39965b376601 ("drm/i915: don't trash the gtt when running out of fences")
> as that changed the errno to -EDEADLK which wasn't handle by the gtt
> fault code either. But later in commit 2feeb52859fc ("drm/i915/gt: Fix
> -EDEADLK handling regression") I changed it again to -ENOBUFS as -EDEADLK
> was now getting used for the ww mutex dance. So this fix only makes
> sense after that last commit.
> 
> Cc: stable@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9479
> Fixes: 2feeb52859fc ("drm/i915/gt: Fix -EDEADLK handling regression")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 1 +
>  1 file changed, 1 insertion(+)
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH] drm/i915: Retry gtt fault when out of fence register

[Ville Syrjälä]

On Thu, Oct 12, 2023 at 03:40:08PM +0200, Greg KH wrote:
> On Thu, Oct 12, 2023 at 04:28:01PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If we can't find a free fence register to handle a fault in the GMADR
> > range just return VM_FAULT_NOPAGE without populating the PTE so that
> > userspace will retry the access and trigger another fault. Eventually
> > we should find a free fence and the fault will get properly handled.
> > 
> > A further improvement idea might be to reserve a fence (or one per CPU?)
> > for the express purpose of handling faults without having to retry. But
> > that would require some additional work.
> > 
> > Looks like this may have gotten broken originally by
> > commit 39965b376601 ("drm/i915: don't trash the gtt when running out of fences")
> > as that changed the errno to -EDEADLK which wasn't handle by the gtt
> > fault code either. But later in commit 2feeb52859fc ("drm/i915/gt: Fix
> > -EDEADLK handling regression") I changed it again to -ENOBUFS as -EDEADLK
> > was now getting used for the ww mutex dance. So this fix only makes
> > sense after that last commit.
> > 
> > Cc: stable@vger.kernel.org
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9479
> > Fixes: 2feeb52859fc ("drm/i915/gt: Fix -EDEADLK handling regression")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

Say what now?

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/i915: Retry gtt fault when out of fence register

[Greg KH]

On Thu, Oct 12, 2023 at 04:53:38PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 12, 2023 at 03:40:08PM +0200, Greg KH wrote:
> > On Thu, Oct 12, 2023 at 04:28:01PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > If we can't find a free fence register to handle a fault in the GMADR
> > > range just return VM_FAULT_NOPAGE without populating the PTE so that
> > > userspace will retry the access and trigger another fault. Eventually
> > > we should find a free fence and the fault will get properly handled.
> > > 
> > > A further improvement idea might be to reserve a fence (or one per CPU?)
> > > for the express purpose of handling faults without having to retry. But
> > > that would require some additional work.
> > > 
> > > Looks like this may have gotten broken originally by
> > > commit 39965b376601 ("drm/i915: don't trash the gtt when running out of fences")
> > > as that changed the errno to -EDEADLK which wasn't handle by the gtt
> > > fault code either. But later in commit 2feeb52859fc ("drm/i915/gt: Fix
> > > -EDEADLK handling regression") I changed it again to -ENOBUFS as -EDEADLK
> > > was now getting used for the ww mutex dance. So this fix only makes
> > > sense after that last commit.
> > > 
> > > Cc: stable@vger.kernel.org
> > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9479
> > > Fixes: 2feeb52859fc ("drm/i915/gt: Fix -EDEADLK handling regression")
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > 
> > <formletter>
> > 
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read:
> >     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to do this properly.
> > 
> > </formletter>
> 
> Say what now?

Sorry, my bot thought this was a patch sent only to stable, I've kicked
it a bit and it shouldn't do that again...

Re: [Intel-gfx] [PATCH] drm/i915: Retry gtt fault when out of fence register

[Andi Shyti]

Hi Ville,

> If we can't find a free fence register to handle a fault in the GMADR
> range just return VM_FAULT_NOPAGE without populating the PTE so that
> userspace will retry the access and trigger another fault. Eventually
> we should find a free fence and the fault will get properly handled.
> 
> A further improvement idea might be to reserve a fence (or one per CPU?)
> for the express purpose of handling faults without having to retry. But
> that would require some additional work.
> 
> Looks like this may have gotten broken originally by
> commit 39965b376601 ("drm/i915: don't trash the gtt when running out of fences")
> as that changed the errno to -EDEADLK which wasn't handle by the gtt
> fault code either. But later in commit 2feeb52859fc ("drm/i915/gt: Fix
> -EDEADLK handling regression") I changed it again to -ENOBUFS as -EDEADLK
> was now getting used for the ww mutex dance. So this fix only makes
> sense after that last commit.
> 
> Cc: stable@vger.kernel.org
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9479
> Fixes: 2feeb52859fc ("drm/i915/gt: Fix -EDEADLK handling regression")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> 

Andi

Re: [Intel-gfx] [PATCH] drm/i915: Retry gtt fault when out of fence register

[Ville Syrjälä]

On Fri, Oct 13, 2023 at 12:53:59PM +0200, Andi Shyti wrote:
> Hi Ville,
> 
> > If we can't find a free fence register to handle a fault in the GMADR
> > range just return VM_FAULT_NOPAGE without populating the PTE so that
> > userspace will retry the access and trigger another fault. Eventually
> > we should find a free fence and the fault will get properly handled.
> > 
> > A further improvement idea might be to reserve a fence (or one per CPU?)
> > for the express purpose of handling faults without having to retry. But
> > that would require some additional work.
> > 
> > Looks like this may have gotten broken originally by
> > commit 39965b376601 ("drm/i915: don't trash the gtt when running out of fences")
> > as that changed the errno to -EDEADLK which wasn't handle by the gtt
> > fault code either. But later in commit 2feeb52859fc ("drm/i915/gt: Fix
> > -EDEADLK handling regression") I changed it again to -ENOBUFS as -EDEADLK
> > was now getting used for the ww mutex dance. So this fix only makes
> > sense after that last commit.
> > 
> > Cc: stable@vger.kernel.org
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9479
> > Fixes: 2feeb52859fc ("drm/i915/gt: Fix -EDEADLK handling regression")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> 

Thanks. Pushed to gt-next.

-- 
Ville Syrjälä
Intel

Re: [PATCH] drm/i915: Retry gtt fault when out of fence register

[Ville Syrjälä]

On Thu, Oct 12, 2023 at 06:12:26PM +0200, Greg KH wrote:
> On Thu, Oct 12, 2023 at 04:53:38PM +0300, Ville Syrjälä wrote:
> > On Thu, Oct 12, 2023 at 03:40:08PM +0200, Greg KH wrote:
> > > On Thu, Oct 12, 2023 at 04:28:01PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > If we can't find a free fence register to handle a fault in the GMADR
> > > > range just return VM_FAULT_NOPAGE without populating the PTE so that
> > > > userspace will retry the access and trigger another fault. Eventually
> > > > we should find a free fence and the fault will get properly handled.
> > > > 
> > > > A further improvement idea might be to reserve a fence (or one per CPU?)
> > > > for the express purpose of handling faults without having to retry. But
> > > > that would require some additional work.
> > > > 
> > > > Looks like this may have gotten broken originally by
> > > > commit 39965b376601 ("drm/i915: don't trash the gtt when running out of fences")
> > > > as that changed the errno to -EDEADLK which wasn't handle by the gtt
> > > > fault code either. But later in commit 2feeb52859fc ("drm/i915/gt: Fix
> > > > -EDEADLK handling regression") I changed it again to -ENOBUFS as -EDEADLK
> > > > was now getting used for the ww mutex dance. So this fix only makes
> > > > sense after that last commit.
> > > > 
> > > > Cc: stable@vger.kernel.org
> > > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9479
> > > > Fixes: 2feeb52859fc ("drm/i915/gt: Fix -EDEADLK handling regression")
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > 
> > > <formletter>
> > > 
> > > This is not the correct way to submit patches for inclusion in the
> > > stable kernel tree.  Please read:
> > >     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > > for how to do this properly.
> > > 
> > > </formletter>
> > 
> > Say what now?
> 
> Sorry, my bot thought this was a patch sent only to stable, I've kicked
> it a bit and it shouldn't do that again...

Ah OK, thanks.

I was a bit worried that my reading comprehension had deterirated enough
that I couldn't figure iut what new requirement in the process I had
violated :)

-- 
Ville Syrjälä
Intel