From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net [217.70.183.200]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 434B51F1303 for ; Wed, 26 Mar 2025 14:12:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.200 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742998376; cv=none; b=NeQIiOMZ1Q9/bIxRfQjq9WWvrBMPC5clVGHnYK/Je8V/VgHJCWOr7pksQUJG+mBPYKiSNhU5zJriuTSOqq0N54fYxOrjs6gctHjhX/Yj2R2nSr8/BEjwdxLzjla1XS9hEYlTXgUDqflphNMZCtKiose5QDPlxztjhHitmUiXsms= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742998376; c=relaxed/simple; bh=2bcgv8P4N2HWexnLuJm1PG7i9iiyp1zJ3HgLIIEQY5c=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=o8jjbs4kuM75YNKeqwglwR/to6y3v6EaTqv4RmWmnjaQ/qOmAB5A7Pz6Y43vVL2tuc6WNMDHv1PeDEKrAZtXZZz0Vqgi1LUKF6lyscbRAco0VGui7xBlmG6MHyspa2u05wlN0ihkKehKSeznhxM5UWHKRcE27S4lmE5OEfwNm/0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=xenomai.org; spf=pass smtp.mailfrom=xenomai.org; dkim=pass (2048-bit key) header.d=xenomai.org header.i=@xenomai.org header.b=WSYAG1/I; arc=none smtp.client-ip=217.70.183.200 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=xenomai.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=xenomai.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=xenomai.org header.i=@xenomai.org header.b="WSYAG1/I" Received: by mail.gandi.net (Postfix) with ESMTPSA id 07920441B4; Wed, 26 Mar 2025 14:12:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xenomai.org; s=gm1; t=1742998370; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=d9PdB6E4fFnDloIBC/sLxm28/87pvwAL7wFilVETVSw=; b=WSYAG1/ICLVyo+DjOYusaEEicdEP6J0C4uUaIY3kGLdubwUrtaZVs0YZ0fUfrvRj4IRhpo HoItF1kH2GvEVNKosi38M3AcIJ8lfZKkF1e5nB+Npk3Tkdvtbkp5GE1h4xjQi2anYwijTb +NJNAZ4dwi+vxLYA5OA17BGAfqvcDfiEEBe0YnvAktMIukmFPGTY+ZvDVVCsFseH6FWGi6 9N4cZMBx/rHak47hFGk4dAb+7DkGaVutu8OjbhTMMdRqazJWS2BNqpDXoVuu8UcRR6gxwL bzw+Z/7PKA9apj+AJukMv/yqiXqEkivVgRUq9D0GiqUESlc8fdb9cQzax12jYg== From: Philippe Gerum To: Florian Bezdeka Cc: Jan Kiszka , Xenomai Subject: Re: [PATCH] x86: dovetail: Fix inverted debug check in lock_vector_lock In-Reply-To: <87bjtn99pa.fsf@xenomai.org> (Philippe Gerum's message of "Wed, 26 Mar 2025 14:56:49 +0100") References: <199b7a20107bf6756f3579debb7313ff96e33e7d.camel@siemens.com> <87ldst30cn.fsf@xenomai.org> <87bjtn99pa.fsf@xenomai.org> User-Agent: mu4e 1.12.8; emacs 29.4 Date: Wed, 26 Mar 2025 15:12:49 +0100 Message-ID: <875xjv98ym.fsf@xenomai.org> Precedence: bulk X-Mailing-List: xenomai@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain X-GND-State: clean X-GND-Score: -100 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduieehjeegucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjughrpefhvfevufgjfhgffffkgggtsehttdertddtredtnecuhfhrohhmpefrhhhilhhiphhpvgcuifgvrhhumhcuoehrphhmseigvghnohhmrghirdhorhhgqeenucggtffrrghtthgvrhhnpedvlefhvdehkeduheevleegiedtueejgfekhfeijeefvdeijeekgeeigfejhfekgeenucfkphepvdgrtddumegvtdgrmedulegsmeeftggutdemleeklegrmeehtgegsgemsgejfhhfmegsrghfnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepvdgrtddumegvtdgrmedulegsmeeftggutdemleeklegrmeehtgegsgemsgejfhhfmegsrghfpdhhvghlohepphihrhhopdhmrghilhhfrhhomheprhhpmhesgigvnhhomhgrihdrohhrghdpnhgspghrtghpthhtohepfedprhgtphhtthhopeigvghnohhmrghisehlihhsthhsrdhlihhnuhigrdguvghvpdhrtghpthhtohepjhgrnhdrkhhishiikhgrsehsihgvmhgvnhhsrdgtohhmpdhrtghpthhtohepfhhlohhrihgrnhdrsggviiguvghkrgesshhivghmvghnshdrtghomh X-GND-Sasl: rpm@xenomai.org Philippe Gerum writes: > Florian Bezdeka writes: > >> On Tue, 2025-03-25 at 10:50 +0100, Philippe Gerum wrote: >>> Florian Bezdeka writes: >>> >>> > On Mon, 2025-03-24 at 18:18 +0100, Jan Kiszka wrote: >>> > > From: Jan Kiszka >>> > > >>> > > Code and comment were no in line, but the comment actually make more >>> > > sense here, given that unlock_vector_lock will unconditionally reenable >>> > > hard IRQs. >>> > > >>> > > Signed-off-by: Jan Kiszka >>> > > --- >>> > > arch/x86/kernel/apic/vector.c | 2 +- >>> > > 1 file changed, 1 insertion(+), 1 deletion(-) >>> > > >>> > > diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c >>> > > index a08b0dff066ac..af60f138d590b 100644 >>> > > --- a/arch/x86/kernel/apic/vector.c >>> > > +++ b/arch/x86/kernel/apic/vector.c >>> > > @@ -142,7 +142,7 @@ void lock_vector_lock(void) >>> > > * entry. In addition, we assume that hard irqs are on as well >>> > > * (which is the regular case). >>> > > */ >>> > > - WARN_ON_ONCE(irq_pipeline_debug() && !hard_irqs_disabled()); >>> > > + WARN_ON_ONCE(irq_pipeline_debug() && hard_irqs_disabled()); >>> > > hard_cond_local_irq_disable(); >>> > > /* Used to the online set of cpus does not change >>> > > * during assign_irq_vector. >>> > > -- >>> > > 2.43.0 >>> > >>> > With that applied there is still one warning left in 6.14: >>> > >>> > [ 17.921496] ------------[ cut here ]------------ >>> > [ 17.921502] WARNING: CPU: 3 PID: 33 at arch/x86/mm/tlb.c:516 switch_mm_irqs_off+0x39d/0x4f0 >>> > [ 17.921512] Modules linked in: >>> > [ 17.921517] CPU: 3 UID: 0 PID: 33 Comm: cpuhp/3 Not tainted 6.14.0-rc7+ #82 >>> > [ 17.921521] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 >>> > [ 17.921523] IRQ stage: Linux >>> > [ 17.921526] RIP: 0010:switch_mm_irqs_off+0x39d/0x4f0 >>> > [ 17.921531] Code: 06 75 de 65 c6 05 e6 82 db 7e 00 e9 77 fd ff ff 48 8b 05 3e 79 18 01 b9 49 00 00 00 48 89 c2 48 c1 ea 20 0f 30 e9 fb fc ff ff <0f> 0b e9 9f fc ff ff 0f 0b e9 4f fd ff ff 65 48 c7 05e >>> > [ 17.921533] RSP: 0018:ffffc90000133de8 EFLAGS: 00010202 >>> > [ 17.921537] RAX: 0000000000000001 RBX: ffffffff8270e000 RCX: 0000000000000207 >>> > [ 17.921539] RDX: 0000000000000206 RSI: ffffffff8270e000 RDI: ffff8880065ae3c0 >>> > [ 17.921541] RBP: ffff88800378ab80 R08: ffff88803e79c5c8 R09: 000000000000019d >>> > [ 17.921543] R10: ffff88803e79c5a0 R11: 0000000000000001 R12: ffff8880065ae3c0 >>> > [ 17.921545] R13: 0000000000000000 R14: 0000000000000003 R15: ffff88803e79c5c8 >>> > [ 17.921558] FS: 0000000000000000(0000) GS:ffff88803e780000(0000) knlGS:0000000000000000 >>> > [ 17.921574] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> > [ 17.921577] CR2: 000055ed7e138e28 CR3: 0000000003ea8000 CR4: 00000000003506f0 >>> > [ 17.921579] Call Trace: >>> > [ 17.921622] >>> > [ 17.921625] ? __warn+0x85/0x140 >>> > [ 17.921630] ? switch_mm_irqs_off+0x39d/0x4f0 >>> > [ 17.921634] ? report_bug+0x164/0x190 >>> > [ 17.921641] ? handle_bug+0x65/0xd0 >>> > [ 17.921645] ? exc_invalid_op+0xb2/0xd0 >>> > [ 17.921648] ? asm_exc_invalid_op+0x1a/0x20 >>> > [ 17.921653] ? switch_mm_irqs_off+0x39d/0x4f0 >>> > [ 17.921657] ? __pfx_sched_cpu_wait_empty+0x10/0x10 >>> > [ 17.921661] sched_cpu_wait_empty+0x93/0xc0 >>> > [ 17.921665] cpuhp_invoke_callback+0x114/0x4b0 >>> > [ 17.921669] cpuhp_thread_fun+0xee/0x190 >>> > [ 17.921674] ? __pfx_smpboot_thread_fn+0x10/0x10 >>> > [ 17.921678] smpboot_thread_fn+0xdd/0x1d0 >>> > [ 17.921683] kthread+0xf1/0x1e0 >>> > [ 17.921689] ? __pfx_kthread+0x10/0x10 >>> > [ 17.921693] ret_from_fork+0x34/0x50 >>> > [ 17.921698] ? __pfx_kthread+0x10/0x10 >>> > [ 17.921701] ret_from_fork_asm+0x1a/0x30 >>> > [ 17.921717] >>> > [ 17.921719] ---[ end trace 0000000000000000 ]--- >>> > >>> > There were some changes to the scheduler code in between. I'm currently >>> > testing with: >>> > >>> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c >>> > index 50b082b41e7d..4427fc13086d 100644 >>> > --- a/kernel/sched/core.c >>> > +++ b/kernel/sched/core.c >>> > @@ -8032,10 +8032,10 @@ static void sched_force_init_mm(void) >>> > >>> > if (mm != &init_mm) { >>> > mmgrab_lazy_tlb(&init_mm); >>> > - local_irq_disable(); >>> > + hard_local_irq_disable(); >>> > current->active_mm = &init_mm; >>> > switch_mm_irqs_off(mm, &init_mm, current); >>> > - local_irq_enable(); >>> > + hard_local_irq_enable(); >>> > finish_arch_post_lock_switch(); >>> > mmdrop_lazy_tlb(mm); >>> > } >>> > >>> > Florian >>> >>> Correct. However, since this is an inband call, we want to keep the >>> virtual interrupt state in line with the expectations of the kernel, so >>> we need to keep the original local_irq* calls in place. Dovetail >>> provides a pair of calls namely (un)protect_inband_mm() which is >>> dedicated to guarding the mm switch code from hardware interrupts, the >>> general logic is visible from switch_mm() in arch/x86/mm/tlb.c. Using >>> these calls in this particular case serves as code annotation as well. >> >> This survived testing as well: >> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c >> index 50b082b41e7d..8bcfb8e7046c 100644 >> --- a/kernel/sched/core.c >> +++ b/kernel/sched/core.c >> @@ -8029,12 +8029,15 @@ void sched_setnuma(struct task_struct *p, int >> nid) >> static void sched_force_init_mm(void) >> { >> struct mm_struct *mm = current->active_mm; >> + long flags; >> >> if (mm != &init_mm) { >> mmgrab_lazy_tlb(&init_mm); >> local_irq_disable(); >> + protect_inband_mm(flags); >> current->active_mm = &init_mm; >> switch_mm_irqs_off(mm, &init_mm, current); >> + unprotect_inband_mm(flags); >> local_irq_enable(); >> finish_arch_post_lock_switch(); >> mmdrop_lazy_tlb(mm); >> >> That should probably be squashed into >> 8646b1d5622f ("x86: dovetail: enable alternate scheduling") >> for the 6.14 branch (only). >> >> Agreed? >> > > Ack. Nope, this is wrong. This should rather go to the generic Dovetail bits in: f185b0770c116 dovetail: add core support -- Philippe.