Re: [PATCH v6 2/4] drm/i915: Use __GFP_KSWAPD_RECLAIM in the capture code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-11-08 at 18:45:45 +0100, Thomas Hellström wrote:
> The capture code is typically run entirely in the fence signalling
> critical path. We're about to add lockdep annotation in an upcoming patch
> which reveals a lockdep splat similar to the below one.
> 
> Fix the associated potential deadlocks using __GFP_KSWAPD_RECLAIM
> (which is the same as GFP_WAIT, but open-coded for clarity) rather than
> GFP_KERNEL for memory allocation in the capture path. This has the
> potential drawback that capture might fail in situations with memory
> pressure.
> 
> [  234.842048] WARNING: possible circular locking dependency detected
> [  234.842050] 5.15.0-rc7+ #20 Tainted: G     U  W
> [  234.842052] ------------------------------------------------------
> [  234.842054] gem_exec_captur/1180 is trying to acquire lock:
> [  234.842056] ffffffffa3e51c00 (fs_reclaim){+.+.}-{0:0}, at: __kmalloc+0x4d/0x330
> [  234.842063]
>                but task is already holding lock:
> [  234.842064] ffffffffa3f57620 (dma_fence_map){++++}-{0:0}, at: i915_vma_snapshot_resource_pin+0x27/0x30 [i915]
> [  234.842138]
>                which lock already depends on the new lock.
> 
> [  234.842140]
>                the existing dependency chain (in reverse order) is:
> [  234.842142]
>                -> #2 (dma_fence_map){++++}-{0:0}:
> [  234.842145]        __dma_fence_might_wait+0x41/0xa0
> [  234.842149]        dma_resv_lockdep+0x1dc/0x28f
> [  234.842151]        do_one_initcall+0x58/0x2d0
> [  234.842154]        kernel_init_freeable+0x273/0x2bf
> [  234.842157]        kernel_init+0x16/0x120
> [  234.842160]        ret_from_fork+0x1f/0x30
> [  234.842163]
>                -> #1 (mmu_notifier_invalidate_range_start){+.+.}-{0:0}:
> [  234.842166]        fs_reclaim_acquire+0x6d/0xd0
> [  234.842168]        __kmalloc_node+0x51/0x3a0
> [  234.842171]        alloc_cpumask_var_node+0x1b/0x30
> [  234.842174]        native_smp_prepare_cpus+0xc7/0x292
> [  234.842177]        kernel_init_freeable+0x160/0x2bf
> [  234.842179]        kernel_init+0x16/0x120
> [  234.842181]        ret_from_fork+0x1f/0x30
> [  234.842184]
>                -> #0 (fs_reclaim){+.+.}-{0:0}:
> [  234.842186]        __lock_acquire+0x1161/0x1dc0
> [  234.842189]        lock_acquire+0xb5/0x2b0
> [  234.842192]        fs_reclaim_acquire+0xa1/0xd0
> [  234.842193]        __kmalloc+0x4d/0x330
> [  234.842196]        i915_vma_coredump_create+0x78/0x5b0 [i915]
> [  234.842253]        intel_engine_coredump_add_vma+0x36/0xe0 [i915]
> [  234.842307]        __i915_gpu_coredump+0x290/0x5e0 [i915]
> [  234.842365]        i915_capture_error_state+0x57/0xa0 [i915]
> [  234.842415]        intel_gt_handle_error+0x348/0x3e0 [i915]
> [  234.842462]        intel_gt_debugfs_reset_store+0x3c/0x90 [i915]
> [  234.842504]        simple_attr_write+0xc1/0xe0
> [  234.842507]        full_proxy_write+0x53/0x80
> [  234.842509]        vfs_write+0xbc/0x350
> [  234.842513]        ksys_write+0x58/0xd0
> [  234.842514]        do_syscall_64+0x38/0x90
> [  234.842516]        entry_SYSCALL_64_after_hwframe+0x44/0xae
> [  234.842519]
>                other info that might help us debug this:
> 
> [  234.842521] Chain exists of:
>                  fs_reclaim --> mmu_notifier_invalidate_range_start --> dma_fence_map
> 
> [  234.842526]  Possible unsafe locking scenario:
> 
> [  234.842528]        CPU0                    CPU1
> [  234.842529]        ----                    ----
> [  234.842531]   lock(dma_fence_map);
> [  234.842532]                                lock(mmu_notifier_invalidate_range_start);
> [  234.842535]                                lock(dma_fence_map);
> [  234.842537]   lock(fs_reclaim);
> [  234.842539]
>                 *** DEADLOCK ***
> 
> [  234.842540] 4 locks held by gem_exec_captur/1180:
> [  234.842543]  #0: ffff9007812d9460 (sb_writers#17){.+.+}-{0:0}, at: ksys_write+0x58/0xd0
> [  234.842547]  #1: ffff900781d9ecb8 (&attr->mutex){+.+.}-{3:3}, at: simple_attr_write+0x3a/0xe0
> [  234.842552]  #2: ffffffffc11913a8 (capture_mutex){+.+.}-{3:3}, at: i915_capture_error_state+0x1a/0xa0 [i915]
> [  234.842602]  #3: ffffffffa3f57620 (dma_fence_map){++++}-{0:0}, at: i915_vma_snapshot_resource_pin+0x27/0x30 [i915]
> [  234.842656]
>                stack backtrace:
> [  234.842658] CPU: 0 PID: 1180 Comm: gem_exec_captur Tainted: G     U  W         5.15.0-rc7+ #20
> [  234.842661] Hardware name: ASUS System Product Name/PRIME B560M-A AC, BIOS 0403 01/26/2021
> [  234.842664] Call Trace:
> [  234.842666]  dump_stack_lvl+0x57/0x72
> [  234.842669]  check_noncircular+0xde/0x100
> [  234.842672]  ? __lock_acquire+0x3bf/0x1dc0
> [  234.842675]  __lock_acquire+0x1161/0x1dc0
> [  234.842678]  lock_acquire+0xb5/0x2b0
> [  234.842680]  ? __kmalloc+0x4d/0x330
> [  234.842683]  ? finish_task_switch.isra.0+0xf2/0x360
> [  234.842686]  ? i915_vma_coredump_create+0x78/0x5b0 [i915]
> [  234.842734]  fs_reclaim_acquire+0xa1/0xd0
> [  234.842737]  ? __kmalloc+0x4d/0x330
> [  234.842739]  __kmalloc+0x4d/0x330
> [  234.842742]  i915_vma_coredump_create+0x78/0x5b0 [i915]
> [  234.842793]  ? capture_vma+0xbe/0x110 [i915]
> [  234.842844]  intel_engine_coredump_add_vma+0x36/0xe0 [i915]
> [  234.842892]  __i915_gpu_coredump+0x290/0x5e0 [i915]
> [  234.842939]  i915_capture_error_state+0x57/0xa0 [i915]
> [  234.842985]  intel_gt_handle_error+0x348/0x3e0 [i915]
> [  234.843032]  ? __mutex_lock+0x81/0x830
> [  234.843035]  ? simple_attr_write+0x3a/0xe0
> [  234.843038]  ? __lock_acquire+0x3bf/0x1dc0
> [  234.843041]  intel_gt_debugfs_reset_store+0x3c/0x90 [i915]
> [  234.843083]  ? _copy_from_user+0x45/0x80
> [  234.843086]  simple_attr_write+0xc1/0xe0
> [  234.843089]  full_proxy_write+0x53/0x80
> [  234.843091]  vfs_write+0xbc/0x350
> [  234.843094]  ksys_write+0x58/0xd0
> [  234.843096]  do_syscall_64+0x38/0x90
> [  234.843098]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [  234.843101] RIP: 0033:0x7fa467480877
> [  234.843103] Code: 75 05 48 83 c4 58 c3 e8 37 4e ff ff 0f 1f 80 00 00 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
> [  234.843108] RSP: 002b:00007ffd14d79b08 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> [  234.843112] RAX: ffffffffffffffda RBX: 00007ffd14d79b60 RCX: 00007fa467480877
> [  234.843114] RDX: 0000000000000014 RSI: 00007ffd14d79b60 RDI: 0000000000000007
> [  234.843116] RBP: 0000000000000007 R08: 0000000000000000 R09: 00007ffd14d79ab0
> [  234.843119] R10: ffffffffffffffff R11: 0000000000000246 R12: 0000000000000014
> [  234.843121] R13: 0000000000000000 R14: 00007ffd14d79b60 R15: 0000000000000005
> 
> v5:
> - Use __GFP_KSWAPD_RECLAIM rather than __GFP_NOWAIT for clarity.
>   (Daniel Vetter)
> v6:
> - Include an instance in execlists_capture_work().
> - Rework the commit message due to patch reordering.
> 
> Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
Looks good to me 

Reviewed-by: Ramalingam C <ramalingam.c@xxxxxxxxx>

> ---
>  drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 3 ++-
>  drivers/gpu/drm/i915/i915_gpu_error.c                | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> index ca03880fa7e4..a69df5e9e77a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
> @@ -2186,7 +2186,8 @@ struct execlists_capture {
>  static void execlists_capture_work(struct work_struct *work)
>  {
>  	struct execlists_capture *cap = container_of(work, typeof(*cap), work);
> -	const gfp_t gfp = GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
> +	const gfp_t gfp = __GFP_KSWAPD_RECLAIM | __GFP_RETRY_MAYFAIL |
> +		__GFP_NOWARN;
>  	struct intel_engine_cs *engine = cap->rq->engine;
>  	struct intel_gt_coredump *gt = cap->error->gt;
>  	struct intel_engine_capture_vma *vma;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 14de64282697..b1e4ce0f798f 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -49,7 +49,7 @@
>  #include "i915_memcpy.h"
>  #include "i915_scatterlist.h"
>  
> -#define ALLOW_FAIL (GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
> +#define ALLOW_FAIL (__GFP_KSWAPD_RECLAIM | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
>  #define ATOMIC_MAYFAIL (GFP_ATOMIC | __GFP_NOWARN)
>  
>  static void __sg_set_buf(struct scatterlist *sg,
> -- 
> 2.31.1
> 



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux