[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <501901E4.7060005@kernel.dk>
Date: Wed, 01 Aug 2012 12:16:04 +0200
From: Jens Axboe <axboe@...nel.dk>
To: Olof Johansson <olof@...om.net>
CC: tj@...nel.org, linux-kernel@...r.kernel.org, vgoyal@...hat.com,
torvalds@...ux-foundation.org
Subject: Re: [REGRESSION] [PATCH] block: uninitialized ioc->nr_tasks triggers
WARN_ON
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] [<ffffffff8107ce8a>] warn_slowpath_common+0x7a/0xb0
> [ 10.886912] [<ffffffff8107ced5>] warn_slowpath_null+0x15/0x20
> [ 10.886913] [<ffffffff8107c088>] copy_process+0x1488/0x1560
> [ 10.886914] [<ffffffff8107c244>] do_fork+0xb4/0x340
> [ 10.886918] [<ffffffff8108effa>] ? recalc_sigpending+0x1a/0x50
> [ 10.886919] [<ffffffff8108f6b2>] ? __set_task_blocked+0x32/0x80
> [ 10.886920] [<ffffffff81091afa>] ? __set_current_blocked+0x3a/0x60
> [ 10.886923] [<ffffffff81051db3>] sys_clone+0x23/0x30
> [ 10.886925] [<ffffffff8179bd73>] stub_clone+0x13/0x20
> [ 10.886927] [<ffffffff8179baa2>] ? 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 <tj@...nel.org>
> 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 <tj@...nel.org>
> Cc: Vivek Goyal <vgoyal@...hat.com>
> Signed-off-by: Jens Axboe <axboe@...nel.dk>
>
>
> 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 <olof@...om.net>
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists