All of lore.kernel.org
 help / color / mirror / Atom feed
* drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-14 12:45 ` Sergey Senozhatsky
  0 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2021-06-14 12:45 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Daniel Vetter, David Airlie, Chris Wilson
  Cc: intel-gfx, dri-devel, linux-kernel

Hi,

We are observing some user-space crashes (sigabort, segfaults etc.)
under moderate memory pressure (pretty far from severe pressure) which
have one thing in common - restrictive GFP mask in setup_scratch_page().

For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c

(trimmed down version)

static int gen8_init_scratch(struct i915_address_space *vm)
{
        setup_scratch_page(vm, __GFP_HIGHMEM);

        vm->scratch_pt = alloc_pt(vm);
        vm->scratch_pd = alloc_pd(vm);
        if (use_4lvl(vm)) {
                vm->scratch_pdp = alloc_pdp(vm);
        }
}

gen8_init_scratch() function puts a rather inconsistent restrictions on mm.

Looking at it line by line:

setup_scratch_page() uses very restrictive gfp mask:
	__GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL

it doesn't try to reclaim anything and fails almost immediately.

alloc_pt() - uses more permissive gfp mask:
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

alloc_pd() - likewise:
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

alloc_pdp() - very permissive gfp mask:
	GFP_KERNEL


So can all allocations in gen8_init_scratch() use
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
?

E.g.

---
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a12430187108..e862680b9c93 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
 
        GEM_BUG_ON(!use_4lvl(vm));
 
-       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
+       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
        if (!pdp)
                return ERR_PTR(-ENOMEM);
 
@@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 {
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
        u32 pde;
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
                return -ENOMEM;
        }
 
-       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
+       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
        if (ret) {
                DRM_ERROR("Scratch setup failed\n");
                /* iounmap will also get called at remove, but meh */
---



It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
gfp mask again.

---
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f614646ed3f9..99d78b1052df 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
                return 0;
        }
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
        struct i915_page_directory * const pd = ppgtt->base.pd;
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
                return -ENOMEM;
        }
 
-       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
+       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
        if (ret) {
                DRM_ERROR("Scratch setup failed\n");
                /* iounmap will also get called at remove, but meh */
---

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-14 12:45 ` Sergey Senozhatsky
  0 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2021-06-14 12:45 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Daniel Vetter, David Airlie, Chris Wilson
  Cc: intel-gfx, linux-kernel, dri-devel

Hi,

We are observing some user-space crashes (sigabort, segfaults etc.)
under moderate memory pressure (pretty far from severe pressure) which
have one thing in common - restrictive GFP mask in setup_scratch_page().

For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c

(trimmed down version)

static int gen8_init_scratch(struct i915_address_space *vm)
{
        setup_scratch_page(vm, __GFP_HIGHMEM);

        vm->scratch_pt = alloc_pt(vm);
        vm->scratch_pd = alloc_pd(vm);
        if (use_4lvl(vm)) {
                vm->scratch_pdp = alloc_pdp(vm);
        }
}

gen8_init_scratch() function puts a rather inconsistent restrictions on mm.

Looking at it line by line:

setup_scratch_page() uses very restrictive gfp mask:
	__GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL

it doesn't try to reclaim anything and fails almost immediately.

alloc_pt() - uses more permissive gfp mask:
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

alloc_pd() - likewise:
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

alloc_pdp() - very permissive gfp mask:
	GFP_KERNEL


So can all allocations in gen8_init_scratch() use
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
?

E.g.

---
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a12430187108..e862680b9c93 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
 
        GEM_BUG_ON(!use_4lvl(vm));
 
-       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
+       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
        if (!pdp)
                return ERR_PTR(-ENOMEM);
 
@@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 {
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
        u32 pde;
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
                return -ENOMEM;
        }
 
-       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
+       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
        if (ret) {
                DRM_ERROR("Scratch setup failed\n");
                /* iounmap will also get called at remove, but meh */
---



It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
gfp mask again.

---
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f614646ed3f9..99d78b1052df 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
                return 0;
        }
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
        struct i915_page_directory * const pd = ppgtt->base.pd;
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
                return -ENOMEM;
        }
 
-       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
+       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
        if (ret) {
                DRM_ERROR("Scratch setup failed\n");
                /* iounmap will also get called at remove, but meh */
---

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [Intel-gfx] drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-14 12:45 ` Sergey Senozhatsky
  0 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2021-06-14 12:45 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Daniel Vetter, David Airlie, Chris Wilson
  Cc: intel-gfx, linux-kernel, dri-devel

Hi,

We are observing some user-space crashes (sigabort, segfaults etc.)
under moderate memory pressure (pretty far from severe pressure) which
have one thing in common - restrictive GFP mask in setup_scratch_page().

For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c

(trimmed down version)

static int gen8_init_scratch(struct i915_address_space *vm)
{
        setup_scratch_page(vm, __GFP_HIGHMEM);

        vm->scratch_pt = alloc_pt(vm);
        vm->scratch_pd = alloc_pd(vm);
        if (use_4lvl(vm)) {
                vm->scratch_pdp = alloc_pdp(vm);
        }
}

gen8_init_scratch() function puts a rather inconsistent restrictions on mm.

Looking at it line by line:

setup_scratch_page() uses very restrictive gfp mask:
	__GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL

it doesn't try to reclaim anything and fails almost immediately.

alloc_pt() - uses more permissive gfp mask:
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

alloc_pd() - likewise:
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

alloc_pdp() - very permissive gfp mask:
	GFP_KERNEL


So can all allocations in gen8_init_scratch() use
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
?

E.g.

---
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a12430187108..e862680b9c93 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
 
        GEM_BUG_ON(!use_4lvl(vm));
 
-       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
+       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
        if (!pdp)
                return ERR_PTR(-ENOMEM);
 
@@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 {
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
        u32 pde;
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
                return -ENOMEM;
        }
 
-       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
+       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
        if (ret) {
                DRM_ERROR("Scratch setup failed\n");
                /* iounmap will also get called at remove, but meh */
---



It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
gfp mask again.

---
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f614646ed3f9..99d78b1052df 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
                return 0;
        }
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
        struct i915_page_directory * const pd = ppgtt->base.pd;
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
                return -ENOMEM;
        }
 
-       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
+       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
        if (ret) {
                DRM_ERROR("Scratch setup failed\n");
                /* iounmap will also get called at remove, but meh */
---
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
  2021-06-14 12:45 ` Sergey Senozhatsky
  (?)
  (?)
@ 2021-06-14 23:38 ` Patchwork
  -1 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2021-06-14 23:38 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
URL   : https://patchwork.freedesktop.org/series/91456/
State : failure

== Summary ==

Applying: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/i915_gem_gtt.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
  2021-06-14 12:45 ` Sergey Senozhatsky
  (?)
@ 2021-06-17 17:27   ` Daniel Vetter
  -1 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2021-06-17 17:27 UTC (permalink / raw)
  To: Sergey Senozhatsky, Matthew Auld
  Cc: Jani Nikula, Joonas Lahtinen, Daniel Vetter, David Airlie,
	Chris Wilson, intel-gfx, dri-devel, linux-kernel

On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> Hi,
> 
> We are observing some user-space crashes (sigabort, segfaults etc.)
> under moderate memory pressure (pretty far from severe pressure) which
> have one thing in common - restrictive GFP mask in setup_scratch_page().
> 
> For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> 
> (trimmed down version)
> 
> static int gen8_init_scratch(struct i915_address_space *vm)
> {
>         setup_scratch_page(vm, __GFP_HIGHMEM);
> 
>         vm->scratch_pt = alloc_pt(vm);
>         vm->scratch_pd = alloc_pd(vm);
>         if (use_4lvl(vm)) {
>                 vm->scratch_pdp = alloc_pdp(vm);
>         }
> }
> 
> gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> 
> Looking at it line by line:
> 
> setup_scratch_page() uses very restrictive gfp mask:
> 	__GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> 
> it doesn't try to reclaim anything and fails almost immediately.
> 
> alloc_pt() - uses more permissive gfp mask:
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> 
> alloc_pd() - likewise:
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> 
> alloc_pdp() - very permissive gfp mask:
> 	GFP_KERNEL
> 
> 
> So can all allocations in gen8_init_scratch() use
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

Yeah that looks all fairly broken tbh. The only thing I didn't know was
that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.

The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
seems to be

commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue May 22 09:36:43 2018 +0100

    drm/i915/gtt: Allow pagedirectory allocations to fail

which used a selftest as justification, not real world workloads, so looks
rather dubious.

Adding Matt Auld to this thread, maybe he has ideas.

Thanks, Daniel

> ?
> 
> E.g.
> 
> ---
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a12430187108..e862680b9c93 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
>  
>         GEM_BUG_ON(!use_4lvl(vm));
>  
> -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
>         if (!pdp)
>                 return ERR_PTR(-ENOMEM);
>  
> @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
>  {
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
>         u32 pde;
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
>                 return -ENOMEM;
>         }
>  
> -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
>         if (ret) {
>                 DRM_ERROR("Scratch setup failed\n");
>                 /* iounmap will also get called at remove, but meh */
> ---
> 
> 
> 
> It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> gfp mask again.
> 
> ---
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index f614646ed3f9..99d78b1052df 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
>                 return 0;
>         }
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
>         struct i915_page_directory * const pd = ppgtt->base.pd;
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
>                 return -ENOMEM;
>         }
>  
> -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
>         if (ret) {
>                 DRM_ERROR("Scratch setup failed\n");
>                 /* iounmap will also get called at remove, but meh */
> ---

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-17 17:27   ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2021-06-17 17:27 UTC (permalink / raw)
  To: Sergey Senozhatsky, Matthew Auld
  Cc: David Airlie, intel-gfx, linux-kernel, Chris Wilson, dri-devel

On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> Hi,
> 
> We are observing some user-space crashes (sigabort, segfaults etc.)
> under moderate memory pressure (pretty far from severe pressure) which
> have one thing in common - restrictive GFP mask in setup_scratch_page().
> 
> For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> 
> (trimmed down version)
> 
> static int gen8_init_scratch(struct i915_address_space *vm)
> {
>         setup_scratch_page(vm, __GFP_HIGHMEM);
> 
>         vm->scratch_pt = alloc_pt(vm);
>         vm->scratch_pd = alloc_pd(vm);
>         if (use_4lvl(vm)) {
>                 vm->scratch_pdp = alloc_pdp(vm);
>         }
> }
> 
> gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> 
> Looking at it line by line:
> 
> setup_scratch_page() uses very restrictive gfp mask:
> 	__GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> 
> it doesn't try to reclaim anything and fails almost immediately.
> 
> alloc_pt() - uses more permissive gfp mask:
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> 
> alloc_pd() - likewise:
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> 
> alloc_pdp() - very permissive gfp mask:
> 	GFP_KERNEL
> 
> 
> So can all allocations in gen8_init_scratch() use
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

Yeah that looks all fairly broken tbh. The only thing I didn't know was
that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.

The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
seems to be

commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue May 22 09:36:43 2018 +0100

    drm/i915/gtt: Allow pagedirectory allocations to fail

which used a selftest as justification, not real world workloads, so looks
rather dubious.

Adding Matt Auld to this thread, maybe he has ideas.

Thanks, Daniel

> ?
> 
> E.g.
> 
> ---
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a12430187108..e862680b9c93 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
>  
>         GEM_BUG_ON(!use_4lvl(vm));
>  
> -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
>         if (!pdp)
>                 return ERR_PTR(-ENOMEM);
>  
> @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
>  {
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
>         u32 pde;
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
>                 return -ENOMEM;
>         }
>  
> -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
>         if (ret) {
>                 DRM_ERROR("Scratch setup failed\n");
>                 /* iounmap will also get called at remove, but meh */
> ---
> 
> 
> 
> It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> gfp mask again.
> 
> ---
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index f614646ed3f9..99d78b1052df 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
>                 return 0;
>         }
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
>         struct i915_page_directory * const pd = ppgtt->base.pd;
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
>                 return -ENOMEM;
>         }
>  
> -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
>         if (ret) {
>                 DRM_ERROR("Scratch setup failed\n");
>                 /* iounmap will also get called at remove, but meh */
> ---

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [Intel-gfx] drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-17 17:27   ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2021-06-17 17:27 UTC (permalink / raw)
  To: Sergey Senozhatsky, Matthew Auld
  Cc: David Airlie, intel-gfx, linux-kernel, Chris Wilson, dri-devel

On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> Hi,
> 
> We are observing some user-space crashes (sigabort, segfaults etc.)
> under moderate memory pressure (pretty far from severe pressure) which
> have one thing in common - restrictive GFP mask in setup_scratch_page().
> 
> For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> 
> (trimmed down version)
> 
> static int gen8_init_scratch(struct i915_address_space *vm)
> {
>         setup_scratch_page(vm, __GFP_HIGHMEM);
> 
>         vm->scratch_pt = alloc_pt(vm);
>         vm->scratch_pd = alloc_pd(vm);
>         if (use_4lvl(vm)) {
>                 vm->scratch_pdp = alloc_pdp(vm);
>         }
> }
> 
> gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> 
> Looking at it line by line:
> 
> setup_scratch_page() uses very restrictive gfp mask:
> 	__GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> 
> it doesn't try to reclaim anything and fails almost immediately.
> 
> alloc_pt() - uses more permissive gfp mask:
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> 
> alloc_pd() - likewise:
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> 
> alloc_pdp() - very permissive gfp mask:
> 	GFP_KERNEL
> 
> 
> So can all allocations in gen8_init_scratch() use
> 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

Yeah that looks all fairly broken tbh. The only thing I didn't know was
that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.

The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
seems to be

commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue May 22 09:36:43 2018 +0100

    drm/i915/gtt: Allow pagedirectory allocations to fail

which used a selftest as justification, not real world workloads, so looks
rather dubious.

Adding Matt Auld to this thread, maybe he has ideas.

Thanks, Daniel

> ?
> 
> E.g.
> 
> ---
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index a12430187108..e862680b9c93 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
>  
>         GEM_BUG_ON(!use_4lvl(vm));
>  
> -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
>         if (!pdp)
>                 return ERR_PTR(-ENOMEM);
>  
> @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
>  {
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
>         u32 pde;
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
>                 return -ENOMEM;
>         }
>  
> -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
>         if (ret) {
>                 DRM_ERROR("Scratch setup failed\n");
>                 /* iounmap will also get called at remove, but meh */
> ---
> 
> 
> 
> It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> gfp mask again.
> 
> ---
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index f614646ed3f9..99d78b1052df 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
>                 return 0;
>         }
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
>         struct i915_page_directory * const pd = ppgtt->base.pd;
>         int ret;
>  
> -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
>         if (ret)
>                 return ret;
>  
> @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
>                 return -ENOMEM;
>         }
>  
> -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
>         if (ret) {
>                 DRM_ERROR("Scratch setup failed\n");
>                 /* iounmap will also get called at remove, but meh */
> ---

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
  2021-06-17 17:27   ` Daniel Vetter
@ 2021-06-18  2:29     ` Sergey Senozhatsky
  -1 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2021-06-18  2:29 UTC (permalink / raw)
  To: Sergey Senozhatsky, Matthew Auld, Jani Nikula, Joonas Lahtinen,
	David Airlie, Chris Wilson, intel-gfx, dri-devel, linux-kernel

On (21/06/17 19:27), Daniel Vetter wrote:
> > 
> > So can all allocations in gen8_init_scratch() use
> > 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> 
> Yeah that looks all fairly broken tbh. The only thing I didn't know was
> that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
> guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.

Looks good.

> The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
> seems to be
> 
> commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue May 22 09:36:43 2018 +0100
> 
>     drm/i915/gtt: Allow pagedirectory allocations to fail
> 
> which used a selftest as justification, not real world workloads, so looks
> rather dubious.

Exactly, the commit we landed internally partially reverts 1abb70f5955
in 4.19 and 5.4 kernels. I don't mind I915_GFP_ALLOW_FAIL and so on, I
kept those bits, but we need reclaim. I can reproduce cases when order:0
allocation fails with
	__GFP_HIGHMEM|__GFP_RETRY_MAYFAIL
but succeeds with
	GFP_KERNEL|__GFP_HIGHMEM|__GFP_RETRY_MAYFAIL


ON a side note, I'm not very sure if __GFP_RETRY_MAYFAIL is actually
needed. Especially seeing it in syscalls is a bit uncommon:

  drm_ioctl()
   i915_gem_context_create_ioctl()
    i915_gem_create_context()
     i915_ppgtt_create()
      setup_scratch_page()           // __GFP_RETRY_MAYFAIL

But with GFP_KERNEL at least it tries to make some reclaim progress
between retries, so it seems to be good enough.

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [Intel-gfx] drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-18  2:29     ` Sergey Senozhatsky
  0 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2021-06-18  2:29 UTC (permalink / raw)
  To: Sergey Senozhatsky, Matthew Auld, Jani Nikula, Joonas Lahtinen,
	David Airlie, Chris Wilson, intel-gfx, dri-devel, linux-kernel

On (21/06/17 19:27), Daniel Vetter wrote:
> > 
> > So can all allocations in gen8_init_scratch() use
> > 	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> 
> Yeah that looks all fairly broken tbh. The only thing I didn't know was
> that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
> guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.

Looks good.

> The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
> seems to be
> 
> commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue May 22 09:36:43 2018 +0100
> 
>     drm/i915/gtt: Allow pagedirectory allocations to fail
> 
> which used a selftest as justification, not real world workloads, so looks
> rather dubious.

Exactly, the commit we landed internally partially reverts 1abb70f5955
in 4.19 and 5.4 kernels. I don't mind I915_GFP_ALLOW_FAIL and so on, I
kept those bits, but we need reclaim. I can reproduce cases when order:0
allocation fails with
	__GFP_HIGHMEM|__GFP_RETRY_MAYFAIL
but succeeds with
	GFP_KERNEL|__GFP_HIGHMEM|__GFP_RETRY_MAYFAIL


ON a side note, I'm not very sure if __GFP_RETRY_MAYFAIL is actually
needed. Especially seeing it in syscalls is a bit uncommon:

  drm_ioctl()
   i915_gem_context_create_ioctl()
    i915_gem_create_context()
     i915_ppgtt_create()
      setup_scratch_page()           // __GFP_RETRY_MAYFAIL

But with GFP_KERNEL at least it tries to make some reclaim progress
between retries, so it seems to be good enough.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
  2021-06-17 17:27   ` Daniel Vetter
  (?)
@ 2021-06-18 15:46     ` Matthew Auld
  -1 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2021-06-18 15:46 UTC (permalink / raw)
  To: Sergey Senozhatsky, Matthew Auld, Jani Nikula, Joonas Lahtinen,
	David Airlie, Chris Wilson, Intel Graphics Development,
	ML dri-devel, kernel list
  Cc: Daniel Vetter

On Thu, 17 Jun 2021 at 18:27, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> > Hi,
> >
> > We are observing some user-space crashes (sigabort, segfaults etc.)
> > under moderate memory pressure (pretty far from severe pressure) which
> > have one thing in common - restrictive GFP mask in setup_scratch_page().
> >
> > For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> >
> > (trimmed down version)
> >
> > static int gen8_init_scratch(struct i915_address_space *vm)
> > {
> >         setup_scratch_page(vm, __GFP_HIGHMEM);
> >
> >         vm->scratch_pt = alloc_pt(vm);
> >         vm->scratch_pd = alloc_pd(vm);
> >         if (use_4lvl(vm)) {
> >                 vm->scratch_pdp = alloc_pdp(vm);
> >         }
> > }
> >
> > gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> >
> > Looking at it line by line:
> >
> > setup_scratch_page() uses very restrictive gfp mask:
> >       __GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> >
> > it doesn't try to reclaim anything and fails almost immediately.
> >
> > alloc_pt() - uses more permissive gfp mask:
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > alloc_pd() - likewise:
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > alloc_pdp() - very permissive gfp mask:
> >       GFP_KERNEL
> >
> >
> > So can all allocations in gen8_init_scratch() use
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
>
> Yeah that looks all fairly broken tbh. The only thing I didn't know was
> that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
> guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.
>
> The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
> seems to be
>
> commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue May 22 09:36:43 2018 +0100
>
>     drm/i915/gtt: Allow pagedirectory allocations to fail
>
> which used a selftest as justification, not real world workloads, so looks
> rather dubious.
>
> Adding Matt Auld to this thread, maybe he has ideas.

The latest code is quite different, but for both scratch and the
various paging structures it's now sharing the same GFP
flags(I915_GFP_ALLOW_FAIL). And for the actual backing page, which is
now a GEM object, we use i915_gem_object_get_pages_internal().

Not sure why scratch wants to be different, and I don't recall
anything funny. At first I thought it might have been related to
needing only one scratch page/directory etc which was then shared
between different VMs, but I don't think we had read-only support in
the driver at that point, so can't be that. But I guess once we did
add that seeing failures in init_scratch() was very unlikely, at least
until gen11+ arrived which then broke read-only support in the HW.

>
> Thanks, Daniel
>
> > ?
> >
> > E.g.
> >
> > ---
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index a12430187108..e862680b9c93 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
> >
> >         GEM_BUG_ON(!use_4lvl(vm));
> >
> > -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> > +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
> >         if (!pdp)
> >                 return ERR_PTR(-ENOMEM);
> >
> > @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> >  {
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
> >         u32 pde;
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> >                 return -ENOMEM;
> >         }
> >
> > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> >         if (ret) {
> >                 DRM_ERROR("Scratch setup failed\n");
> >                 /* iounmap will also get called at remove, but meh */
> > ---
> >
> >
> >
> > It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> > gfp mask again.
> >
> > ---
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index f614646ed3f9..99d78b1052df 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> >                 return 0;
> >         }
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
> >         struct i915_page_directory * const pd = ppgtt->base.pd;
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> >                 return -ENOMEM;
> >         }
> >
> > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> >         if (ret) {
> >                 DRM_ERROR("Scratch setup failed\n");
> >                 /* iounmap will also get called at remove, but meh */
> > ---
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-18 15:46     ` Matthew Auld
  0 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2021-06-18 15:46 UTC (permalink / raw)
  To: Sergey Senozhatsky, Matthew Auld, Jani Nikula, Joonas Lahtinen,
	David Airlie, Chris Wilson, Intel Graphics Development,
	ML dri-devel, kernel list

On Thu, 17 Jun 2021 at 18:27, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> > Hi,
> >
> > We are observing some user-space crashes (sigabort, segfaults etc.)
> > under moderate memory pressure (pretty far from severe pressure) which
> > have one thing in common - restrictive GFP mask in setup_scratch_page().
> >
> > For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> >
> > (trimmed down version)
> >
> > static int gen8_init_scratch(struct i915_address_space *vm)
> > {
> >         setup_scratch_page(vm, __GFP_HIGHMEM);
> >
> >         vm->scratch_pt = alloc_pt(vm);
> >         vm->scratch_pd = alloc_pd(vm);
> >         if (use_4lvl(vm)) {
> >                 vm->scratch_pdp = alloc_pdp(vm);
> >         }
> > }
> >
> > gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> >
> > Looking at it line by line:
> >
> > setup_scratch_page() uses very restrictive gfp mask:
> >       __GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> >
> > it doesn't try to reclaim anything and fails almost immediately.
> >
> > alloc_pt() - uses more permissive gfp mask:
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > alloc_pd() - likewise:
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > alloc_pdp() - very permissive gfp mask:
> >       GFP_KERNEL
> >
> >
> > So can all allocations in gen8_init_scratch() use
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
>
> Yeah that looks all fairly broken tbh. The only thing I didn't know was
> that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
> guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.
>
> The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
> seems to be
>
> commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue May 22 09:36:43 2018 +0100
>
>     drm/i915/gtt: Allow pagedirectory allocations to fail
>
> which used a selftest as justification, not real world workloads, so looks
> rather dubious.
>
> Adding Matt Auld to this thread, maybe he has ideas.

The latest code is quite different, but for both scratch and the
various paging structures it's now sharing the same GFP
flags(I915_GFP_ALLOW_FAIL). And for the actual backing page, which is
now a GEM object, we use i915_gem_object_get_pages_internal().

Not sure why scratch wants to be different, and I don't recall
anything funny. At first I thought it might have been related to
needing only one scratch page/directory etc which was then shared
between different VMs, but I don't think we had read-only support in
the driver at that point, so can't be that. But I guess once we did
add that seeing failures in init_scratch() was very unlikely, at least
until gen11+ arrived which then broke read-only support in the HW.

>
> Thanks, Daniel
>
> > ?
> >
> > E.g.
> >
> > ---
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index a12430187108..e862680b9c93 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
> >
> >         GEM_BUG_ON(!use_4lvl(vm));
> >
> > -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> > +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
> >         if (!pdp)
> >                 return ERR_PTR(-ENOMEM);
> >
> > @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> >  {
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
> >         u32 pde;
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> >                 return -ENOMEM;
> >         }
> >
> > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> >         if (ret) {
> >                 DRM_ERROR("Scratch setup failed\n");
> >                 /* iounmap will also get called at remove, but meh */
> > ---
> >
> >
> >
> > It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> > gfp mask again.
> >
> > ---
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index f614646ed3f9..99d78b1052df 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> >                 return 0;
> >         }
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
> >         struct i915_page_directory * const pd = ppgtt->base.pd;
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> >                 return -ENOMEM;
> >         }
> >
> > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> >         if (ret) {
> >                 DRM_ERROR("Scratch setup failed\n");
> >                 /* iounmap will also get called at remove, but meh */
> > ---
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [Intel-gfx] drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-18 15:46     ` Matthew Auld
  0 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2021-06-18 15:46 UTC (permalink / raw)
  To: Sergey Senozhatsky, Matthew Auld, Jani Nikula, Joonas Lahtinen,
	David Airlie, Chris Wilson, Intel Graphics Development,
	ML dri-devel, kernel list

On Thu, 17 Jun 2021 at 18:27, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> > Hi,
> >
> > We are observing some user-space crashes (sigabort, segfaults etc.)
> > under moderate memory pressure (pretty far from severe pressure) which
> > have one thing in common - restrictive GFP mask in setup_scratch_page().
> >
> > For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> >
> > (trimmed down version)
> >
> > static int gen8_init_scratch(struct i915_address_space *vm)
> > {
> >         setup_scratch_page(vm, __GFP_HIGHMEM);
> >
> >         vm->scratch_pt = alloc_pt(vm);
> >         vm->scratch_pd = alloc_pd(vm);
> >         if (use_4lvl(vm)) {
> >                 vm->scratch_pdp = alloc_pdp(vm);
> >         }
> > }
> >
> > gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> >
> > Looking at it line by line:
> >
> > setup_scratch_page() uses very restrictive gfp mask:
> >       __GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> >
> > it doesn't try to reclaim anything and fails almost immediately.
> >
> > alloc_pt() - uses more permissive gfp mask:
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > alloc_pd() - likewise:
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > alloc_pdp() - very permissive gfp mask:
> >       GFP_KERNEL
> >
> >
> > So can all allocations in gen8_init_scratch() use
> >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
>
> Yeah that looks all fairly broken tbh. The only thing I didn't know was
> that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
> guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.
>
> The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
> seems to be
>
> commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Tue May 22 09:36:43 2018 +0100
>
>     drm/i915/gtt: Allow pagedirectory allocations to fail
>
> which used a selftest as justification, not real world workloads, so looks
> rather dubious.
>
> Adding Matt Auld to this thread, maybe he has ideas.

The latest code is quite different, but for both scratch and the
various paging structures it's now sharing the same GFP
flags(I915_GFP_ALLOW_FAIL). And for the actual backing page, which is
now a GEM object, we use i915_gem_object_get_pages_internal().

Not sure why scratch wants to be different, and I don't recall
anything funny. At first I thought it might have been related to
needing only one scratch page/directory etc which was then shared
between different VMs, but I don't think we had read-only support in
the driver at that point, so can't be that. But I guess once we did
add that seeing failures in init_scratch() was very unlikely, at least
until gen11+ arrived which then broke read-only support in the HW.

>
> Thanks, Daniel
>
> > ?
> >
> > E.g.
> >
> > ---
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index a12430187108..e862680b9c93 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
> >
> >         GEM_BUG_ON(!use_4lvl(vm));
> >
> > -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> > +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
> >         if (!pdp)
> >                 return ERR_PTR(-ENOMEM);
> >
> > @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> >  {
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
> >         u32 pde;
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> >                 return -ENOMEM;
> >         }
> >
> > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> >         if (ret) {
> >                 DRM_ERROR("Scratch setup failed\n");
> >                 /* iounmap will also get called at remove, but meh */
> > ---
> >
> >
> >
> > It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> > gfp mask again.
> >
> > ---
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index f614646ed3f9..99d78b1052df 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> >                 return 0;
> >         }
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
> >         struct i915_page_directory * const pd = ppgtt->base.pd;
> >         int ret;
> >
> > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> >         if (ret)
> >                 return ret;
> >
> > @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> >                 return -ENOMEM;
> >         }
> >
> > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> >         if (ret) {
> >                 DRM_ERROR("Scratch setup failed\n");
> >                 /* iounmap will also get called at remove, but meh */
> > ---
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
  2021-06-18 15:46     ` Matthew Auld
  (?)
@ 2021-06-21 14:10       ` Daniel Vetter
  -1 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2021-06-21 14:10 UTC (permalink / raw)
  To: Matthew Auld
  Cc: Sergey Senozhatsky, Jani Nikula, Joonas Lahtinen, David Airlie,
	Chris Wilson, Intel Graphics Development, ML dri-devel,
	kernel list, Daniel Vetter

On Fri, Jun 18, 2021 at 04:46:24PM +0100, Matthew Auld wrote:
> On Thu, 17 Jun 2021 at 18:27, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> > > Hi,
> > >
> > > We are observing some user-space crashes (sigabort, segfaults etc.)
> > > under moderate memory pressure (pretty far from severe pressure) which
> > > have one thing in common - restrictive GFP mask in setup_scratch_page().
> > >
> > > For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> > >
> > > (trimmed down version)
> > >
> > > static int gen8_init_scratch(struct i915_address_space *vm)
> > > {
> > >         setup_scratch_page(vm, __GFP_HIGHMEM);
> > >
> > >         vm->scratch_pt = alloc_pt(vm);
> > >         vm->scratch_pd = alloc_pd(vm);
> > >         if (use_4lvl(vm)) {
> > >                 vm->scratch_pdp = alloc_pdp(vm);
> > >         }
> > > }
> > >
> > > gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> > >
> > > Looking at it line by line:
> > >
> > > setup_scratch_page() uses very restrictive gfp mask:
> > >       __GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> > >
> > > it doesn't try to reclaim anything and fails almost immediately.
> > >
> > > alloc_pt() - uses more permissive gfp mask:
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> > >
> > > alloc_pd() - likewise:
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> > >
> > > alloc_pdp() - very permissive gfp mask:
> > >       GFP_KERNEL
> > >
> > >
> > > So can all allocations in gen8_init_scratch() use
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > Yeah that looks all fairly broken tbh. The only thing I didn't know was
> > that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
> > guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.
> >
> > The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
> > seems to be
> >
> > commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Tue May 22 09:36:43 2018 +0100
> >
> >     drm/i915/gtt: Allow pagedirectory allocations to fail
> >
> > which used a selftest as justification, not real world workloads, so looks
> > rather dubious.
> >
> > Adding Matt Auld to this thread, maybe he has ideas.
> 
> The latest code is quite different, but for both scratch and the
> various paging structures it's now sharing the same GFP
> flags(I915_GFP_ALLOW_FAIL). And for the actual backing page, which is
> now a GEM object, we use i915_gem_object_get_pages_internal().
> 
> Not sure why scratch wants to be different, and I don't recall
> anything funny. At first I thought it might have been related to
> needing only one scratch page/directory etc which was then shared
> between different VMs, but I don't think we had read-only support in
> the driver at that point, so can't be that. But I guess once we did
> add that seeing failures in init_scratch() was very unlikely, at least
> until gen11+ arrived which then broke read-only support in the HW.

If there is something, then shmem get_pages has some reason to use
__GFP_RETRY_MAYFAIL - at least way back when dev->struct_mutex was still
everywhere we had some paths to directly reclaim gem bo when the
allocations failed.

But I think that all disappeared, so all the reasons for MAYFAIL have gone
away - if there's no fallback or call to our own shrinker or anything like
that, then we must rely on core mm to try really hard to find the memory
we want.

This all goes back to

commit 07f73f6912667621276b002e33844ef283d98203
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Sep 14 16:50:30 2009 +0100

    drm/i915: Improve behaviour under memory pressure

but with a lot of detours and confusion going on (__GFP_NORETRY wasn't
actually what we wanted, which is why __GFP_RETRY_MAYFAIL now exists).
-Daniel

> 
> >
> > Thanks, Daniel
> >
> > > ?
> > >
> > > E.g.
> > >
> > > ---
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index a12430187108..e862680b9c93 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
> > >
> > >         GEM_BUG_ON(!use_4lvl(vm));
> > >
> > > -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> > > +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
> > >         if (!pdp)
> > >                 return ERR_PTR(-ENOMEM);
> > >
> > > @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> > >  {
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
> > >         u32 pde;
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> > >                 return -ENOMEM;
> > >         }
> > >
> > > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> > >         if (ret) {
> > >                 DRM_ERROR("Scratch setup failed\n");
> > >                 /* iounmap will also get called at remove, but meh */
> > > ---
> > >
> > >
> > >
> > > It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> > > gfp mask again.
> > >
> > > ---
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index f614646ed3f9..99d78b1052df 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> > >                 return 0;
> > >         }
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
> > >         struct i915_page_directory * const pd = ppgtt->base.pd;
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> > >                 return -ENOMEM;
> > >         }
> > >
> > > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> > >         if (ret) {
> > >                 DRM_ERROR("Scratch setup failed\n");
> > >                 /* iounmap will also get called at remove, but meh */
> > > ---
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-21 14:10       ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2021-06-21 14:10 UTC (permalink / raw)
  To: Matthew Auld
  Cc: David Airlie, Intel Graphics Development, kernel list,
	Chris Wilson, Sergey Senozhatsky, ML dri-devel

On Fri, Jun 18, 2021 at 04:46:24PM +0100, Matthew Auld wrote:
> On Thu, 17 Jun 2021 at 18:27, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> > > Hi,
> > >
> > > We are observing some user-space crashes (sigabort, segfaults etc.)
> > > under moderate memory pressure (pretty far from severe pressure) which
> > > have one thing in common - restrictive GFP mask in setup_scratch_page().
> > >
> > > For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> > >
> > > (trimmed down version)
> > >
> > > static int gen8_init_scratch(struct i915_address_space *vm)
> > > {
> > >         setup_scratch_page(vm, __GFP_HIGHMEM);
> > >
> > >         vm->scratch_pt = alloc_pt(vm);
> > >         vm->scratch_pd = alloc_pd(vm);
> > >         if (use_4lvl(vm)) {
> > >                 vm->scratch_pdp = alloc_pdp(vm);
> > >         }
> > > }
> > >
> > > gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> > >
> > > Looking at it line by line:
> > >
> > > setup_scratch_page() uses very restrictive gfp mask:
> > >       __GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> > >
> > > it doesn't try to reclaim anything and fails almost immediately.
> > >
> > > alloc_pt() - uses more permissive gfp mask:
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> > >
> > > alloc_pd() - likewise:
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> > >
> > > alloc_pdp() - very permissive gfp mask:
> > >       GFP_KERNEL
> > >
> > >
> > > So can all allocations in gen8_init_scratch() use
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > Yeah that looks all fairly broken tbh. The only thing I didn't know was
> > that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
> > guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.
> >
> > The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
> > seems to be
> >
> > commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Tue May 22 09:36:43 2018 +0100
> >
> >     drm/i915/gtt: Allow pagedirectory allocations to fail
> >
> > which used a selftest as justification, not real world workloads, so looks
> > rather dubious.
> >
> > Adding Matt Auld to this thread, maybe he has ideas.
> 
> The latest code is quite different, but for both scratch and the
> various paging structures it's now sharing the same GFP
> flags(I915_GFP_ALLOW_FAIL). And for the actual backing page, which is
> now a GEM object, we use i915_gem_object_get_pages_internal().
> 
> Not sure why scratch wants to be different, and I don't recall
> anything funny. At first I thought it might have been related to
> needing only one scratch page/directory etc which was then shared
> between different VMs, but I don't think we had read-only support in
> the driver at that point, so can't be that. But I guess once we did
> add that seeing failures in init_scratch() was very unlikely, at least
> until gen11+ arrived which then broke read-only support in the HW.

If there is something, then shmem get_pages has some reason to use
__GFP_RETRY_MAYFAIL - at least way back when dev->struct_mutex was still
everywhere we had some paths to directly reclaim gem bo when the
allocations failed.

But I think that all disappeared, so all the reasons for MAYFAIL have gone
away - if there's no fallback or call to our own shrinker or anything like
that, then we must rely on core mm to try really hard to find the memory
we want.

This all goes back to

commit 07f73f6912667621276b002e33844ef283d98203
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Sep 14 16:50:30 2009 +0100

    drm/i915: Improve behaviour under memory pressure

but with a lot of detours and confusion going on (__GFP_NORETRY wasn't
actually what we wanted, which is why __GFP_RETRY_MAYFAIL now exists).
-Daniel

> 
> >
> > Thanks, Daniel
> >
> > > ?
> > >
> > > E.g.
> > >
> > > ---
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index a12430187108..e862680b9c93 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
> > >
> > >         GEM_BUG_ON(!use_4lvl(vm));
> > >
> > > -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> > > +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
> > >         if (!pdp)
> > >                 return ERR_PTR(-ENOMEM);
> > >
> > > @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> > >  {
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
> > >         u32 pde;
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> > >                 return -ENOMEM;
> > >         }
> > >
> > > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> > >         if (ret) {
> > >                 DRM_ERROR("Scratch setup failed\n");
> > >                 /* iounmap will also get called at remove, but meh */
> > > ---
> > >
> > >
> > >
> > > It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> > > gfp mask again.
> > >
> > > ---
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index f614646ed3f9..99d78b1052df 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> > >                 return 0;
> > >         }
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
> > >         struct i915_page_directory * const pd = ppgtt->base.pd;
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> > >                 return -ENOMEM;
> > >         }
> > >
> > > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> > >         if (ret) {
> > >                 DRM_ERROR("Scratch setup failed\n");
> > >                 /* iounmap will also get called at remove, but meh */
> > > ---
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [Intel-gfx] drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels
@ 2021-06-21 14:10       ` Daniel Vetter
  0 siblings, 0 replies; 15+ messages in thread
From: Daniel Vetter @ 2021-06-21 14:10 UTC (permalink / raw)
  To: Matthew Auld
  Cc: David Airlie, Intel Graphics Development, kernel list,
	Chris Wilson, Sergey Senozhatsky, ML dri-devel

On Fri, Jun 18, 2021 at 04:46:24PM +0100, Matthew Auld wrote:
> On Thu, 17 Jun 2021 at 18:27, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Mon, Jun 14, 2021 at 09:45:37PM +0900, Sergey Senozhatsky wrote:
> > > Hi,
> > >
> > > We are observing some user-space crashes (sigabort, segfaults etc.)
> > > under moderate memory pressure (pretty far from severe pressure) which
> > > have one thing in common - restrictive GFP mask in setup_scratch_page().
> > >
> > > For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c
> > >
> > > (trimmed down version)
> > >
> > > static int gen8_init_scratch(struct i915_address_space *vm)
> > > {
> > >         setup_scratch_page(vm, __GFP_HIGHMEM);
> > >
> > >         vm->scratch_pt = alloc_pt(vm);
> > >         vm->scratch_pd = alloc_pd(vm);
> > >         if (use_4lvl(vm)) {
> > >                 vm->scratch_pdp = alloc_pdp(vm);
> > >         }
> > > }
> > >
> > > gen8_init_scratch() function puts a rather inconsistent restrictions on mm.
> > >
> > > Looking at it line by line:
> > >
> > > setup_scratch_page() uses very restrictive gfp mask:
> > >       __GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL
> > >
> > > it doesn't try to reclaim anything and fails almost immediately.
> > >
> > > alloc_pt() - uses more permissive gfp mask:
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> > >
> > > alloc_pd() - likewise:
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> > >
> > > alloc_pdp() - very permissive gfp mask:
> > >       GFP_KERNEL
> > >
> > >
> > > So can all allocations in gen8_init_scratch() use
> > >       GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
> >
> > Yeah that looks all fairly broken tbh. The only thing I didn't know was
> > that GFP_DMA32 wasn't a full gfp mask with reclaim bits set as needed. I
> > guess it would be clearer if we use GFP_KERNEL | __GFP_DMA32 for these.
> >
> > The commit that introduced a lot of this, including I915_GFP_ALLOW_FAIL
> > seems to be
> >
> > commit 1abb70f5955d1a9021f96359a2c6502ca569b68d
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Tue May 22 09:36:43 2018 +0100
> >
> >     drm/i915/gtt: Allow pagedirectory allocations to fail
> >
> > which used a selftest as justification, not real world workloads, so looks
> > rather dubious.
> >
> > Adding Matt Auld to this thread, maybe he has ideas.
> 
> The latest code is quite different, but for both scratch and the
> various paging structures it's now sharing the same GFP
> flags(I915_GFP_ALLOW_FAIL). And for the actual backing page, which is
> now a GEM object, we use i915_gem_object_get_pages_internal().
> 
> Not sure why scratch wants to be different, and I don't recall
> anything funny. At first I thought it might have been related to
> needing only one scratch page/directory etc which was then shared
> between different VMs, but I don't think we had read-only support in
> the driver at that point, so can't be that. But I guess once we did
> add that seeing failures in init_scratch() was very unlikely, at least
> until gen11+ arrived which then broke read-only support in the HW.

If there is something, then shmem get_pages has some reason to use
__GFP_RETRY_MAYFAIL - at least way back when dev->struct_mutex was still
everywhere we had some paths to directly reclaim gem bo when the
allocations failed.

But I think that all disappeared, so all the reasons for MAYFAIL have gone
away - if there's no fallback or call to our own shrinker or anything like
that, then we must rely on core mm to try really hard to find the memory
we want.

This all goes back to

commit 07f73f6912667621276b002e33844ef283d98203
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Sep 14 16:50:30 2009 +0100

    drm/i915: Improve behaviour under memory pressure

but with a lot of detours and confusion going on (__GFP_NORETRY wasn't
actually what we wanted, which is why __GFP_RETRY_MAYFAIL now exists).
-Daniel

> 
> >
> > Thanks, Daniel
> >
> > > ?
> > >
> > > E.g.
> > >
> > > ---
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index a12430187108..e862680b9c93 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
> > >
> > >         GEM_BUG_ON(!use_4lvl(vm));
> > >
> > > -       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
> > > +       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
> > >         if (!pdp)
> > >                 return ERR_PTR(-ENOMEM);
> > >
> > > @@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> > >  {
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
> > >         u32 pde;
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> > >                 return -ENOMEM;
> > >         }
> > >
> > > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> > >         if (ret) {
> > >                 DRM_ERROR("Scratch setup failed\n");
> > >                 /* iounmap will also get called at remove, but meh */
> > > ---
> > >
> > >
> > >
> > > It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
> > > gfp mask again.
> > >
> > > ---
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index f614646ed3f9..99d78b1052df 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
> > >                 return 0;
> > >         }
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
> > >         struct i915_page_directory * const pd = ppgtt->base.pd;
> > >         int ret;
> > >
> > > -       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
> > > +       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
> > >         if (ret)
> > >                 return ret;
> > >
> > > @@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
> > >                 return -ENOMEM;
> > >         }
> > >
> > > -       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
> > > +       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
> > >         if (ret) {
> > >                 DRM_ERROR("Scratch setup failed\n");
> > >                 /* iounmap will also get called at remove, but meh */
> > > ---
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-06-21 14:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-14 12:45 drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels Sergey Senozhatsky
2021-06-14 12:45 ` [Intel-gfx] " Sergey Senozhatsky
2021-06-14 12:45 ` Sergey Senozhatsky
2021-06-14 23:38 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork
2021-06-17 17:27 ` Daniel Vetter
2021-06-17 17:27   ` [Intel-gfx] " Daniel Vetter
2021-06-17 17:27   ` Daniel Vetter
2021-06-18  2:29   ` Sergey Senozhatsky
2021-06-18  2:29     ` [Intel-gfx] " Sergey Senozhatsky
2021-06-18 15:46   ` Matthew Auld
2021-06-18 15:46     ` [Intel-gfx] " Matthew Auld
2021-06-18 15:46     ` Matthew Auld
2021-06-21 14:10     ` Daniel Vetter
2021-06-21 14:10       ` [Intel-gfx] " Daniel Vetter
2021-06-21 14:10       ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.