[BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Krzysztof Kozlowski]

Hi,

On recent next I see bugs during boot (after bringing up user-space or
during reboot):
kernel BUG at ../mm/vmalloc.c:470!
On all my boards. On QEMU I see something similar, although the
message is "Internal error: Oops - undefined instruction: 0 [#1] ARM",

The calltrace is:
[   34.565126] [<c0275c9c>] (__free_vmap_area) from [<c0276044>]
(__purge_vmap_area_lazy+0xd0/0x170)
[   34.573963] [<c0276044>] (__purge_vmap_area_lazy) from [<c0276d50>]
(_vm_unmap_aliases+0x1fc/0x244)
[   34.582974] [<c0276d50>] (_vm_unmap_aliases) from [<c0279500>]
(__vunmap+0x170/0x200)
[   34.590770] [<c0279500>] (__vunmap) from [<c01d5a70>]
(do_free_init+0x40/0x5c)
[   34.597955] [<c01d5a70>] (do_free_init) from [<c01478f4>]
(process_one_work+0x228/0x810)
[   34.606018] [<c01478f4>] (process_one_work) from [<c0147f0c>]
(worker_thread+0x30/0x570)
[   34.614077] [<c0147f0c>] (worker_thread) from [<c014e8b4>]
(kthread+0x134/0x164)
[   34.621438] [<c014e8b4>] (kthread) from [<c01010b4>]
(ret_from_fork+0x14/0x20)

Full log here:
https://krzk.eu/#/builders/1/builds/3356/steps/14/logs/serial0
https://krzk.eu/#/builders/22/builds/1118/steps/35/logs/serial0

Bisect pointed to:
728e0fbf263e3ed359c10cb13623390564102881 is the first bad commit
commit 728e0fbf263e3ed359c10cb13623390564102881
Author: Uladzislau Rezki (Sony) <urezki@gmail.com>
Date:   Sat Jun 1 12:20:19 2019 +1000
    mm/vmalloc.c: get rid of one single unlink_va() when merge

Boards:
1. Arch ARM Linux
2. exynos_defconfig
3. Exynos boards (Odroid XU3, etc), ARMv7, octa-core (Cortex-A7+A15),
Exynos5422 SoC
4. Systemd: v239, static IP set in kernel command line

Best regards,
Krzysztof

Re: [BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Uladzislau Rezki]

Hello, Krzysztof.

On Mon, Jun 03, 2019 at 11:07:46AM +0200, Krzysztof Kozlowski wrote:
> Hi,
> 
> On recent next I see bugs during boot (after bringing up user-space or
> during reboot):
> kernel BUG at ../mm/vmalloc.c:470!
> On all my boards. On QEMU I see something similar, although the
> message is "Internal error: Oops - undefined instruction: 0 [#1] ARM",
> 
> The calltrace is:
> [   34.565126] [<c0275c9c>] (__free_vmap_area) from [<c0276044>]
> (__purge_vmap_area_lazy+0xd0/0x170)
> [   34.573963] [<c0276044>] (__purge_vmap_area_lazy) from [<c0276d50>]
> (_vm_unmap_aliases+0x1fc/0x244)
> [   34.582974] [<c0276d50>] (_vm_unmap_aliases) from [<c0279500>]
> (__vunmap+0x170/0x200)
> [   34.590770] [<c0279500>] (__vunmap) from [<c01d5a70>]
> (do_free_init+0x40/0x5c)
> [   34.597955] [<c01d5a70>] (do_free_init) from [<c01478f4>]
> (process_one_work+0x228/0x810)
> [   34.606018] [<c01478f4>] (process_one_work) from [<c0147f0c>]
> (worker_thread+0x30/0x570)
> [   34.614077] [<c0147f0c>] (worker_thread) from [<c014e8b4>]
> (kthread+0x134/0x164)
> [   34.621438] [<c014e8b4>] (kthread) from [<c01010b4>]
> (ret_from_fork+0x14/0x20)
> 
> Full log here:
> https://krzk.eu/#/builders/1/builds/3356/steps/14/logs/serial0
> https://krzk.eu/#/builders/22/builds/1118/steps/35/logs/serial0
> 
> Bisect pointed to:
> 728e0fbf263e3ed359c10cb13623390564102881 is the first bad commit
> commit 728e0fbf263e3ed359c10cb13623390564102881
> Author: Uladzislau Rezki (Sony) <urezki@gmail.com>
> Date:   Sat Jun 1 12:20:19 2019 +1000
>     mm/vmalloc.c: get rid of one single unlink_va() when merge
> 
I have checked the linux-next. I can confirm it happens because of:
 mm/vmalloc.c: get rid of one single unlink_va() when merge

The problem is that, it has been applied wrongly into linux-next tree
for some reason, i do not why. Probably due to the fact that i based
my work on 5.1/2-rcX, whereas linux-next is a bit ahead of it. If so,
sorry for that.

See below the clean patch for remotes/linux-next/master:

<snip>
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 650c89f38c1e..0ed95b864e31 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -719,9 +719,6 @@ merge_or_add_vmap_area(struct vmap_area *va,
                        /* Check and update the tree if needed. */
                        augment_tree_propagate_from(sibling);

-                       /* Remove this VA, it has been merged. */
-                       unlink_va(va, root);
-
                        /* Free vmap_area object. */
                        kmem_cache_free(vmap_area_cachep, va);

@@ -746,12 +743,11 @@ merge_or_add_vmap_area(struct vmap_area *va,
                        /* Check and update the tree if needed. */
                        augment_tree_propagate_from(sibling);

-                       /* Remove this VA, it has been merged. */
-                       unlink_va(va, root);
+                       if (merged)
+                               unlink_va(va, root);

                        /* Free vmap_area object. */
                        kmem_cache_free(vmap_area_cachep, va);
-
                        return;
                }
        }
-- 
2.11.0
<snip>

Andrew, i am not sure how to proceed with that. Should i send an updated series
based on linux-next tip or you can fix directly that patch?

Thank you!

--
Vlad Rezki

Re: [BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Krzysztof Kozlowski]

On Mon, 3 Jun 2019 at 15:59, Uladzislau Rezki <urezki@gmail.com> wrote:
>
> Hello, Krzysztof.
>
> On Mon, Jun 03, 2019 at 11:07:46AM +0200, Krzysztof Kozlowski wrote:
> > Hi,
> >
> > On recent next I see bugs during boot (after bringing up user-space or
> > during reboot):
> > kernel BUG at ../mm/vmalloc.c:470!
> > On all my boards. On QEMU I see something similar, although the
> > message is "Internal error: Oops - undefined instruction: 0 [#1] ARM",

Indeed it looks like effect of merge conflict resolution or applying.
When I look at MMOTS, it is the same as yours:
http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/commit/?id=b77b8cce67f246109f9d87417a32cd38f0398f2f

However in linux-next it is different.

Stephen, any thoughts?

Best regards,
Krzysztof

> >
> > The calltrace is:
> > [   34.565126] [<c0275c9c>] (__free_vmap_area) from [<c0276044>]
> > (__purge_vmap_area_lazy+0xd0/0x170)
> > [   34.573963] [<c0276044>] (__purge_vmap_area_lazy) from [<c0276d50>]
> > (_vm_unmap_aliases+0x1fc/0x244)
> > [   34.582974] [<c0276d50>] (_vm_unmap_aliases) from [<c0279500>]
> > (__vunmap+0x170/0x200)
> > [   34.590770] [<c0279500>] (__vunmap) from [<c01d5a70>]
> > (do_free_init+0x40/0x5c)
> > [   34.597955] [<c01d5a70>] (do_free_init) from [<c01478f4>]
> > (process_one_work+0x228/0x810)
> > [   34.606018] [<c01478f4>] (process_one_work) from [<c0147f0c>]
> > (worker_thread+0x30/0x570)
> > [   34.614077] [<c0147f0c>] (worker_thread) from [<c014e8b4>]
> > (kthread+0x134/0x164)
> > [   34.621438] [<c014e8b4>] (kthread) from [<c01010b4>]
> > (ret_from_fork+0x14/0x20)
> >
> > Full log here:
> > https://krzk.eu/#/builders/1/builds/3356/steps/14/logs/serial0
> > https://krzk.eu/#/builders/22/builds/1118/steps/35/logs/serial0
> >
> > Bisect pointed to:
> > 728e0fbf263e3ed359c10cb13623390564102881 is the first bad commit
> > commit 728e0fbf263e3ed359c10cb13623390564102881
> > Author: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > Date:   Sat Jun 1 12:20:19 2019 +1000
> >     mm/vmalloc.c: get rid of one single unlink_va() when merge
> >
> I have checked the linux-next. I can confirm it happens because of:
>  mm/vmalloc.c: get rid of one single unlink_va() when merge
>
> The problem is that, it has been applied wrongly into linux-next tree
> for some reason, i do not why. Probably due to the fact that i based
> my work on 5.1/2-rcX, whereas linux-next is a bit ahead of it. If so,
> sorry for that.
>
> See below the clean patch for remotes/linux-next/master:
>
> <snip>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 650c89f38c1e..0ed95b864e31 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -719,9 +719,6 @@ merge_or_add_vmap_area(struct vmap_area *va,
>                         /* Check and update the tree if needed. */
>                         augment_tree_propagate_from(sibling);
>
> -                       /* Remove this VA, it has been merged. */
> -                       unlink_va(va, root);
> -
>                         /* Free vmap_area object. */
>                         kmem_cache_free(vmap_area_cachep, va);
>
> @@ -746,12 +743,11 @@ merge_or_add_vmap_area(struct vmap_area *va,
>                         /* Check and update the tree if needed. */
>                         augment_tree_propagate_from(sibling);
>
> -                       /* Remove this VA, it has been merged. */
> -                       unlink_va(va, root);
> +                       if (merged)
> +                               unlink_va(va, root);
>
>                         /* Free vmap_area object. */
>                         kmem_cache_free(vmap_area_cachep, va);
> -
>                         return;
>                 }
>         }
> --
> 2.11.0
> <snip>
>
> Andrew, i am not sure how to proceed with that. Should i send an updated series
> based on linux-next tip or you can fix directly that patch?
>
> Thank you!
>
> --
> Vlad Rezki

Re: [BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

Hi Krzysztof,

On Mon, 3 Jun 2019 16:10:40 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> Indeed it looks like effect of merge conflict resolution or applying.
> When I look at MMOTS, it is the same as yours:
> http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/commit/?id=b77b8cce67f246109f9d87417a32cd38f0398f2f
> 
> However in linux-next it is different.
> 
> Stephen, any thoughts?

Have you had a look at today's linux-next?  It looks correct in
there.  Andrew updated his patch series over the weekend.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Krzysztof Kozlowski]

On Mon, 3 Jun 2019 at 16:32, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Krzysztof,
>
> On Mon, 3 Jun 2019 16:10:40 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > Indeed it looks like effect of merge conflict resolution or applying.
> > When I look at MMOTS, it is the same as yours:
> > http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/commit/?id=b77b8cce67f246109f9d87417a32cd38f0398f2f
> >
> > However in linux-next it is different.
> >
> > Stephen, any thoughts?
>
> Have you had a look at today's linux-next?  It looks correct in
> there.  Andrew updated his patch series over the weekend.

Yes, I am looking at today's next. Both the source code and the commit
728e0fbf263e3ed359c10cb13623390564102881 have wrong "if (merged)" (put
in wrong hunk).

Best regards,
Krzysztof

Re: [BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]

Hi Krzysztof,

On Mon, 3 Jun 2019 16:35:22 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, 3 Jun 2019 at 16:32, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > On Mon, 3 Jun 2019 16:10:40 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:  
> > >
> > > Indeed it looks like effect of merge conflict resolution or applying.
> > > When I look at MMOTS, it is the same as yours:
> > > http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/commit/?id=b77b8cce67f246109f9d87417a32cd38f0398f2f
> > >
> > > However in linux-next it is different.
> > >
> > > Stephen, any thoughts?  
> >
> > Have you had a look at today's linux-next?  It looks correct in
> > there.  Andrew updated his patch series over the weekend.  
> 
> Yes, I am looking at today's next. Both the source code and the commit
> 728e0fbf263e3ed359c10cb13623390564102881 have wrong "if (merged)" (put
> in wrong hunk).

OK, I have replaced that commit with this:

From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
Subject: mm/vmalloc.c: get rid of one single unlink_va() when merge

It does not make sense to try to "unlink" the node that is definitely not
linked with a list nor tree.  On the first merge step VA just points to
the previously disconnected busy area.

On the second step, check if the node has been merged and do "unlink" if
so, because now it points to an object that must be linked.

Link: http://lkml.kernel.org/r/20190527151843.27416-4-urezki@gmail.com
Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
Acked-by: Hillf Danton <hdanton@sina.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Joel Fernandes <joelaf@google.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oleksiy Avramchenko <oleksiy.avramchenko@sonymobile.com>
Cc: Roman Gushchin <guro@fb.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Garnier <thgarnie@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/vmalloc.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

--- a/mm/vmalloc.c~mm-vmap-get-rid-of-one-single-unlink_va-when-merge
+++ a/mm/vmalloc.c
@@ -719,9 +719,6 @@ merge_or_add_vmap_area(struct vmap_area
 			/* Check and update the tree if needed. */
 			augment_tree_propagate_from(sibling);
 
-			/* Remove this VA, it has been merged. */
-			unlink_va(va, root);
-
 			/* Free vmap_area object. */
 			kmem_cache_free(vmap_area_cachep, va);
 
@@ -746,12 +743,11 @@ merge_or_add_vmap_area(struct vmap_area
 			/* Check and update the tree if needed. */
 			augment_tree_propagate_from(sibling);
 
-			/* Remove this VA, it has been merged. */
-			unlink_va(va, root);
+			if (merged)
+				unlink_va(va, root);
 
 			/* Free vmap_area object. */
 			kmem_cache_free(vmap_area_cachep, va);
-
 			return;
 		}
 	}
_

Which is the patch from mmots but with different line numbers.
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Krzysztof Kozlowski]

On Mon, 3 Jun 2019 at 17:11, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Krzysztof,
>
> On Mon, 3 Jun 2019 16:35:22 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Mon, 3 Jun 2019 at 16:32, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > On Mon, 3 Jun 2019 16:10:40 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >
> > > > Indeed it looks like effect of merge conflict resolution or applying.
> > > > When I look at MMOTS, it is the same as yours:
> > > > http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/commit/?id=b77b8cce67f246109f9d87417a32cd38f0398f2f
> > > >
> > > > However in linux-next it is different.
> > > >
> > > > Stephen, any thoughts?
> > >
> > > Have you had a look at today's linux-next?  It looks correct in
> > > there.  Andrew updated his patch series over the weekend.
> >
> > Yes, I am looking at today's next. Both the source code and the commit
> > 728e0fbf263e3ed359c10cb13623390564102881 have wrong "if (merged)" (put
> > in wrong hunk).
>
> OK, I have replaced that commit with this:

Thank you!

Best regards,
Krzysztof

Re: [BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Dexuan-Linux Cui]

On Mon, Jun 3, 2019 at 7:37 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, 3 Jun 2019 at 16:32, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi Krzysztof,
> >
> > On Mon, 3 Jun 2019 16:10:40 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > Indeed it looks like effect of merge conflict resolution or applying.
> > > When I look at MMOTS, it is the same as yours:
> > > http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/commit/?id=b77b8cce67f246109f9d87417a32cd38f0398f2f
> > >
> > > However in linux-next it is different.
> > >
> > > Stephen, any thoughts?
> >
> > Have you had a look at today's linux-next?  It looks correct in
> > there.  Andrew updated his patch series over the weekend.
>
> Yes, I am looking at today's next. Both the source code and the commit
> 728e0fbf263e3ed359c10cb13623390564102881 have wrong "if (merged)" (put
> in wrong hunk).
>
> Best regards,
> Krzysztof

FYI, we also see the issue in our x86 VM running on Hyper-V.

Thanks,
Dexuan

Re: [BUG BISECT] bug mm/vmalloc.c:470 (mm/vmalloc.c: get rid of one single unlink_va() when merge)

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]

Hi all,

On Tue, 4 Jun 2019 00:31:53 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Krzysztof,
> 
> On Mon, 3 Jun 2019 16:10:40 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > Indeed it looks like effect of merge conflict resolution or applying.
> > When I look at MMOTS, it is the same as yours:
> > http://git.cmpxchg.org/cgit.cgi/linux-mmots.git/commit/?id=b77b8cce67f246109f9d87417a32cd38f0398f2f
> > 
> > However in linux-next it is different.
> > 
> > Stephen, any thoughts?  
> 
> Have you had a look at today's linux-next?  It looks correct in
> there.  Andrew updated his patch series over the weekend.

Actually, this is the patch from mmotm (note 'm'):

From: "Uladzislau Rezki (Sony)" <urezki@gmail.com>
Subject: mm/vmalloc.c: get rid of one single unlink_va() when merge

It does not make sense to try to "unlink" the node that is definitely not
linked with a list nor tree.  On the first merge step VA just points to
the previously disconnected busy area.

On the second step, check if the node has been merged and do "unlink" if
so, because now it points to an object that must be linked.

Link: http://lkml.kernel.org/r/20190527151843.27416-4-urezki@gmail.com
Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
Acked-by: Hillf Danton <hdanton@sina.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Joel Fernandes <joelaf@google.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oleksiy Avramchenko <oleksiy.avramchenko@sonymobile.com>
Cc: Roman Gushchin <guro@fb.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Thomas Garnier <thgarnie@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/vmalloc.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

--- a/mm/vmalloc.c~mm-vmap-get-rid-of-one-single-unlink_va-when-merge
+++ a/mm/vmalloc.c
@@ -719,8 +719,8 @@ merge_or_add_vmap_area(struct vmap_area
 			/* Check and update the tree if needed. */
 			augment_tree_propagate_from(sibling);
 
-			/* Remove this VA, it has been merged. */
-			unlink_va(va, root);
+			if (merged)
+				unlink_va(va, root);
 
 			/* Free vmap_area object. */
 			kmem_cache_free(vmap_area_cachep, va);
@@ -746,9 +746,6 @@ merge_or_add_vmap_area(struct vmap_area
 			/* Check and update the tree if needed. */
 			augment_tree_propagate_from(sibling);
 
-			/* Remove this VA, it has been merged. */
-			unlink_va(va, root);
-
 			/* Free vmap_area object. */
 			kmem_cache_free(vmap_area_cachep, va);
 
_

Do I need to replace that for tomorrow?
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]