From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754371Ab2HAKQR (ORCPT ); Wed, 1 Aug 2012 06:16:17 -0400 Received: from merlin.infradead.org ([205.233.59.134]:35511 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752430Ab2HAKQQ (ORCPT ); Wed, 1 Aug 2012 06:16:16 -0400 Message-ID: <501901E4.7060005@kernel.dk> Date: Wed, 01 Aug 2012 12:16:04 +0200 From: Jens Axboe MIME-Version: 1.0 To: Olof Johansson CC: tj@kernel.org, linux-kernel@vger.kernel.org, vgoyal@redhat.com, torvalds@linux-foundation.org Subject: Re: [REGRESSION] [PATCH] block: uninitialized ioc->nr_tasks triggers WARN_ON References: <20120717213643.GA24726@quad.lixom.net> In-Reply-To: <20120717213643.GA24726@quad.lixom.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/17/2012 11:36 PM, Olof Johansson wrote: > Hi, > > I'm using the old-fashioned 'dump' backup tool, and I noticed that it spews the > below warning as of 3.5-rc1 and later (3.4 is fine): > > [ 10.886893] ------------[ cut here ]------------ > [ 10.886904] WARNING: at include/linux/iocontext.h:140 copy_process+0x1488/0x1560() > [ 10.886905] Hardware name: Bochs > [ 10.886906] Modules linked in: > [ 10.886908] Pid: 2430, comm: dump Not tainted 3.5.0-rc7+ #27 > [ 10.886908] Call Trace: > [ 10.886911] [] warn_slowpath_common+0x7a/0xb0 > [ 10.886912] [] warn_slowpath_null+0x15/0x20 > [ 10.886913] [] copy_process+0x1488/0x1560 > [ 10.886914] [] do_fork+0xb4/0x340 > [ 10.886918] [] ? recalc_sigpending+0x1a/0x50 > [ 10.886919] [] ? __set_task_blocked+0x32/0x80 > [ 10.886920] [] ? __set_current_blocked+0x3a/0x60 > [ 10.886923] [] sys_clone+0x23/0x30 > [ 10.886925] [] stub_clone+0x13/0x20 > [ 10.886927] [] ? system_call_fastpath+0x16/0x1b > [ 10.886928] ---[ end trace 32a14af7ee6a590b ]--- > > Reproducing is easy, I can hit it on a KVM system with a very basic > config (x86_64 make defconfig + enable the drivers needed). To hit it, > just install dump (on debian/ubuntu, not sure what the package might be > called on Fedora), and: > > dump -o -f /tmp/foo / > > You'll see the warning in dmesg once it forks off the I/O process and > starts dumping filesystem contents. > > I bisected it down to the following commit: > > commit f6e8d01bee036460e03bd4f6a79d014f98ba712e > Author: Tejun Heo > Date: Mon Mar 5 13:15:26 2012 -0800 > > block: add io_context->active_ref > > Currently ioc->nr_tasks is used to decide two things - whether an ioc > is done issuing IOs and whether it's shared by multiple tasks. This > patch separate out the first into ioc->active_ref, which is acquired > and released using {get|put}_io_context_active() respectively. > > This will be used to associate bio's with a given task. This patch > doesn't introduce any visible behavior change. > > Signed-off-by: Tejun Heo > Cc: Vivek Goyal > Signed-off-by: Jens Axboe > > > It seems like the init of ioc->nr_tasks was removed in that patch, > so it starts out at 0 instead of 1. > > Tejun, is the right thing here to add back the init, or should something else > be done? > > The below patch removes the warning, but I haven't done any more extensive > testing on it. > > > Signed-off-by: Olof Johansson > > diff --git a/block/blk-ioc.c b/block/blk-ioc.c > index 893b800..fab4cdd 100644 > --- a/block/blk-ioc.c > +++ b/block/blk-ioc.c > @@ -244,6 +244,7 @@ int create_task_io_context(struct task_struct *task, gfp_t gfp_flags, int node) > > /* initialize */ > atomic_long_set(&ioc->refcount, 1); > + atomic_set(&ioc->nr_tasks, 1); > atomic_set(&ioc->active_ref, 1); > spin_lock_init(&ioc->lock); > INIT_RADIX_TREE(&ioc->icq_tree, GFP_ATOMIC | __GFP_HIGH); Thanks Olof, analysis (and patch) look correct. Applied for 3.6. -- Jens Axboe