From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 36870C2BA12 for ; Thu, 2 Apr 2020 15:54:09 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E2C0920737 for ; Thu, 2 Apr 2020 15:54:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="Ay8L+idr" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E2C0920737 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6BE2E8E0008; Thu, 2 Apr 2020 11:54:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 647E68E0007; Thu, 2 Apr 2020 11:54:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 585A28E0008; Thu, 2 Apr 2020 11:54:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0249.hostedemail.com [216.40.44.249]) by kanga.kvack.org (Postfix) with ESMTP id 419D38E0007 for ; Thu, 2 Apr 2020 11:54:08 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id F133A8248047 for ; Thu, 2 Apr 2020 15:54:07 +0000 (UTC) X-FDA: 76663361334.01.owl69_7b530bc8f725d X-HE-Tag: owl69_7b530bc8f725d X-Filterd-Recvd-Size: 9032 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf24.hostedemail.com (Postfix) with ESMTP for ; Thu, 2 Apr 2020 15:54:07 +0000 (UTC) Received: from paulmck-ThinkPad-P72.home (50-39-105-78.bvtn.or.frontiernet.net [50.39.105.78]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 793FD2063A; Thu, 2 Apr 2020 15:54:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1585842846; bh=4Eu298RZ2IWYgmD/vOfOB6+9cpJ48Q2EoWqLKqCJRtc=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=Ay8L+idr6K8LOgJLPqEPdE0Z1gVQT47uLiObP4g66rkbmMb/PXuVhVjBucd7eTcWf n5UA5MbyZBE13CQCInce1Kv4ohKzrPFNYQRND3IQale4O5OWn525djNEuult6w5bUo 1aUY8NBcEBqpe5tiEL6fORJCFENwC9AaI5f/mnkQ= Received: by paulmck-ThinkPad-P72.home (Postfix, from userid 1000) id 499613521885; Thu, 2 Apr 2020 08:54:06 -0700 (PDT) Date: Thu, 2 Apr 2020 08:54:06 -0700 From: "Paul E. McKenney" To: Qian Cai Cc: Michael Ellerman , Peter Zijlstra , Ingo Molnar , juri.lelli@redhat.com, dietmar.eggemann@arm.com, vincent.guittot@linaro.org, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, tglx@linutronix.de, "James.Bottomley@hansenpartnership.com" , deller@gmx.de, linuxppc-dev@lists.ozlabs.org, linux-parisc@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Nicholas Piggin Subject: Re: [PATCH v2] sched/core: fix illegal RCU from offline CPUs Message-ID: <20200402155406.GP19865@paulmck-ThinkPad-P72> Reply-To: paulmck@kernel.org References: <20200401214033.8448-1-cai@lca.pw> <87369mt9kf.fsf@mpe.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Apr 02, 2020 at 10:00:16AM -0400, Qian Cai wrote: >=20 >=20 > > On Apr 2, 2020, at 7:24 AM, Michael Ellerman wro= te: > >=20 > > Qian Cai writes: > >> From: Peter Zijlstra > >>=20 > >> In the CPU-offline process, it calls mmdrop() after idle entry and t= he > >> subsequent call to cpuhp_report_idle_dead(). Once execution passes t= he > >> call to rcu_report_dead(), RCU is ignoring the CPU, which results in > >> lockdep complaining when mmdrop() uses RCU from either memcg or > >> debugobjects below. > >>=20 > >> Fix it by cleaning up the active_mm state from BP instead. Every arc= h > >> which has CONFIG_HOTPLUG_CPU should have already called idle_task_ex= it() > >> from AP. The only exception is parisc because it switches them to > >> &init_mm unconditionally (see smp_boot_one_cpu() and smp_cpu_init())= , > >> but the patch will still work there because it calls mmgrab(&init_mm= ) in > >> smp_cpu_init() and then should call mmdrop(&init_mm) in finish_cpu()= . > >=20 > > Thanks for debugging this. How did you hit it in the first place? >=20 > Just repeatedly offline/online CPUs which will eventually cause an idle= thread > refcount goes to 0 and trigger __mmdrop() and of course it needs to ena= ble > lockdep (PROVE_RCU?) as well as having luck to hit the cgroup, workqueu= e > or debugobject code paths to call RCU. >=20 > >=20 > > A link to the original thread would have helped me: > >=20 > > https://lore.kernel.org/lkml/20200113190331.12788-1-cai@lca.pw/ > >=20 > >> WARNING: suspicious RCU usage > >> ----------------------------- > >> kernel/workqueue.c:710 RCU or wq_pool_mutex should be held! > >>=20 > >> other info that might help us debug this: > >>=20 > >> RCU used illegally from offline CPU! > >> Call Trace: > >> dump_stack+0xf4/0x164 (unreliable) > >> lockdep_rcu_suspicious+0x140/0x164 > >> get_work_pool+0x110/0x150 > >> __queue_work+0x1bc/0xca0 > >> queue_work_on+0x114/0x120 > >> css_release+0x9c/0xc0 > >> percpu_ref_put_many+0x204/0x230 > >> free_pcp_prepare+0x264/0x570 > >> free_unref_page+0x38/0xf0 > >> __mmdrop+0x21c/0x2c0 > >> idle_task_exit+0x170/0x1b0 > >> pnv_smp_cpu_kill_self+0x38/0x2e0 > >> cpu_die+0x48/0x64 > >> arch_cpu_idle_dead+0x30/0x50 > >> do_idle+0x2f4/0x470 > >> cpu_startup_entry+0x38/0x40 > >> start_secondary+0x7a8/0xa80 > >> start_secondary_resume+0x10/0x14 > >=20 > > Do we know when this started happening? ie. can we determine a Fixes > > tag? >=20 > I don=E2=80=99t know. I looked at some commits that it seems the code w= as like that > even 10-year ago. It must be nobody who cares to run lockdep (PROVE_RCU= ?) > with CPU hotplug very regularly. I do run this combination quite frequently, but only as part of rcutorture, which might not be a representative workload. For one thing, it has a minimal userspace consisting only of a trivial init program. I don't recall having ever seen this. (I have seen one recent complaint about an IPI being sent to an offline CPU, but I cannot prove that this was not due to RCU bugs that I was chasing at the time.) Thanx, Paul > >> > >> Signed-off-by: Qian Cai > >> --- > >> arch/powerpc/platforms/powernv/smp.c | 1 - > >> include/linux/sched/mm.h | 2 ++ > >> kernel/cpu.c | 18 +++++++++++++++++- > >> kernel/sched/core.c | 5 +++-- > >> 4 files changed, 22 insertions(+), 4 deletions(-) > >>=20 > >> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/pla= tforms/powernv/smp.c > >> index 13e251699346..b2ba3e95bda7 100644 > >> --- a/arch/powerpc/platforms/powernv/smp.c > >> +++ b/arch/powerpc/platforms/powernv/smp.c > >> @@ -167,7 +167,6 @@ static void pnv_smp_cpu_kill_self(void) > >> /* Standard hot unplug procedure */ > >>=20 > >> idle_task_exit(); > >> - current->active_mm =3D NULL; /* for sanity */ > >=20 > > If I'm reading it right, we'll now be running with active_mm =3D=3D i= nit_mm > > in the offline loop. > >=20 > > I guess that's fine, I can't think of any reason it would matter, and= it > > seems like we were NULL'ing it out just for paranoia's sake not becau= se > > of any actual problem. > >=20 > > Acked-by: Michael Ellerman (powerpc) > >=20 > >=20 > > cheers > >=20 > >> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h > >> index c49257a3b510..a132d875d351 100644 > >> --- a/include/linux/sched/mm.h > >> +++ b/include/linux/sched/mm.h > >> @@ -49,6 +49,8 @@ static inline void mmdrop(struct mm_struct *mm) > >> __mmdrop(mm); > >> } > >>=20 > >> +void mmdrop(struct mm_struct *mm); > >> + > >> /* > >> * This has to be called after a get_task_mm()/mmget_not_zero() > >> * followed by taking the mmap_sem for writing before modifying the > >> diff --git a/kernel/cpu.c b/kernel/cpu.c > >> index 2371292f30b0..244d30544377 100644 > >> --- a/kernel/cpu.c > >> +++ b/kernel/cpu.c > >> @@ -3,6 +3,7 @@ > >> * > >> * This code is licenced under the GPL. > >> */ > >> +#include > >> #include > >> #include > >> #include > >> @@ -564,6 +565,21 @@ static int bringup_cpu(unsigned int cpu) > >> return bringup_wait_for_ap(cpu); > >> } > >>=20 > >> +static int finish_cpu(unsigned int cpu) > >> +{ > >> + struct task_struct *idle =3D idle_thread_get(cpu); > >> + struct mm_struct *mm =3D idle->active_mm; > >> + > >> + /* > >> + * idle_task_exit() will have switched to &init_mm, now > >> + * clean up any remaining active_mm state. > >> + */ > >> + if (mm !=3D &init_mm) > >> + idle->active_mm =3D &init_mm; > >> + mmdrop(mm); > >> + return 0; > >> +} > >> + > >> /* > >> * Hotplug state machine related functions > >> */ > >> @@ -1549,7 +1565,7 @@ static struct cpuhp_step cpuhp_hp_states[] =3D= { > >> [CPUHP_BRINGUP_CPU] =3D { > >> .name =3D "cpu:bringup", > >> .startup.single =3D bringup_cpu, > >> - .teardown.single =3D NULL, > >> + .teardown.single =3D finish_cpu, > >> .cant_stop =3D true, > >> }, > >> /* Final state before CPU kills itself */ > >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c > >> index a2694ba82874..8787958339d5 100644 > >> --- a/kernel/sched/core.c > >> +++ b/kernel/sched/core.c > >> @@ -6200,13 +6200,14 @@ void idle_task_exit(void) > >> struct mm_struct *mm =3D current->active_mm; > >>=20 > >> BUG_ON(cpu_online(smp_processor_id())); > >> + BUG_ON(current !=3D this_rq()->idle); > >>=20 > >> if (mm !=3D &init_mm) { > >> switch_mm(mm, &init_mm, current); > >> - current->active_mm =3D &init_mm; > >> finish_arch_post_lock_switch(); > >> } > >> - mmdrop(mm); > >> + > >> + /* finish_cpu(), as ran on the BP, will clean up the active_mm sta= te */ > >> } > >>=20 > >> /* > >> --=20 > >> 2.21.0 (Apple Git-122.2) >=20