Re: WARNING: possible recursive locking detected in mnt_want_write/mnt_want_write_file_path

Re: WARNING: possible recursive locking detected in mnt_want_write/mnt_want_write_file_path

[Miklos Szeredi]

Cc: linux-unionfs@vger

On Wed, Oct 17, 2018 at 10:00 AM, Stefan Agner <stefan@agner.ch> wrote:
> Hi,
>
> I noticed this warning since we moved to 4.18. It appears when
> using Docker (which uses overlayfs). Is this a known issue?
>
> [  543.235366] WARNING: possible recursive locking detected
> [  543.240747] 4.18.14 #1 Not tainted
> [  543.244195] --------------------------------------------
> [  543.249573] dockerd/522 is trying to acquire lock:
> [  543.254426] 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write+0x20/0x4c
> [  543.261152] but task is already holding lock:
> [  543.267053] 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write_file_path+0x24/0x54
> [  543.274641] other info that might help us debug this:
> [  543.281242]  Possible unsafe locking scenario:
> [  543.287227]        CPU0
> [  543.289706]        ----
> [  543.292183]   lock(sb_writers#7);
> [  543.295547]   lock(sb_writers#7);
> [  543.298912]  *** DEADLOCK ***
> [  543.306825]  May be due to missing lock nesting notation
> [  543.315594] 2 locks held by dockerd/522:
> [  543.320487]  #0: 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write_file_path+0x24/0x54
> [  543.330353]  #1: fbe4681b (&ovl_i_mutex_key[depth]){+.+.}, at: chown_common+0xf8/0x1c0
> [  543.340298] stack backtrace:
> [  543.346553] CPU: 0 PID: 522 Comm: dockerd Not tainted 4.18.14 #1
> [  543.353556] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [  543.361088] Backtrace:
> [  543.364530] [<c010ecd0>] (dump_backtrace) from [<c010f058>] (show_stack+0x18/0x1c)
> [  543.374116]  r7:00000000 r6:600d0093 r5:00000000 r4:c118cc04
> [  543.380816] [<c010f040>] (show_stack) from [<c0bca044>] (dump_stack+0xb4/0xec)
> [  543.390112] [<c0bc9f90>] (dump_stack) from [<c0190890>] (__lock_acquire+0xd0c/0x1978)
> [  543.400071]  r10:e80f0628 r9:c18b2ad0 r8:e80f0000 r7:00000000 r6:e80f05e8 r5:c1623390
> [  543.410132]  r4:c1623390 r3:e5e422a5
> [  543.414845] [<c018fb84>] (__lock_acquire) from [<c0191cfc>] (lock_acquire+0x70/0x90)
> [  543.424956]  r10:00000004 r9:00000001 r8:00000001 r7:00000001 r6:600d0013 r5:00000000
> [  543.435317]  r4:ffffe000
> [  543.439144] [<c0191c8c>] (lock_acquire) from [<c02a0ce4>] (__sb_start_write+0x114/0x1b4)
> [  543.449944]  r8:c02c57d8 r7:ec1191f4 r6:00000000 r5:00000000 r4:ec1191f4
> [  543.458074] [<c02a0bd0>] (__sb_start_write) from [<c02c57d8>] (mnt_want_write+0x20/0x4c)
> [  543.468997]  r10:00000004 r9:dbb48ac8 r8:c1108908 r7:e80f9ee8 r6:e80f9ee8 r5:dbb48ac8
> [  543.479693]  r4:edea6610
> [  543.483672] [<c02c57b8>] (mnt_want_write) from [<bf09eb5c>] (ovl_want_write+0x1c/0x20 [overlay])
> [  543.495307]  r5:dbb48ac8 r4:00000000
> [  543.500396] [<bf09eb40>] (ovl_want_write [overlay]) from [<bf0a02e0>] (ovl_setattr+0x30/0x110 [overlay])
> [  543.512871] [<bf0a02b0>] (ovl_setattr [overlay]) from [<c02c0658>] (notify_change+0x25c/0x440)
> [  543.524467]  r9:dbb48ac8 r8:c1108908 r7:e80f9ee8 r6:dbb4cd88 r5:00000000 r4:00001846
> [  543.535205] [<c02c03fc>] (notify_change) from [<c029a050>] (chown_common+0x108/0x1c0)
> [  543.546047]  r10:e8488c88 r9:dbb4ce40 r8:00000000 r7:00000000 r6:00000000 r5:dbb4cd88
> [  543.556891]  r4:e80f8000
> [  543.560893] [<c0299f48>] (chown_common) from [<c029b664>] (ksys_fchown+0x44/0x78)
> [  543.571335]  r10:000000cf r9:e80f8000 r8:00000000 r7:00000000 r6:00000000 r5:e8488c80
> [  543.582164]  r4:e8488c81
> [  543.586163] [<c029b620>] (ksys_fchown) from [<c029b6a8>] (sys_fchown+0x10/0x14)
> [  543.596418]  r9:e80f8000 r8:c01011e4 r7:000000cf r6:1421d548 r5:00000000 r4:00000000
> [  543.607166] [<c029b698>] (sys_fchown) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
> [  543.617828] Exception stack(0xe80f9fa8 to 0xe80f9ff0)
> [  543.624387] 9fa0:                   00000000 00000000 00000012 00000000 00000000 00000000
> [  543.635546] 9fc0: 00000000 00000000 1421d548 000000cf 149cf708 00000000 1425e620 02970ec4
> [  543.646767] 9fe0: 149cf705 1503f618 007572d0 007f1ce4
>
> --
> Stefan

Re: WARNING: possible recursive locking detected in mnt_want_write/mnt_want_write_file_path

[Amir Goldstein]

On Wed, Oct 17, 2018 at 11:29 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> Cc: linux-unionfs@vger
>
> On Wed, Oct 17, 2018 at 10:00 AM, Stefan Agner <stefan@agner.ch> wrote:
> > Hi,
> >
> > I noticed this warning since we moved to 4.18. It appears when
> > using Docker (which uses overlayfs). Is this a known issue?
> >
> > [  543.235366] WARNING: possible recursive locking detected
> > [  543.240747] 4.18.14 #1 Not tainted
> > [  543.244195] --------------------------------------------
> > [  543.249573] dockerd/522 is trying to acquire lock:
> > [  543.254426] 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write+0x20/0x4c
> > [  543.261152] but task is already holding lock:
> > [  543.267053] 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write_file_path+0x24/0x54
> > [  543.274641] other info that might help us debug this:
> > [  543.281242]  Possible unsafe locking scenario:
> > [  543.287227]        CPU0
> > [  543.289706]        ----
> > [  543.292183]   lock(sb_writers#7);
> > [  543.295547]   lock(sb_writers#7);
> > [  543.298912]  *** DEADLOCK ***
> > [  543.306825]  May be due to missing lock nesting notation
> > [  543.315594] 2 locks held by dockerd/522:
> > [  543.320487]  #0: 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write_file_path+0x24/0x54
> > [  543.330353]  #1: fbe4681b (&ovl_i_mutex_key[depth]){+.+.}, at: chown_common+0xf8/0x1c0

This is a stable tree regression because upstream commit
a6795a585929 vfs: fix freeze protection in mnt_want_write_file() for overlayfs

SHOULD NOT have been applied to kernel <= 4.18

Miklos,

You must have confused "the algorithm" by including a "fix" commit in
rc1 pull request
without mentioning that it "Fixes" a commit in the same pull request.
It probably didn't help that the commit applied cleanly to the wrong
function :-/

Upstream commit a6795a585929:
@@ -441,10 +441,10 @@ int mnt_want_write_file(struct file *file)
 {
        int ret;

-       sb_start_write(file->f_path.mnt->mnt_sb);
+       sb_start_write(file_inode(file)->i_sb);

Backported v4.18.14 commit 5e1002ab5c9b:
@@ -446,10 +446,10 @@ int mnt_want_write_file_path(struct file *file)
 {
        int ret;

-       sb_start_write(file->f_path.mnt->mnt_sb);
+       sb_start_write(file_inode(file)->i_sb);

Ouch! Is there a way for git/quilt to raise a red flag in cases like this?

I could reproduce another lockdep warning on v4.18.14 with xfstest
overlay/030, which is fixed by reverting this commit.

Stefan,

Can you verify that reverting that commit solved the problem for you?

Thanks,
Amir.

Re: WARNING: possible recursive locking detected in mnt_want_write/mnt_want_write_file_path

[Sasha Levin]

On Wed, Oct 17, 2018 at 11:24 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Wed, Oct 17, 2018 at 11:29 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > Cc: linux-unionfs@vger
> >
> > On Wed, Oct 17, 2018 at 10:00 AM, Stefan Agner <stefan@agner.ch> wrote:
> > > Hi,
> > >
> > > I noticed this warning since we moved to 4.18. It appears when
> > > using Docker (which uses overlayfs). Is this a known issue?
> > >
> > > [  543.235366] WARNING: possible recursive locking detected
> > > [  543.240747] 4.18.14 #1 Not tainted
> > > [  543.244195] --------------------------------------------
> > > [  543.249573] dockerd/522 is trying to acquire lock:
> > > [  543.254426] 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write+0x20/0x4c
> > > [  543.261152] but task is already holding lock:
> > > [  543.267053] 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write_file_path+0x24/0x54
> > > [  543.274641] other info that might help us debug this:
> > > [  543.281242]  Possible unsafe locking scenario:
> > > [  543.287227]        CPU0
> > > [  543.289706]        ----
> > > [  543.292183]   lock(sb_writers#7);
> > > [  543.295547]   lock(sb_writers#7);
> > > [  543.298912]  *** DEADLOCK ***
> > > [  543.306825]  May be due to missing lock nesting notation
> > > [  543.315594] 2 locks held by dockerd/522:
> > > [  543.320487]  #0: 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write_file_path+0x24/0x54
> > > [  543.330353]  #1: fbe4681b (&ovl_i_mutex_key[depth]){+.+.}, at: chown_common+0xf8/0x1c0
>
> This is a stable tree regression because upstream commit
> a6795a585929 vfs: fix freeze protection in mnt_want_write_file() for overlayfs
>
> SHOULD NOT have been applied to kernel <= 4.18
>
> Miklos,
>
> You must have confused "the algorithm" by including a "fix" commit in
> rc1 pull request
> without mentioning that it "Fixes" a commit in the same pull request.

Additionally, code metrics look a lot like a fix (small patch, similar
amount of lines added/removed, no changes in function complexity,
etc).

> It probably didn't help that the commit applied cleanly to the wrong
> function :-/
>
> Upstream commit a6795a585929:
> @@ -441,10 +441,10 @@ int mnt_want_write_file(struct file *file)
>  {
>         int ret;
>
> -       sb_start_write(file->f_path.mnt->mnt_sb);
> +       sb_start_write(file_inode(file)->i_sb);
>
> Backported v4.18.14 commit 5e1002ab5c9b:
> @@ -446,10 +446,10 @@ int mnt_want_write_file_path(struct file *file)
>  {
>         int ret;
>
> -       sb_start_write(file->f_path.mnt->mnt_sb);
> +       sb_start_write(file_inode(file)->i_sb);
>
> Ouch! Is there a way for git/quilt to raise a red flag in cases like this?

This is incredible, I didn't know git does that. I thought it would
just generate a conflict.

--
Thanks,
Sasha

Re: WARNING: possible recursive locking detected in mnt_want_write/mnt_want_write_file_path

[Miklos Szeredi]

On Wed, Oct 17, 2018 at 5:24 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Oct 17, 2018 at 11:29 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>>
>> Cc: linux-unionfs@vger
>>
>> On Wed, Oct 17, 2018 at 10:00 AM, Stefan Agner <stefan@agner.ch> wrote:
>> > Hi,
>> >
>> > I noticed this warning since we moved to 4.18. It appears when
>> > using Docker (which uses overlayfs). Is this a known issue?
>> >
>> > [  543.235366] WARNING: possible recursive locking detected
>> > [  543.240747] 4.18.14 #1 Not tainted
>> > [  543.244195] --------------------------------------------
>> > [  543.249573] dockerd/522 is trying to acquire lock:
>> > [  543.254426] 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write+0x20/0x4c
>> > [  543.261152] but task is already holding lock:
>> > [  543.267053] 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write_file_path+0x24/0x54
>> > [  543.274641] other info that might help us debug this:
>> > [  543.281242]  Possible unsafe locking scenario:
>> > [  543.287227]        CPU0
>> > [  543.289706]        ----
>> > [  543.292183]   lock(sb_writers#7);
>> > [  543.295547]   lock(sb_writers#7);
>> > [  543.298912]  *** DEADLOCK ***
>> > [  543.306825]  May be due to missing lock nesting notation
>> > [  543.315594] 2 locks held by dockerd/522:
>> > [  543.320487]  #0: 86b0f89c (sb_writers#7){.+.+}, at: mnt_want_write_file_path+0x24/0x54
>> > [  543.330353]  #1: fbe4681b (&ovl_i_mutex_key[depth]){+.+.}, at: chown_common+0xf8/0x1c0
>
> This is a stable tree regression because upstream commit
> a6795a585929 vfs: fix freeze protection in mnt_want_write_file() for overlayfs
>
> SHOULD NOT have been applied to kernel <= 4.18
>
> Miklos,
>
> You must have confused "the algorithm" by including a "fix" commit in
> rc1 pull request
> without mentioning that it "Fixes" a commit in the same pull request.

I didn't know there were heuristics other than "Cc: stable@.."

> It probably didn't help that the commit applied cleanly to the wrong
> function :-/

Heh...

Thanks,
Miklos

Re: WARNING: possible recursive locking detected in mnt_want_write/mnt_want_write_file_path

[Amir Goldstein]

On Wed, Oct 17, 2018 at 9:37 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, Oct 17, 2018 at 5:24 PM, Amir Goldstein <amir73il@gmail.com> wrote:

> > Miklos,
> >
> > You must have confused "the algorithm" by including a "fix" commit in
> > rc1 pull request
> > without mentioning that it "Fixes" a commit in the same pull request.
>
> I didn't know there were heuristics other than "Cc: stable@.."
>

Yes. Given a Fixes: label, I bet Cc: stable is not needed to auto select
for stable??

Sasha,

Please note that lack of Fixes: or Cc: stable from some subsystems
is a rather strong indication for NOT for stable.

I suppose that in case of stable branch regression due to incorrect
patch selection there is a strong negative feedback to the algorithm?

Thanks,
Amir.

Re: WARNING: possible recursive locking detected in mnt_want_write/mnt_want_write_file_path

[Stefan Agner]

On 18.10.2018 13:50, Amir Goldstein wrote:
> On Wed, Oct 17, 2018 at 9:37 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>>
>> On Wed, Oct 17, 2018 at 5:24 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> 
>> > Miklos,
>> >
>> > You must have confused "the algorithm" by including a "fix" commit in
>> > rc1 pull request
>> > without mentioning that it "Fixes" a commit in the same pull request.
>>
>> I didn't know there were heuristics other than "Cc: stable@.."
>>
> 
> Yes. Given a Fixes: label, I bet Cc: stable is not needed to auto select
> for stable??

Given a Fixes: probably would prevented it to be selected since the
commit it references is not part of 4.18...

Anyway, as expected, reverting the commit on stable 4.18.4 fixes the
warning for me.

Thanks for tracking down!

--
Stefan

> 
> Sasha,
> 
> Please note that lack of Fixes: or Cc: stable from some subsystems
> is a rather strong indication for NOT for stable.
> 
> I suppose that in case of stable branch regression due to incorrect
> patch selection there is a strong negative feedback to the algorithm?
> 
> Thanks,
> Amir.

Re: WARNING: possible recursive locking detected in mnt_want_write/mnt_want_write_file_path

[Amir Goldstein]

On Thu, Oct 18, 2018 at 4:25 PM Stefan Agner <stefan@agner.ch> wrote:
>
> On 18.10.2018 13:50, Amir Goldstein wrote:
> > On Wed, Oct 17, 2018 at 9:37 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >>
> >> On Wed, Oct 17, 2018 at 5:24 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> >
> >> > Miklos,
> >> >
> >> > You must have confused "the algorithm" by including a "fix" commit in
> >> > rc1 pull request
> >> > without mentioning that it "Fixes" a commit in the same pull request.
> >>
> >> I didn't know there were heuristics other than "Cc: stable@.."
> >>
> >
> > Yes. Given a Fixes: label, I bet Cc: stable is not needed to auto select
> > for stable??
>
> Given a Fixes: probably would prevented it to be selected since the
> commit it references is not part of 4.18...
>

Indeed. adding a Fixes label for non-stable patches as well would be
good practice.

Thanks,
Amir.