xfs: circular locking dependency between fs_reclaim and sb_internal [kernel 4.18]

xfs: circular locking dependency between fs_reclaim and sb_internal [kernel 4.18]

[Tetsuo Handa]

I'm hitting below lockdep warning (essentially same with
http://lkml.kernel.org/r/1518666178.6070.25.camel@gmail.com ) as of commit
ba4dbdedd3edc279 ("Merge tag 'jfs-4.18' of git://github.com/kleikamp/linux-shaggy")
on linux.git . I think that this became visible by commit 93781325da6e07af
("lockdep: fix fs_reclaim annotation") which went to 4.18-rc1. What should we do?

[   63.207781] 
[   63.471926] ======================================================
[   64.432812] WARNING: possible circular locking dependency detected
[   64.948272] 4.18.0-rc1+ #594 Tainted: G                T
[   65.512546] ------------------------------------------------------
[   65.519722] kswapd0/79 is trying to acquire lock:
[   65.525792] 00000000f3581fab (sb_internal){.+.+}, at: xfs_trans_alloc+0xe0/0x120 [xfs]
[   66.034497] 
[   66.034497] but task is already holding lock:
[   66.661024] 00000000c7665973 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x0/0x30
[   67.554480] 
[   67.554480] which lock already depends on the new lock.
[   67.554480] 
[   68.760085] 
[   68.760085] the existing dependency chain (in reverse order) is:
[   69.258520] 
[   69.258520] -> #1 (fs_reclaim){+.+.}:
[   69.623516] 
[   69.623516] -> #0 (sb_internal){.+.+}:
[   70.152322] 
[   70.152322] other info that might help us debug this:
[   70.152322] 
[   70.164923]  Possible unsafe locking scenario:
[   70.164923] 
[   70.168174]        CPU0                    CPU1
[   70.170441]        ----                    ----
[   70.171827]   lock(fs_reclaim);
[   70.172771]                                lock(sb_internal);
[   70.174447]                                lock(fs_reclaim);
[   70.176104]   lock(sb_internal);
[   70.177062] 
[   70.177062]  *** DEADLOCK ***
[   70.177062] 
[   70.178789] 1 lock held by kswapd0/79:
[   70.179893]  #0: 00000000c7665973 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x0/0x30
[   70.182213] 
[   70.182213] stack backtrace:
[   70.184100] CPU: 2 PID: 79 Comm: kswapd0 Kdump: loaded Tainted: G                T 4.18.0-rc1+ #594
[   70.187320] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/19/2017
[   70.191064] Call Trace:
[   70.192487]  ? dump_stack+0x85/0xcb
[   70.194186]  ? print_circular_bug.isra.37+0x1d7/0x1e4
[   70.196343]  ? __lock_acquire+0x126e/0x1330
[   70.198232]  ? lock_acquire+0x51/0x70
[   70.199964]  ? lock_acquire+0x51/0x70
[   70.201735]  ? xfs_trans_alloc+0xe0/0x120 [xfs]
[   70.203727]  ? __sb_start_write+0x126/0x1a0       /* __preempt_count_add at ./arch/x86/include/asm/preempt.h:76 (inlined by) percpu_down_read_preempt_disable at ./include/linux/percpu-rwsem.h:38 (inlined by) percpu_down_read at ./include/linux/percpu-rwsem.h:59 (inlined by) __sb_start_write at fs/super.c:1403 */
[   70.205618]  ? xfs_trans_alloc+0xe0/0x120 [xfs]
[   70.207624]  ? xfs_trans_alloc+0xe0/0x120 [xfs]   /* sb_start_intwrite at ./include/linux/fs.h:1601 (inlined by) xfs_trans_alloc at fs/xfs/xfs_trans.c:259 */
[   70.209599]  ? xfs_iomap_write_allocate+0x1df/0x360 [xfs] /* xfs_iomap_write_allocate at fs/xfs/xfs_iomap.c:713 */
[   70.211801]  ? xfs_iunlock+0x6f/0x180 [xfs]
[   70.213657]  ? xfs_map_blocks+0x1c6/0x240 [xfs]   /* xfs_map_blocks at fs/xfs/xfs_aops.c:424 */
[   70.215602]  ? xfs_do_writepage+0x16d/0x7c0 [xfs] /* xfs_writepage_map at fs/xfs/xfs_aops.c:979 (inlined by) xfs_do_writepage at fs/xfs/xfs_aops.c:1162 */
[   70.217585]  ? find_held_lock+0x2d/0x90
[   70.219308]  ? clear_page_dirty_for_io+0x19d/0x470
[   70.221291]  ? xfs_vm_writepage+0x31/0x60 [xfs]   /* xfs_vm_writepage at fs/xfs/xfs_aops.c:1181 */
[   70.223201]  ? pageout.isra.53+0x122/0x4f0        /* pageout at mm/vmscan.c:683 */
[   70.224967]  ? shrink_page_list+0xcc4/0x18f0      /* shrink_page_list at mm/vmscan.c:1200 */
[   70.226771]  ? shrink_inactive_list+0x2e8/0x5b0   /* spin_lock_irq at ./include/linux/spinlock.h:335 (inlined by) shrink_inactive_list at mm/vmscan.c:1787 */
[   70.228624]  ? shrink_node_memcg+0x367/0x780      /* shrink_list at mm/vmscan.c:2095 (inlined by) shrink_node_memcg at mm/vmscan.c:2358 */
[   70.230433]  ? __lock_is_held+0x55/0x90
[   70.232167]  ? shrink_node+0xc9/0x470
[   70.233792]  ? shrink_node+0xc9/0x470             /* shrink_node at mm/vmscan.c:2576 */
[   70.235395]  ? balance_pgdat+0x154/0x3a0          /* kswapd_shrink_node at mm/vmscan.c:3304 (inlined by) balance_pgdat at mm/vmscan.c:3405 */
[   70.237070]  ? finish_task_switch+0x93/0x290
[   70.238861]  ? kswapd+0x151/0x370
[   70.240464]  ? wait_woken+0xa0/0xa0
[   70.242069]  ? balance_pgdat+0x3a0/0x3a0
[   70.243746]  ? kthread+0x112/0x130
[   70.245294]  ? kthread_create_worker_on_cpu+0x70/0x70
[   70.247281]  ? ret_from_fork+0x24/0x30

Re: xfs: circular locking dependency between fs_reclaim and sb_internal [kernel 4.18]

[Dave Chinner]

On Wed, Jun 20, 2018 at 08:25:51PM +0900, Tetsuo Handa wrote:
> I'm hitting below lockdep warning (essentially same with
> http://lkml.kernel.org/r/1518666178.6070.25.camel@gmail.com ) as of commit
> ba4dbdedd3edc279 ("Merge tag 'jfs-4.18' of git://github.com/kleikamp/linux-shaggy")
> on linux.git . I think that this became visible by commit 93781325da6e07af
> ("lockdep: fix fs_reclaim annotation") which went to 4.18-rc1. What should we do?

Fix the broken lockdep code?

Superblock freeze level accounting is not a lock. By the time a
freeze has got to the state where xfs_trans_alloc() would block,
we've already frozen new data writes and drained all the dirty
pages.  Hence if the filesystem is frozen down to the transaction
level, kswapd will never enter this path on the filesystem because
there will be no dirty pages to write back.

So this is a false positive.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: xfs: circular locking dependency between fs_reclaim and sb_internal [kernel 4.18]

[Dave Chinner]

On Wed, Jun 20, 2018 at 08:25:51PM +0900, Tetsuo Handa wrote:
> I'm hitting below lockdep warning (essentially same with
> http://lkml.kernel.org/r/1518666178.6070.25.camel@gmail.com ) as of commit
> ba4dbdedd3edc279 ("Merge tag 'jfs-4.18' of git://github.com/kleikamp/linux-shaggy")
> on linux.git . I think that this became visible by commit 93781325da6e07af
> ("lockdep: fix fs_reclaim annotation") which went to 4.18-rc1. What should we do?
> 
> [   63.207781] 
> [   63.471926] ======================================================
> [   64.432812] WARNING: possible circular locking dependency detected
> [   64.948272] 4.18.0-rc1+ #594 Tainted: G                T
> [   65.512546] ------------------------------------------------------
> [   65.519722] kswapd0/79 is trying to acquire lock:
> [   65.525792] 00000000f3581fab (sb_internal){.+.+}, at: xfs_trans_alloc+0xe0/0x120 [xfs]
> [   66.034497] 
> [   66.034497] but task is already holding lock:
> [   66.661024] 00000000c7665973 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x0/0x30
> [   67.554480] 
> [   67.554480] which lock already depends on the new lock.
> [   67.554480] 
> [   68.760085] 
> [   68.760085] the existing dependency chain (in reverse order) is:
> [   69.258520] 
> [   69.258520] -> #1 (fs_reclaim){+.+.}:
> [   69.623516] 
> [   69.623516] -> #0 (sb_internal){.+.+}:

BTW, where's the stack trace that was recorded when this ordering
was seen? Normally lockdep gives us all the relevant stack traces in
a report, and without this trace to tell us where it saw this order,
this bug report is mostly useless because we don't know what
inversion it has recorded and is complaining about.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[PATCH] Makefile: Fix backtrace breakage

[Tetsuo Handa]

>From 7208bf13827fa7c7d6196ee20f7678eff0d29b36 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Thu, 21 Jun 2018 14:15:10 +0900
Subject: [PATCH] Makefile: Fix backtrace breakage

Dave Chinner noticed that backtrace part is missing in a lockdep report.

  [   68.760085] the existing dependency chain (in reverse order) is:
  [   69.258520]
  [   69.258520] -> #1 (fs_reclaim){+.+.}:
  [   69.623516]
  [   69.623516] -> #0 (sb_internal){.+.+}:
  [   70.152322]
  [   70.152322] other info that might help us debug this:

Since the kernel was using CONFIG_FTRACE_MCOUNT_RECORD=n &&
CONFIG_FRAME_POINTER=n, objtool_args was not properly calculated
due to incorrectly placed "endif" in commit 96f60dfa5819a065 ("trace:
Use -mcount-record for dynamic ftrace").

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 scripts/Makefile.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 34d9e9c..55f22f4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -239,6 +239,7 @@ cmd_record_mcount =						\
 	     "$(CC_FLAGS_FTRACE)" ]; then			\
 		$(sub_cmd_record_mcount)			\
 	fi;
+endif
 endif # CONFIG_FTRACE_MCOUNT_RECORD
 
 ifdef CONFIG_STACK_VALIDATION
@@ -263,7 +264,6 @@ ifneq ($(RETPOLINE_CFLAGS),)
   objtool_args += --retpoline
 endif
 endif
-endif
 
 
 ifdef CONFIG_MODVERSIONS
-- 
1.8.3.1

[PATCH] Makefile: Fix backtrace breakage

[Tetsuo Handa]

Re: [PATCH] Makefile: Fix backtrace breakage

[Greg Thelen]

On Wed, Jun 20, 2018 at 10:47 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> From 7208bf13827fa7c7d6196ee20f7678eff0d29b36 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Thu, 21 Jun 2018 14:15:10 +0900
> Subject: [PATCH] Makefile: Fix backtrace breakage
>
> Dave Chinner noticed that backtrace part is missing in a lockdep report.
>
>   [   68.760085] the existing dependency chain (in reverse order) is:
>   [   69.258520]
>   [   69.258520] -> #1 (fs_reclaim){+.+.}:
>   [   69.623516]
>   [   69.623516] -> #0 (sb_internal){.+.+}:
>   [   70.152322]
>   [   70.152322] other info that might help us debug this:
>
> Since the kernel was using CONFIG_FTRACE_MCOUNT_RECORD=n &&
> CONFIG_FRAME_POINTER=n, objtool_args was not properly calculated
> due to incorrectly placed "endif" in commit 96f60dfa5819a065 ("trace:
> Use -mcount-record for dynamic ftrace").
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Dave Chinner <david@fromorbit.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>

This looks similar to https://lkml.org/lkml/2018/6/8/545

> ---
>  scripts/Makefile.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 34d9e9c..55f22f4 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -239,6 +239,7 @@ cmd_record_mcount =                                         \
>              "$(CC_FLAGS_FTRACE)" ]; then                       \
>                 $(sub_cmd_record_mcount)                        \
>         fi;
> +endif
>  endif # CONFIG_FTRACE_MCOUNT_RECORD
>
>  ifdef CONFIG_STACK_VALIDATION
> @@ -263,7 +264,6 @@ ifneq ($(RETPOLINE_CFLAGS),)
>    objtool_args += --retpoline
>  endif
>  endif
> -endif
>
>
>  ifdef CONFIG_MODVERSIONS
> --
> 1.8.3.1
>

Re: [PATCH] Makefile: Fix backtrace breakage

[Andi Kleen]

On Thu, Jun 21, 2018 at 02:47:05PM +0900, Tetsuo Handa wrote:
> From 7208bf13827fa7c7d6196ee20f7678eff0d29b36 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Thu, 21 Jun 2018 14:15:10 +0900
> Subject: [PATCH] Makefile: Fix backtrace breakage
> 
> Dave Chinner noticed that backtrace part is missing in a lockdep report.
> 
>   [   68.760085] the existing dependency chain (in reverse order) is:
>   [   69.258520]
>   [   69.258520] -> #1 (fs_reclaim){+.+.}:
>   [   69.623516]
>   [   69.623516] -> #0 (sb_internal){.+.+}:
>   [   70.152322]
>   [   70.152322] other info that might help us debug this:

Thanks. Was already fixed earlier I believe.

-Andi

Re: [PATCH] Makefile: Fix backtrace breakage

[Steven Rostedt]

On Thu, 21 Jun 2018 13:48:34 -0700
Andi Kleen <ak@linux.intel.com> wrote:

> On Thu, Jun 21, 2018 at 02:47:05PM +0900, Tetsuo Handa wrote:
> > From 7208bf13827fa7c7d6196ee20f7678eff0d29b36 Mon Sep 17 00:00:00 2001
> > From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> > Date: Thu, 21 Jun 2018 14:15:10 +0900
> > Subject: [PATCH] Makefile: Fix backtrace breakage
> > 
> > Dave Chinner noticed that backtrace part is missing in a lockdep report.
> > 
> >   [   68.760085] the existing dependency chain (in reverse order) is:
> >   [   69.258520]
> >   [   69.258520] -> #1 (fs_reclaim){+.+.}:
> >   [   69.623516]
> >   [   69.623516] -> #0 (sb_internal){.+.+}:
> >   [   70.152322]
> >   [   70.152322] other info that might help us debug this:  
> 
> Thanks. Was already fixed earlier I believe.
> 
> 

I actually just pulled the patch in an hour ago, and I'm currently
testing it along with other patches.

Thanks!

-- Steve

Re: [PATCH] Makefile: Fix backtrace breakage

[Steven Rostedt]

On Thu, 21 Jun 2018 17:30:27 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> I actually just pulled the patch in an hour ago, and I'm currently
> testing it along with other patches.

To clear up any ambiguity, this is the patch I pulled in:

 http://lkml.kernel.org/r/20180608214746.136554-1-gthelen@google.com

-- Steve