linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [PATCH] f2fs: fix potential deadlock
@ 2017-07-31 12:32 Chao Yu
  2017-07-31 23:49 ` Jaegeuk Kim
  0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2017-07-31 12:32 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu

generic/241 reports below bug:

 ======================================================
 WARNING: possible circular locking dependency detected
 4.13.0-rc1+ #32 Tainted: G           O
 ------------------------------------------------------
 f2fs_gc-250:0/22186 is trying to acquire lock:
  (&sbi->gc_mutex){+.+...}, at: [<f8fa7f0b>] f2fs_sync_fs+0x7b/0x1b0 [f2fs]

 but task is already holding lock:
  (sb_internal#2){++++.-}, at: [<f8fb5609>] gc_thread_func+0x159/0x4a0 [f2fs]

 which lock already depends on the new lock.

 the existing dependency chain (in reverse order) is:

 -> #2 (sb_internal#2){++++.-}:
        __lock_acquire+0x405/0x7b0
        lock_acquire+0xae/0x220
        __sb_start_write+0x11d/0x1f0
        f2fs_evict_inode+0x2d6/0x4e0 [f2fs]
        evict+0xa8/0x170
        iput+0x1fb/0x2c0
        f2fs_sync_inode_meta+0x3f/0xf0 [f2fs]
        write_checkpoint+0x1b1/0x750 [f2fs]
        f2fs_sync_fs+0x85/0x1b0 [f2fs]
        f2fs_do_sync_file.isra.24+0x137/0xa30 [f2fs]
        f2fs_sync_file+0x34/0x40 [f2fs]
        vfs_fsync_range+0x4a/0xa0
        do_fsync+0x3c/0x60
        SyS_fdatasync+0x15/0x20
        do_fast_syscall_32+0xa1/0x1b0
        entry_SYSENTER_32+0x4c/0x7b

 -> #1 (&sbi->cp_mutex){+.+...}:
        __lock_acquire+0x405/0x7b0
        lock_acquire+0xae/0x220
        __mutex_lock+0x4f/0x830
        mutex_lock_nested+0x25/0x30
        write_checkpoint+0x2f/0x750 [f2fs]
        f2fs_sync_fs+0x85/0x1b0 [f2fs]
        sync_filesystem+0x67/0x80
        generic_shutdown_super+0x27/0x100
        kill_block_super+0x22/0x50
        kill_f2fs_super+0x3a/0x40 [f2fs]
        deactivate_locked_super+0x3d/0x70
        deactivate_super+0x40/0x60
        cleanup_mnt+0x39/0x70
        __cleanup_mnt+0x10/0x20
        task_work_run+0x69/0x80
        exit_to_usermode_loop+0x57/0x92
        do_fast_syscall_32+0x18c/0x1b0
        entry_SYSENTER_32+0x4c/0x7b

 -> #0 (&sbi->gc_mutex){+.+...}:
        validate_chain.isra.36+0xc50/0xdb0
        __lock_acquire+0x405/0x7b0
        lock_acquire+0xae/0x220
        __mutex_lock+0x4f/0x830
        mutex_lock_nested+0x25/0x30
        f2fs_sync_fs+0x7b/0x1b0 [f2fs]
        f2fs_balance_fs_bg+0xb9/0x200 [f2fs]
        gc_thread_func+0x302/0x4a0 [f2fs]
        kthread+0xe9/0x120
        ret_from_fork+0x19/0x24

 other info that might help us debug this:

 Chain exists of:
   &sbi->gc_mutex --> &sbi->cp_mutex --> sb_internal#2

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(sb_internal#2);
                                lock(&sbi->cp_mutex);
                                lock(sb_internal#2);
   lock(&sbi->gc_mutex);

  *** DEADLOCK ***

 1 lock held by f2fs_gc-250:0/22186:
  #0:  (sb_internal#2){++++.-}, at: [<f8fb5609>] gc_thread_func+0x159/0x4a0 [f2fs]

 stack backtrace:
 CPU: 2 PID: 22186 Comm: f2fs_gc-250:0 Tainted: G           O    4.13.0-rc1+ #32
 Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
 Call Trace:
  dump_stack+0x5f/0x92
  print_circular_bug+0x1b3/0x1bd
  validate_chain.isra.36+0xc50/0xdb0
  ? __this_cpu_preempt_check+0xf/0x20
  __lock_acquire+0x405/0x7b0
  lock_acquire+0xae/0x220
  ? f2fs_sync_fs+0x7b/0x1b0 [f2fs]
  __mutex_lock+0x4f/0x830
  ? f2fs_sync_fs+0x7b/0x1b0 [f2fs]
  mutex_lock_nested+0x25/0x30
  ? f2fs_sync_fs+0x7b/0x1b0 [f2fs]
  f2fs_sync_fs+0x7b/0x1b0 [f2fs]
  f2fs_balance_fs_bg+0xb9/0x200 [f2fs]
  gc_thread_func+0x302/0x4a0 [f2fs]
  ? preempt_schedule_common+0x2f/0x4d
  ? f2fs_gc+0x540/0x540 [f2fs]
  kthread+0xe9/0x120
  ? f2fs_gc+0x540/0x540 [f2fs]
  ? kthread_create_on_node+0x30/0x30
  ret_from_fork+0x19/0x24

The deadlock occurs in below condition:
GC Thread			Thread B
- sb_start_intwrite
				- f2fs_sync_file
				 - f2fs_sync_fs
				  - mutex_lock(&sbi->gc_mutex)
				   - write_checkpoint
				    - block_operations
				     - f2fs_sync_inode_meta
				      - iput
				       - sb_start_intwrite
 - mutex_lock(&sbi->gc_mutex)

Fix this by altering sb_start_intwrite to sb_start_write_trylock.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/gc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 1c0117f60083..f57cadae1a30 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -55,7 +55,8 @@ static int gc_thread_func(void *data)
 		}
 #endif
 
-		sb_start_intwrite(sbi->sb);
+		if (!sb_start_write_trylock(sbi->sb))
+			continue;
 
 		/*
 		 * [GC triggering condition]
@@ -96,7 +97,7 @@ static int gc_thread_func(void *data)
 		/* balancing f2fs's metadata periodically */
 		f2fs_balance_fs_bg(sbi);
 next:
-		sb_end_intwrite(sbi->sb);
+		sb_end_write(sbi->sb);
 
 	} while (!kthread_should_stop());
 	return 0;
-- 
2.13.1.388.g69e6b9b4f4a9

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] f2fs: fix potential deadlock
  2017-07-31 12:32 [PATCH] f2fs: fix potential deadlock Chao Yu
@ 2017-07-31 23:49 ` Jaegeuk Kim
  2017-08-01 15:39   ` [f2fs-dev] " Chao Yu
  0 siblings, 1 reply; 3+ messages in thread
From: Jaegeuk Kim @ 2017-07-31 23:49 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao

Hi Chao,

Let me integrate this patch into original freeze patch:
  (f2fs: make background threads of f2fs being aware of freezing)

Thanks,

On 07/31, Chao Yu wrote:
> generic/241 reports below bug:
> 
>  ======================================================
>  WARNING: possible circular locking dependency detected
>  4.13.0-rc1+ #32 Tainted: G           O
>  ------------------------------------------------------
>  f2fs_gc-250:0/22186 is trying to acquire lock:
>   (&sbi->gc_mutex){+.+...}, at: [<f8fa7f0b>] f2fs_sync_fs+0x7b/0x1b0 [f2fs]
> 
>  but task is already holding lock:
>   (sb_internal#2){++++.-}, at: [<f8fb5609>] gc_thread_func+0x159/0x4a0 [f2fs]
> 
>  which lock already depends on the new lock.
> 
>  the existing dependency chain (in reverse order) is:
> 
>  -> #2 (sb_internal#2){++++.-}:
>         __lock_acquire+0x405/0x7b0
>         lock_acquire+0xae/0x220
>         __sb_start_write+0x11d/0x1f0
>         f2fs_evict_inode+0x2d6/0x4e0 [f2fs]
>         evict+0xa8/0x170
>         iput+0x1fb/0x2c0
>         f2fs_sync_inode_meta+0x3f/0xf0 [f2fs]
>         write_checkpoint+0x1b1/0x750 [f2fs]
>         f2fs_sync_fs+0x85/0x1b0 [f2fs]
>         f2fs_do_sync_file.isra.24+0x137/0xa30 [f2fs]
>         f2fs_sync_file+0x34/0x40 [f2fs]
>         vfs_fsync_range+0x4a/0xa0
>         do_fsync+0x3c/0x60
>         SyS_fdatasync+0x15/0x20
>         do_fast_syscall_32+0xa1/0x1b0
>         entry_SYSENTER_32+0x4c/0x7b
> 
>  -> #1 (&sbi->cp_mutex){+.+...}:
>         __lock_acquire+0x405/0x7b0
>         lock_acquire+0xae/0x220
>         __mutex_lock+0x4f/0x830
>         mutex_lock_nested+0x25/0x30
>         write_checkpoint+0x2f/0x750 [f2fs]
>         f2fs_sync_fs+0x85/0x1b0 [f2fs]
>         sync_filesystem+0x67/0x80
>         generic_shutdown_super+0x27/0x100
>         kill_block_super+0x22/0x50
>         kill_f2fs_super+0x3a/0x40 [f2fs]
>         deactivate_locked_super+0x3d/0x70
>         deactivate_super+0x40/0x60
>         cleanup_mnt+0x39/0x70
>         __cleanup_mnt+0x10/0x20
>         task_work_run+0x69/0x80
>         exit_to_usermode_loop+0x57/0x92
>         do_fast_syscall_32+0x18c/0x1b0
>         entry_SYSENTER_32+0x4c/0x7b
> 
>  -> #0 (&sbi->gc_mutex){+.+...}:
>         validate_chain.isra.36+0xc50/0xdb0
>         __lock_acquire+0x405/0x7b0
>         lock_acquire+0xae/0x220
>         __mutex_lock+0x4f/0x830
>         mutex_lock_nested+0x25/0x30
>         f2fs_sync_fs+0x7b/0x1b0 [f2fs]
>         f2fs_balance_fs_bg+0xb9/0x200 [f2fs]
>         gc_thread_func+0x302/0x4a0 [f2fs]
>         kthread+0xe9/0x120
>         ret_from_fork+0x19/0x24
> 
>  other info that might help us debug this:
> 
>  Chain exists of:
>    &sbi->gc_mutex --> &sbi->cp_mutex --> sb_internal#2
> 
>   Possible unsafe locking scenario:
> 
>         CPU0                    CPU1
>         ----                    ----
>    lock(sb_internal#2);
>                                 lock(&sbi->cp_mutex);
>                                 lock(sb_internal#2);
>    lock(&sbi->gc_mutex);
> 
>   *** DEADLOCK ***
> 
>  1 lock held by f2fs_gc-250:0/22186:
>   #0:  (sb_internal#2){++++.-}, at: [<f8fb5609>] gc_thread_func+0x159/0x4a0 [f2fs]
> 
>  stack backtrace:
>  CPU: 2 PID: 22186 Comm: f2fs_gc-250:0 Tainted: G           O    4.13.0-rc1+ #32
>  Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>  Call Trace:
>   dump_stack+0x5f/0x92
>   print_circular_bug+0x1b3/0x1bd
>   validate_chain.isra.36+0xc50/0xdb0
>   ? __this_cpu_preempt_check+0xf/0x20
>   __lock_acquire+0x405/0x7b0
>   lock_acquire+0xae/0x220
>   ? f2fs_sync_fs+0x7b/0x1b0 [f2fs]
>   __mutex_lock+0x4f/0x830
>   ? f2fs_sync_fs+0x7b/0x1b0 [f2fs]
>   mutex_lock_nested+0x25/0x30
>   ? f2fs_sync_fs+0x7b/0x1b0 [f2fs]
>   f2fs_sync_fs+0x7b/0x1b0 [f2fs]
>   f2fs_balance_fs_bg+0xb9/0x200 [f2fs]
>   gc_thread_func+0x302/0x4a0 [f2fs]
>   ? preempt_schedule_common+0x2f/0x4d
>   ? f2fs_gc+0x540/0x540 [f2fs]
>   kthread+0xe9/0x120
>   ? f2fs_gc+0x540/0x540 [f2fs]
>   ? kthread_create_on_node+0x30/0x30
>   ret_from_fork+0x19/0x24
> 
> The deadlock occurs in below condition:
> GC Thread			Thread B
> - sb_start_intwrite
> 				- f2fs_sync_file
> 				 - f2fs_sync_fs
> 				  - mutex_lock(&sbi->gc_mutex)
> 				   - write_checkpoint
> 				    - block_operations
> 				     - f2fs_sync_inode_meta
> 				      - iput
> 				       - sb_start_intwrite
>  - mutex_lock(&sbi->gc_mutex)
> 
> Fix this by altering sb_start_intwrite to sb_start_write_trylock.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/gc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 1c0117f60083..f57cadae1a30 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -55,7 +55,8 @@ static int gc_thread_func(void *data)
>  		}
>  #endif
>  
> -		sb_start_intwrite(sbi->sb);
> +		if (!sb_start_write_trylock(sbi->sb))
> +			continue;
>  
>  		/*
>  		 * [GC triggering condition]
> @@ -96,7 +97,7 @@ static int gc_thread_func(void *data)
>  		/* balancing f2fs's metadata periodically */
>  		f2fs_balance_fs_bg(sbi);
>  next:
> -		sb_end_intwrite(sbi->sb);
> +		sb_end_write(sbi->sb);
>  
>  	} while (!kthread_should_stop());
>  	return 0;
> -- 
> 2.13.1.388.g69e6b9b4f4a9

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [f2fs-dev] [PATCH] f2fs: fix potential deadlock
  2017-07-31 23:49 ` Jaegeuk Kim
@ 2017-08-01 15:39   ` Chao Yu
  0 siblings, 0 replies; 3+ messages in thread
From: Chao Yu @ 2017-08-01 15:39 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel

Hi Jaegeuk,

On 2017/8/1 7:49, Jaegeuk Kim wrote:
> Hi Chao,
> 
> Let me integrate this patch into original freeze patch:
>   (f2fs: make background threads of f2fs being aware of freezing)

It's OK to me. :)

Thanks,

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-08-01 15:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-31 12:32 [PATCH] f2fs: fix potential deadlock Chao Yu
2017-07-31 23:49 ` Jaegeuk Kim
2017-08-01 15:39   ` [f2fs-dev] " Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).