[kernel] silence another rcu_reference warning

Dave Jones davej at fedoraproject.org
Thu Sep 30 19:57:22 UTC 2010


commit e67d8276aa83126a235ce4a23d2b302e3d7267ea
Author: Dave Jones <davej at redhat.com>
Date:   Thu Sep 30 15:57:12 2010 -0400

    silence another rcu_reference warning

 kernel.spec                       |    8 ++
 linux-2.6-rcu-sched-warning.patch |  215 +++++++++++++++++++++++++++++++++++++
 2 files changed, 223 insertions(+), 0 deletions(-)
---
diff --git a/kernel.spec b/kernel.spec
index 3613ec7..08c29b8 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -681,6 +681,8 @@ Patch2912: linux-2.6-v4l-dvb-ir-core-update.patch
 #Patch2916: lirc-staging-2.6.36-fixes.patch
 Patch2917: hdpvr-ir-enable.patch
 
+Patch3000: linux-2.6-rcu-sched-warning.patch
+
 # fs fixes
 
 # NFSv4
@@ -1293,6 +1295,9 @@ ApplyOptionalPatch linux-2.6-v4l-dvb-experimental.patch
 # enable IR receiver on Hauppauge HD PVR (v4l-dvb merge pending)
 ApplyPatch hdpvr-ir-enable.patch
 
+# silence another rcu_reference warning
+ApplyPatch linux-2.6-rcu-sched-warning.patch
+
 # Patches headed upstream
 ApplyPatch disable-i8042-check-on-apple-mac.patch
 
@@ -1939,6 +1944,9 @@ fi
 #                 ||     ||
 
 %changelog
+* Thu Sep 30 2010 Dave Jones <davej at redhat.com>
+- silence another rcu_reference warning
+
 * Thu Sep 30 2010 Kyle McMartin <kyle at redhat.com> 2.6.36-0.30.rc6
 - Collection of patches to make intel_ips work properly.
 
diff --git a/linux-2.6-rcu-sched-warning.patch b/linux-2.6-rcu-sched-warning.patch
new file mode 100644
index 0000000..ab3ff00
--- /dev/null
+++ b/linux-2.6-rcu-sched-warning.patch
@@ -0,0 +1,215 @@
+From davej  Thu Sep 16 11:55:58 2010
+Return-Path: linux-kernel-owner at vger.kernel.org
+X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on gelk
+X-Spam-Level: 
+X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED,
+	T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1
+Received: from mail.corp.redhat.com [10.5.5.52]
+	by gelk with IMAP (fetchmail-6.3.17)
+	for <davej at localhost> (single-drop); Thu, 16 Sep 2010 11:55:58 -0400 (EDT)
+Received: from zmta02.collab.prod.int.phx2.redhat.com (LHLO
+ zmta02.collab.prod.int.phx2.redhat.com) (10.5.5.32) by
+ mail04.corp.redhat.com with LMTP; Thu, 16 Sep 2010 11:51:27 -0400 (EDT)
+Received: from localhost (localhost.localdomain [127.0.0.1])
+	by zmta02.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 4889C9FC56;
+	Thu, 16 Sep 2010 11:51:27 -0400 (EDT)
+Received: from zmta02.collab.prod.int.phx2.redhat.com ([127.0.0.1])
+	by localhost (zmta02.collab.prod.int.phx2.redhat.com [127.0.0.1]) (amavisd-new, port 10024)
+	with ESMTP id 94mQrmwfCpY4; Thu, 16 Sep 2010 11:51:27 -0400 (EDT)
+Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16])
+	by zmta02.collab.prod.int.phx2.redhat.com (Postfix) with ESMTP id 0DBDB9FC4B;
+	Thu, 16 Sep 2010 11:51:27 -0400 (EDT)
+Received: from mx1.redhat.com (ext-mx05.extmail.prod.ext.phx2.redhat.com [10.5.110.9])
+	by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o8GFpQnO003857;
+	Thu, 16 Sep 2010 11:51:26 -0400
+Received: from vger.kernel.org (vger.kernel.org [209.132.180.67])
+	by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o8GFFCFE031066;
+	Thu, 16 Sep 2010 11:51:17 -0400
+Received: (majordomo at vger.kernel.org) by vger.kernel.org via listexpand
+	id S1755493Ab0IPPvH (ORCPT <rfc822;jasowang at redhat.com> + 41 others);
+	Thu, 16 Sep 2010 11:51:07 -0400
+Received: from casper.infradead.org ([85.118.1.10]:41834 "EHLO
+	casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
+	with ESMTP id S1754921Ab0IPPvC convert rfc822-to-8bit (ORCPT
+	<rfc822;linux-kernel at vger.kernel.org>);
+	Thu, 16 Sep 2010 11:51:02 -0400
+Received: from f199130.upc-f.chello.nl ([80.56.199.130] helo=laptop)
+	by casper.infradead.org with esmtpsa (Exim 4.72 #1 (Red Hat Linux))
+	id 1OwGjI-0003VE-Ux; Thu, 16 Sep 2010 15:50:33 +0000
+Received: by laptop (Postfix, from userid 1000)
+	id 6DCDB100AEB1D; Thu, 16 Sep 2010 17:50:32 +0200 (CEST)
+Subject: Re: 2.6.35-stable/ppc64/p7: suspicious rcu_dereference_check()
+ usage detected during 2.6.35-stable boot
+From: Peter Zijlstra <peterz at infradead.org>
+To: paulmck at linux.vnet.ibm.com
+Cc: Subrata Modak <subrata at linux.vnet.ibm.com>,
+        linux-kernel <linux-kernel at vger.kernel.org>,
+        Li Zefan <lizf at cn.fujitsu.com>, Linuxppc-dev <Linuxppc-dev at ozlabs.org>,
+        sachinp <sachinp at linux.vnet.ibm.com>,
+        DIVYA PRAKASH <dipraksh at linux.vnet.ibm.com>,
+        "Valdis.Kletnieks" <Valdis.Kletnieks at vt.edu>
+In-Reply-To: <20100809161200.GC3026 at linux.vnet.ibm.com>
+References: <1280739132.15317.9.camel at subratamodak.linux.ibm.com>
+	 <20100809161200.GC3026 at linux.vnet.ibm.com>
+Content-Type: text/plain; charset="UTF-8"
+Content-Transfer-Encoding: 8BIT
+Date: 	Thu, 16 Sep 2010 17:50:31 +0200
+Message-ID: <1284652231.2275.569.camel at laptop>
+Mime-Version: 1.0
+Sender: linux-kernel-owner at vger.kernel.org
+Precedence: bulk
+List-ID: <linux-kernel.vger.kernel.org>
+X-Mailing-List: 	linux-kernel at vger.kernel.org
+X-RedHat-Spam-Score: -2.31  (RCVD_IN_DNSWL_MED,T_RP_MATCHES_RCVD)
+X-Scanned-By: MIMEDefang 2.67 on 10.5.11.16
+X-Scanned-By: MIMEDefang 2.67 on 10.5.110.9
+Status: RO
+Content-Length: 6752
+Lines: 145
+
+On Mon, 2010-08-09 at 09:12 -0700, Paul E. McKenney wrote:
+
+> > [    0.051203] CPU0: AMD QEMU Virtual CPU version 0.12.4 stepping 03
+> > [    0.052999] lockdep: fixing up alternatives.
+> > [    0.054105]
+> > [    0.054106] ===================================================
+> > [    0.054999] [ INFO: suspicious rcu_dereference_check() usage. ]
+> > [    0.054999] ---------------------------------------------------
+> > [    0.054999] kernel/sched.c:616 invoked rcu_dereference_check() without protection!
+> > [    0.054999]
+> > [    0.054999] other info that might help us debug this:
+> > [    0.054999]
+> > [    0.054999]
+> > [    0.054999] rcu_scheduler_active = 1, debug_locks = 1
+> > [    0.054999] 3 locks held by swapper/1:
+> > [    0.054999]  #0:  (cpu_add_remove_lock){+.+.+.}, at: [<ffffffff814be933>] cpu_up+0x42/0x6a
+> > [    0.054999]  #1:  (cpu_hotplug.lock){+.+.+.}, at: [<ffffffff810400d8>] cpu_hotplug_begin+0x2a/0x51
+> > [    0.054999]  #2:  (&rq->lock){-.-...}, at: [<ffffffff814be2f7>] init_idle+0x2f/0x113
+> > [    0.054999]
+> > [    0.054999] stack backtrace:
+> > [    0.054999] Pid: 1, comm: swapper Not tainted 2.6.35 #1
+> > [    0.054999] Call Trace:
+> > [    0.054999]  [<ffffffff81068054>] lockdep_rcu_dereference+0x9b/0xa3
+> > [    0.054999]  [<ffffffff810325c3>] task_group+0x7b/0x8a
+> > [    0.054999]  [<ffffffff810325e5>] set_task_rq+0x13/0x40
+> > [    0.054999]  [<ffffffff814be39a>] init_idle+0xd2/0x113
+> > [    0.054999]  [<ffffffff814be78a>] fork_idle+0xb8/0xc7
+> > [    0.054999]  [<ffffffff81068717>] ? mark_held_locks+0x4d/0x6b
+> > [    0.054999]  [<ffffffff814bcebd>] do_fork_idle+0x17/0x2b
+> > [    0.054999]  [<ffffffff814bc89b>] native_cpu_up+0x1c1/0x724
+> > [    0.054999]  [<ffffffff814bcea6>] ? do_fork_idle+0x0/0x2b
+> > [    0.054999]  [<ffffffff814be876>] _cpu_up+0xac/0x127
+> > [    0.054999]  [<ffffffff814be946>] cpu_up+0x55/0x6a
+> > [    0.054999]  [<ffffffff81ab562a>] kernel_init+0xe1/0x1ff
+> > [    0.054999]  [<ffffffff81003854>] kernel_thread_helper+0x4/0x10
+> > [    0.054999]  [<ffffffff814c353c>] ? restore_args+0x0/0x30
+> > [    0.054999]  [<ffffffff81ab5549>] ? kernel_init+0x0/0x1ff
+> > [    0.054999]  [<ffffffff81003850>] ? kernel_thread_helper+0x0/0x10
+> > [    0.056074] Booting Node   0, Processors  #1lockdep: fixing up alternatives.
+> > [    0.130045]  #2lockdep: fixing up alternatives.
+> > [    0.203089]  #3 Ok.
+> > [    0.275286] Brought up 4 CPUs
+> > [    0.276005] Total of 4 processors activated (16017.17 BogoMIPS).
+> 
+> This does look like a new one, thank you for reporting it!
+> 
+> Here is my analysis, which should at least provide some humor value to
+> those who understand the code better than I do.  ;-)
+> 
+> So the corresponding rcu_dereference_check() is in
+> task_subsys_state_check(), and is fetching the cpu_cgroup_subsys_id
+> element of the newly created task's task->cgroups->subsys[] array.
+> The "git grep" command finds only three uses of cpu_cgroup_subsys_id,
+> but no definition.
+> 
+> Now, fork_idle() invokes copy_process(), which invokes cgroup_fork(),
+> which sets the child process's ->cgroups pointer to that of the parent,
+> also invoking get_css_set(), which increments the corresponding reference
+> count, doing both operations under task_lock() protection (->alloc_lock).
+> Because fork_idle() does not specify any of CLONE_NEWNS, CLONE_NEWUTS,
+> CLONE_NEWIPC, CLONE_NEWPID, or CLONE_NEWNET, copy_namespaces() should
+> not create a new namespace, and so there should be no ns_cgroup_clone().
+> We should thus retain the parent's ->cgroups pointer.  And copy_process()
+> installs the new task in the various lists, so that the task is externally
+> accessible upon return.
+> 
+> After a non-error return from copy_process(), fork_init() invokes
+> init_idle_pid(), which does not appear to affect the task's cgroup
+> state.  Next fork_init() invokes init_idle(), which in turn invokes
+> __set_task_cpu(), which invokes set_task_rq(), which calls task_group()
+> several times, which calls task_subsys_state_check(), which calls the
+> rcu_dereference_check() that complained above.
+> 
+> However, the result returns by rcu_dereference_check() is stored into
+> the task structure:
+> 
+> 	p->se.cfs_rq = task_group(p)->cfs_rq[cpu];
+> 	p->se.parent = task_group(p)->se[cpu];
+> 
+> This means that the corresponding structure must have been tied down with
+> a reference count or some such.  If such a reference has been taken, then
+> this complaint is a false positive, and could be suppressed by putting
+> rcu_read_lock() and rcu_read_unlock() around the call to init_idle()
+> from fork_idle().  However, although, reference to the enclosing ->cgroups
+> struct css_set is held, it is not clear to me that this reference applies
+> to the structures pointed to by the ->subsys[] array, especially given
+> that the cgroup_subsys_state structures referenced by this array have
+> their own reference count, which does not appear to me to be acquired
+> by this code path.
+> 
+> Or are the cgroup_subsys_state structures referenced by idle tasks
+> never freed or some such?
+
+I would hope so!, the idle tasks should be part of the root cgroup,
+which is not removable.
+
+The problem is that while we do in-fact hold rq->lock, the newly spawned
+idle thread's cpu is not yet set to the correct cpu so the lockdep check
+in task_group():
+
+  lockdep_is_held(&task_rq(p)->lock)
+
+will fail.
+
+But of a chicken and egg problem. Setting the cpu needs to have the cpu
+set ;-)
+
+Ingo, why do we have rq->lock there at all? The CPU isn't up and running
+yet, nothing should be touching it.
+
+Signed-off-by: Peter Zijlstra <a.p.zijlstra at chello.nl>
+---
+ kernel/sched.c |   12 ++++++++++++
+ 1 files changed, 12 insertions(+), 0 deletions(-)
+
+diff --git a/kernel/sched.c b/kernel/sched.c
+index bd8b487..6241049 100644
+--- a/kernel/sched.c
++++ b/kernel/sched.c
+@@ -5332,7 +5332,19 @@ void __cpuinit init_idle(struct task_struct *idle, int cpu)
+ 	idle->se.exec_start = sched_clock();
+ 
+ 	cpumask_copy(&idle->cpus_allowed, cpumask_of(cpu));
++	/*
++	 * We're having a chicken and egg problem, even though we are
++	 * holding rq->lock, the cpu isn't yet set to this cpu so the
++	 * lockdep check in task_group() will fail.
++	 *
++	 * Similar case to sched_fork(). / Alternatively we could
++	 * use task_rq_lock() here and obtain the other rq->lock.
++	 *
++	 * Silence PROVE_RCU
++	 */
++	rcu_read_lock();
+ 	__set_task_cpu(idle, cpu);
++	rcu_read_unlock();
+ 
+ 	rq->curr = rq->idle = idle;
+ #if defined(CONFIG_SMP) && defined(__ARCH_WANT_UNLOCKED_CTXSW)
+
+--
+To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
+the body of a message to majordomo at vger.kernel.org
+More majordomo info at  http://vger.kernel.org/majordomo-info.html
+Please read the FAQ at  http://www.tux.org/lkml/
+


More information about the scm-commits mailing list