From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from youngberry.canonical.com ([91.189.89.112]:41399 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932372AbcARW0o (ORCPT ); Mon, 18 Jan 2016 17:26:44 -0500 Date: Mon, 18 Jan 2016 22:26:39 +0000 From: Luis Henriques To: Ben Hutchings Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, akpm@linux-foundation.org, Linus Torvalds , kernel test robot , Michal Hocko , Tetsuo Handa Subject: Re: [PATCH 3.2 47/70] vmstat: allocate vmstat_wq before it is used Message-ID: <20160118222639.GC21074@charon.olymp> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: stable-owner@vger.kernel.org List-ID: 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 > > commit 751e5f5c753e8d447bcf89f9e96b9616ac081628 upstream. > > kernel test robot has reported the following crash: > > BUG: unable to handle kernel NULL pointer dereference at 00000100 > IP: [] __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:[] 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 > Signed-off-by: Michal Hocko > Tested-by: Tetsuo Handa > Cc: Andrew Morton > Signed-off-by: Linus Torvalds > [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 > --- > 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