Re: [Intel-gfx] [PATCH 1/3] drm/i915: Allocate intel_engine_coredump_alloc with ALLOW_FAIL

John Harrison
2022-01-19T01:30:45Z

On 1/18/2022 13:43, Matthew Brost wrote:
> Allocate intel_engine_coredump_alloc with ALLOW_FAIL rather than
> GFP_KERNEL do fully decouple the error capture from fence signalling.
s/do/to/

>
> Fixes: 8b91cdd4f8649 ("drm/i915: Use __GFP_KSWAPD_RECLAIM in the capture code")
Does this really count as a bug fix over that patch? Isn't it more of a
changing in policy now that we do DRM fence signalling and that other
changes related to error capture behaviour have been implemented.

>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gpu_error.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 67f3515f07e7a..aee42eae4729f 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1516,7 +1516,7 @@ capture_engine(struct intel_engine_cs *engine,
> struct i915_request *rq = NULL;
> unsigned long flags;
>
> - ee = intel_engine_coredump_alloc(engine, GFP_KERNEL);
> + ee = intel_engine_coredump_alloc(engine, ALLOW_FAIL);
This still makes me nervous that we will fail to allocate engine
captures in stress test scenarios, which are exactly the kind of
situations where we need valid error captures.

There is also still a GFP_KERNEL in __i915_error_grow(). Doesn't that
need updating as well?

John.

> if (!ee)
> return NULL;
>