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

Groups > linux.kernel > #1311258 > unrolled thread

[PATCH 3.2 47/70] vmstat: allocate vmstat_wq before it is used

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 3.2 47/70] vmstat: allocate vmstat_wq before it is used Ben Hutchings <ben@decadent.org.uk> - 2016-01-18 04:40 +0100
    Re: [PATCH 3.2 47/70] vmstat: allocate vmstat_wq before it is used Luis Henriques <luis.henriques@canonical.com> - 2016-01-18 23:30 +0100

#1311258 — [PATCH 3.2 47/70] vmstat: allocate vmstat_wq before it is used

3.2.76-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Michal Hocko <mhocko@suse.com>

commit 751e5f5c753e8d447bcf89f9e96b9616ac081628 upstream.

kernel test robot has reported the following crash:

  BUG: unable to handle kernel NULL pointer dereference at 00000100
  IP: [<c1074df6>] __queue_work+0x26/0x390
  *pdpt = 0000000000000000 *pde = f000ff53f000ff53 *pde = f000ff53f000ff53
  Oops: 0000 [#1] PREEMPT PREEMPT SMP SMP
  CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.4.0-rc4-00139-g373ccbe #1
  Workqueue: events vmstat_shepherd
  task: cb684600 ti: cb7ba000 task.ti: cb7ba000
  EIP: 0060:[<c1074df6>] EFLAGS: 00010046 CPU: 0
  EIP is at __queue_work+0x26/0x390
  EAX: 00000046 EBX: cbb37800 ECX: cbb37800 EDX: 00000000
  ESI: 00000000 EDI: 00000000 EBP: cb7bbe68 ESP: cb7bbe38
   DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
  CR0: 8005003b CR2: 00000100 CR3: 01fd5000 CR4: 000006b0
  Stack:
  Call Trace:
    __queue_delayed_work+0xa1/0x160
    queue_delayed_work_on+0x36/0x60
    vmstat_shepherd+0xad/0xf0
    process_one_work+0x1aa/0x4c0
    worker_thread+0x41/0x440
    kthread+0xb0/0xd0
    ret_from_kernel_thread+0x21/0x40

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>
Tested-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[bwh: Backported to 3.2: This precise race condition doesn't exist, but there
 is a similar potential race with CPU hotplug.  So move the alloc_workqueue()
 above register_cpu_notifier().]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
 mm/vmstat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1203,9 +1203,10 @@ static int __init setup_vmstat(void)
 #ifdef CONFIG_SMP
 	int cpu;
 
+	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
+
 	register_cpu_notifier(&vmstat_notifier);
 
-	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
 	for_each_online_cpu(cpu)
 		start_cpu_timer(cpu);
 #endif

[toc] | [next] | [standalone]

#1311827

On Mon, Jan 18, 2016 at 03:18:35AM +0000, Ben Hutchings wrote:
> 3.2.76-rc1 review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Michal Hocko <mhocko@suse.com>
> 
> commit 751e5f5c753e8d447bcf89f9e96b9616ac081628 upstream.
> 
> kernel test robot has reported the following crash:
> 
>   BUG: unable to handle kernel NULL pointer dereference at 00000100
>   IP: [<c1074df6>] __queue_work+0x26/0x390
>   *pdpt = 0000000000000000 *pde = f000ff53f000ff53 *pde = f000ff53f000ff53
>   Oops: 0000 [#1] PREEMPT PREEMPT SMP SMP
>   CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.4.0-rc4-00139-g373ccbe #1
>   Workqueue: events vmstat_shepherd
>   task: cb684600 ti: cb7ba000 task.ti: cb7ba000
>   EIP: 0060:[<c1074df6>] EFLAGS: 00010046 CPU: 0
>   EIP is at __queue_work+0x26/0x390
>   EAX: 00000046 EBX: cbb37800 ECX: cbb37800 EDX: 00000000
>   ESI: 00000000 EDI: 00000000 EBP: cb7bbe68 ESP: cb7bbe38
>    DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
>   CR0: 8005003b CR2: 00000100 CR3: 01fd5000 CR4: 000006b0
>   Stack:
>   Call Trace:
>     __queue_delayed_work+0xa1/0x160
>     queue_delayed_work_on+0x36/0x60
>     vmstat_shepherd+0xad/0xf0
>     process_one_work+0x1aa/0x4c0
>     worker_thread+0x41/0x440
>     kthread+0xb0/0xd0
>     ret_from_kernel_thread+0x21/0x40
> 
> 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>
> Tested-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> [bwh: Backported to 3.2: This precise race condition doesn't exist, but there
>  is a similar potential race with CPU hotplug.  So move the alloc_workqueue()
>  above register_cpu_notifier().]

FWIW, this looks correct to me and I'm queuing a similar fix for the
3.16 kernel as the race is similar.

Cheers,
--
Luís

> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  mm/vmstat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1203,9 +1203,10 @@ static int __init setup_vmstat(void)
>  #ifdef CONFIG_SMP
>  	int cpu;
>  
> +	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
> +
>  	register_cpu_notifier(&vmstat_notifier);
>  
> -	vmstat_wq = alloc_workqueue("vmstat", WQ_FREEZABLE|WQ_MEM_RECLAIM, 0);
>  	for_each_online_cpu(cpu)
>  		start_cpu_timer(cpu);
>  #endif
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web