Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1304332 > unrolled thread

[PATCH] vmstat: allocate vmstat_wq before it is used

Started byMichal Hocko <mhocko@kernel.org>
First post2016-01-08 11:20 +0100
Last post2016-01-10 21:10 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH] vmstat: allocate vmstat_wq before it is used Michal Hocko <mhocko@kernel.org> - 2016-01-08 11:20 +0100
    Re: [PATCH] vmstat: allocate vmstat_wq before it is used Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-09 01:20 +0100
      Re: [PATCH] vmstat: allocate vmstat_wq before it is used Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-09 01:20 +0100
        Re: [PATCH] vmstat: allocate vmstat_wq before it is used Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-01-09 08:00 +0100
        Re: [PATCH] vmstat: allocate vmstat_wq before it is used Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-01-09 08:10 +0100
        Re: [PATCH] vmstat: allocate vmstat_wq before it is used Michal Hocko <mhocko@kernel.org> - 2016-01-10 21:10 +0100

#1304332 — [PATCH] vmstat: allocate vmstat_wq before it is used

FromMichal Hocko <mhocko@kernel.org>
Date2016-01-08 11:20 +0100
Subject[PATCH] vmstat: allocate vmstat_wq before it is used
Message-ID<qOCjg-5hH-17@gated-at.bofh.it>
From: Michal Hocko <mhocko@suse.com>

kernel test robot has reported the following crash:
[    3.870718] BUG: unable to handle kernel NULL pointer dereferenceNULL pointer dereference at 00000100
 at 00000100
[    3.872615] IP: [<c1074df6>] __queue_work+0x26/0x390 [<c1074df6>] __queue_work+0x26/0x390
[    3.873758] *pdpt = 0000000000000000 *pde = f000ff53f000ff53 *pde = f000ff53f000ff53
[    3.875096] Oops: 0000 [#1] PREEMPT PREEMPT SMP SMP
[    3.876130] CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.4.0-rc4-00139-g373ccbe #1
[    3.878135] Workqueue: events vmstat_shepherd
[    3.879207] task: cb684600 ti: cb7ba000 task.ti: cb7ba000
[    3.880445] EIP: 0060:[<c1074df6>] EFLAGS: 00010046 CPU: 0
[    3.881704] EIP is at __queue_work+0x26/0x390
[    3.882823] EAX: 00000046 EBX: cbb37800 ECX: cbb37800 EDX: 00000000
[    3.884457] ESI: 00000000 EDI: 00000000 EBP: cb7bbe68 ESP: cb7bbe38
[    3.886005]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[    3.887229] CR0: 8005003b CR2: 00000100 CR3: 01fd5000 CR4: 000006b0
[    3.888663] Stack:
[    3.895204] Call Trace:
[    3.895854]  [<c1a381dd>] ? mutex_unlock+0xd/0x10
[    3.897120]  [<c1075221>] __queue_delayed_work+0xa1/0x160
[    3.898530]  [<c10764c6>] queue_delayed_work_on+0x36/0x60
[    3.899790]  [<c11494bd>] vmstat_shepherd+0xad/0xf0
[    3.900899]  [<c1075a7a>] process_one_work+0x1aa/0x4c0
[    3.902093]  [<c10759e2>] ? process_one_work+0x112/0x4c0
[    3.903520]  [<c10ac31e>] ? do_raw_spin_lock+0xe/0x150
[    3.904853]  [<c1075dd1>] worker_thread+0x41/0x440
[    3.906023]  [<c1075d90>] ? process_one_work+0x4c0/0x4c0
[    3.907242]  [<c107b7c0>] kthread+0xb0/0xd0
[    3.908188]  [<c1a3c651>] ret_from_kernel_thread+0x21/0x40
[    3.909601]  [<c107b710>] ? __kthread_parkme+0x80/0x80

The reason is that start_shepherd_timer schedules the shepherd work item
which uses vmstat_wq (vmstat_shepherd) before setup_vmstat allocates
that workqueue so if the further initialization takes more than HZ
we might end up scheduling on a NULL vmstat_wq. This is really unlikely
but not impossible.

Fixes: 373ccbe59270 ("mm, vmstat: allow WQ concurrency to discover memory reclaim doesn't make any progress")
Reported-by: kernel test robot <ying.huang@linux.intel.com>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
Hi Linus,
I am not marking this for stable because I hope we can sneak it into 4.4.
The patch is trivial and obvious. I am sorry about the breakage. If you prefer 
to postpone it to 4.5-rc1 because this is not really that critical and shouldn't
happen most of the time then I will repost with stable tag added.

Thanks!

 mm/vmstat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/vmstat.c b/mm/vmstat.c
index 4ebc17d948cb..c54fd2924f25 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1483,6 +1483,7 @@ static void __init start_shepherd_timer(void)
 		BUG();
 	cpumask_copy(cpu_stat_off, cpu_online_mask);
 
+	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 	schedule_delayed_work(&shepherd,
 		round_jiffies_relative(sysctl_stat_interval));
 }
@@ -1550,7 +1551,6 @@ static int __init setup_vmstat(void)
 
 	start_shepherd_timer();
 	cpu_notifier_register_done();
-	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 #endif
 #ifdef CONFIG_PROC_FS
 	proc_create("buddyinfo", S_IRUGO, NULL, &fragmentation_file_operations);
-- 
2.6.4

[toc] | [next] | [standalone]


#1305101

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-01-09 01:20 +0100
Message-ID<qOPq9-5WM-9@gated-at.bofh.it>
In reply to#1304332
On Fri, Jan 8, 2016 at 2:18 AM, Michal Hocko <mhocko@kernel.org> wrote:
>
> I am not marking this for stable because I hope we can sneak it into 4.4.
> The patch is trivial and obvious.

Was there confirmation this fixed the issue? Just verifying..

               Linus

[toc] | [prev] | [next] | [standalone]


#1305103

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2016-01-09 01:20 +0100
Message-ID<qOPq9-5WM-15@gated-at.bofh.it>
In reply to#1305101
On Fri, Jan 8, 2016 at 4:13 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Was there confirmation this fixed the issue? Just verifying..

Oh, and Tetsuo claims that it needs a cc stable regardless, because
the commit that caused this to appear was marked for stable and
already got backported. Yes?

                   Linus

[toc] | [prev] | [next] | [standalone]


#1305183

FromTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date2016-01-09 08:00 +0100
Message-ID<qOVFg-1M9-1@gated-at.bofh.it>
In reply to#1305103
Linus Torvalds wrote:
> On Fri, Jan 8, 2016 at 4:13 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Was there confirmation this fixed the issue? Just verifying..

I confirmed that

----------
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 4ebc17d..0486d3d 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -27,6 +27,7 @@
 #include <linux/mm_inline.h>
 #include <linux/page_ext.h>
 #include <linux/page_owner.h>
+#include <linux/delay.h>
 
 #include "internal.h"
 
@@ -1550,6 +1551,7 @@ static int __init setup_vmstat(void)
 
 	start_shepherd_timer();
 	cpu_notifier_register_done();
+	ssleep(3);
 	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 #endif
 #ifdef CONFIG_PROC_FS
----------

always reproduces this bug and

----------
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 4ebc17d..358c1b7 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -27,6 +27,7 @@
 #include <linux/mm_inline.h>
 #include <linux/page_ext.h>
 #include <linux/page_owner.h>
+#include <linux/delay.h>
 
 #include "internal.h"
 
@@ -1483,6 +1484,8 @@ static void __init start_shepherd_timer(void)
 		BUG();
 	cpumask_copy(cpu_stat_off, cpu_online_mask);
 
+	ssleep(3);
+	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 	schedule_delayed_work(&shepherd,
 		round_jiffies_relative(sysctl_stat_interval));
 }
@@ -1550,7 +1553,6 @@ static int __init setup_vmstat(void)
 
 	start_shepherd_timer();
 	cpu_notifier_register_done();
-	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 #endif
 #ifdef CONFIG_PROC_FS
 	proc_create("buddyinfo", S_IRUGO, NULL, &fragmentation_file_operations);
----------

never reproduces this bug. Thus, I think it is OK to sneak it into 4.4.

> 
> Oh, and Tetsuo claims that it needs a cc stable regardless, because
> the commit that caused this to appear was marked for stable and
> already got backported. Yes?
> 
>                    Linus
> 

Yes, please.

[toc] | [prev] | [next] | [standalone]


#1305186

FromTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date2016-01-09 08:10 +0100
Message-ID<qOVOW-26C-11@gated-at.bofh.it>
In reply to#1305103
Linus Torvalds wrote:
> On Fri, Jan 8, 2016 at 4:13 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Was there confirmation this fixed the issue? Just verifying..

I confirmed that

----------
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 4ebc17d..0486d3d 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -27,6 +27,7 @@
 #include <linux/mm_inline.h>
 #include <linux/page_ext.h>
 #include <linux/page_owner.h>
+#include <linux/delay.h>
 
 #include "internal.h"
 
@@ -1550,6 +1551,7 @@ static int __init setup_vmstat(void)
 
 	start_shepherd_timer();
 	cpu_notifier_register_done();
+	ssleep(3);
 	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 #endif
 #ifdef CONFIG_PROC_FS
----------

always reproduces this bug and

----------
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 4ebc17d..358c1b7 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -27,6 +27,7 @@
 #include <linux/mm_inline.h>
 #include <linux/page_ext.h>
 #include <linux/page_owner.h>
+#include <linux/delay.h>
 
 #include "internal.h"
 
@@ -1483,6 +1484,8 @@ static void __init start_shepherd_timer(void)
 		BUG();
 	cpumask_copy(cpu_stat_off, cpu_online_mask);
 
+	ssleep(3);
+	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 	schedule_delayed_work(&shepherd,
 		round_jiffies_relative(sysctl_stat_interval));
 }
@@ -1550,7 +1553,6 @@ static int __init setup_vmstat(void)
 
 	start_shepherd_timer();
 	cpu_notifier_register_done();
-	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 #endif
 #ifdef CONFIG_PROC_FS
 	proc_create("buddyinfo", S_IRUGO, NULL, &fragmentation_file_operations);
----------

never reproduces this bug. Thus, I think it is OK to sneak it into 4.4.

> 
> Oh, and Tetsuo claims that it needs a cc stable regardless, because
> the commit that caused this to appear was marked for stable and
> already got backported. Yes?
> 
>                    Linus
> 

Yes, please.

[toc] | [prev] | [next] | [standalone]


#1305671

FromMichal Hocko <mhocko@kernel.org>
Date2016-01-10 21:10 +0100
Message-ID<qPutk-s4-17@gated-at.bofh.it>
In reply to#1305103
On Fri 08-01-16 16:16:37, Linus Torvalds wrote:
> On Fri, Jan 8, 2016 at 4:13 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Was there confirmation this fixed the issue? Just verifying..
> 
> Oh, and Tetsuo claims that it needs a cc stable regardless, because
> the commit that caused this to appear was marked for stable and
> already got backported. Yes?

Yes, I have missed that the original patch got backported to 3.2 stable
tree. I do not see it in other branches.

Thanks!
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web