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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 50CE8C433EF for ; Fri, 15 Oct 2021 15:37:38 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9FEB661037 for ; Fri, 15 Oct 2021 15:37:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 9FEB661037 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4HW9Pq5H6hz3cG2 for ; Sat, 16 Oct 2021 02:37:35 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=arm.com (client-ip=217.140.110.172; helo=foss.arm.com; envelope-from=valentin.schneider@arm.com; receiver=) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lists.ozlabs.org (Postfix) with ESMTP id 4HW9PL1q4Lz2yfn for ; Sat, 16 Oct 2021 02:37:07 +1100 (AEDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6C10111D4; Fri, 15 Oct 2021 08:37:04 -0700 (PDT) Received: from e113632-lin (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 4D3F23F66F; Fri, 15 Oct 2021 08:37:03 -0700 (PDT) From: Valentin Schneider To: Nathan Lynch , linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] powerpc/smp: do not decrement idle task preempt count in CPU offline In-Reply-To: <20211015145548.2269836-1-nathanl@linux.ibm.com> References: <20211015145548.2269836-1-nathanl@linux.ibm.com> Date: Fri, 15 Oct 2021 16:36:57 +0100 Message-ID: <87h7dijo4m.mognet@arm.com> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: srikar@linux.vnet.ibm.com, peterz@infradead.org, linux-kernel@vger.kernel.org, clg@kaod.org, mingo@kernel.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On 15/10/21 09:55, Nathan Lynch wrote: > With PREEMPT_COUNT=y, when a CPU is offlined and then onlined again, we > get: > > BUG: scheduling while atomic: swapper/1/0/0x00000000 > no locks held by swapper/1/0. > CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.15.0-rc2+ #100 > Call Trace: > dump_stack_lvl+0xac/0x108 > __schedule_bug+0xac/0xe0 > __schedule+0xcf8/0x10d0 > schedule_idle+0x3c/0x70 > do_idle+0x2d8/0x4a0 > cpu_startup_entry+0x38/0x40 > start_secondary+0x2ec/0x3a0 > start_secondary_prolog+0x10/0x14 > > This is because powerpc's arch_cpu_idle_dead() decrements the idle task's > preempt count, for reasons explained in commit a7c2bb8279d2 ("powerpc: > Re-enable preemption before cpu_die()"), specifically "start_secondary() > expects a preempt_count() of 0." > > However, since commit 2c669ef6979c ("powerpc/preempt: Don't touch the idle > task's preempt_count during hotplug") and commit f1a0a376ca0c ("sched/core: > Initialize the idle task with preemption disabled"), that justification no > longer holds. > > The idle task isn't supposed to re-enable preemption, so remove the > vestigial preempt_enable() from the CPU offline path. > Humph, I got confused because 2c669ef6979c explicitly mentions hotplug, but that's the hotplug machinery which is already involved for bringing up the secondaries at boot time. IIUC your issue here is the preempt_count being messed up when hot-unplugging a CPU, which leads to fireworks during hotplug (IOW I didn't test my last patch against hotplug - my bad!) Reviewed-by: Valentin Schneider > Tested with pseries and powernv in qemu, and pseries on PowerVM. > > Fixes: 2c669ef6979c ("powerpc/preempt: Don't touch the idle task's preempt_count during hotplug") > Fixes: f1a0a376ca0c ("sched/core: Initialize the idle task with preemption disabled") I think only the first Fixes: is needed. > Signed-off-by: Nathan Lynch > --- > arch/powerpc/kernel/smp.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c > index 9cc7d3dbf439..605bab448f84 100644 > --- a/arch/powerpc/kernel/smp.c > +++ b/arch/powerpc/kernel/smp.c > @@ -1730,8 +1730,6 @@ void __cpu_die(unsigned int cpu) > > void arch_cpu_idle_dead(void) > { > - sched_preempt_enable_no_resched(); > - > /* > * Disable on the down path. This will be re-enabled by > * start_secondary() via start_secondary_resume() below > -- > 2.31.1