From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 84B3E1991CB for ; Wed, 8 Jul 2026 09:52:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783504362; cv=none; b=YpuEAakY9xFI7sHXP3qv+bWcreP3+QegnK6nhmQNCErTAYSH1cbrh+hBSJ3JnS8J4j+uu27iVXCbZ10lnPTT6OtuqpVA3bVV5NFtx0DLAtkMBXCPkox/j/N704cNxW+Z8tF4Nw4TFJTnYxidy+JKWeBGpTEGwju0yQNH9CaxCFg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783504362; c=relaxed/simple; bh=JBsw7YlYuZJB3VaLMi3Hk89MfuFYnEr6R9yuPivCnQQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JqgCToJiKwnAQLhgTnVemzU3oqQm9oh4Jhs2jNZiLMmmj4pnZbeJYlQvvrGKe7yYhAxIQCUjhJUvEBqNaueRU0LMrJ5oGtBNlpBWGrjSf/soA09abBZXEPw6aJZAJrKlyo9NIbgLZa5WBpErdfeCFVYM0Tick/eXrRpQHcj9BXI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=PkxYh3Gh; arc=none smtp.client-ip=209.85.221.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="PkxYh3Gh" Received: by mail-wr1-f50.google.com with SMTP id ffacd0b85a97d-472326ca506so311385f8f.2 for ; Wed, 08 Jul 2026 02:52:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1783504359; x=1784109159; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=NBi04AiFTSvUFLtcC7vl2YbJQcl1LSsfX/jfNqEazlw=; b=PkxYh3GhSnPmUZOF/LWY63Dxn1PaaSAFJ/IvE+Wn24usrzSeqv2g5bNYBRsUUMsxoX tC6/pjejgzFqzlDATmjd2dSJR892h4Ecoi1//hP6qcVI9nvfoCmdXVPJVTsCfGxdfJA4 PhrHpSagUGp6u260hRG78nhySDjQb3n334PsZkNHFPyrPC6NTyOuavEtYXqelfE14D8e sQ9T4VB6XqdfFEDQTR9z5I9G/uXvt+C/6fkY09pm1yYIUVtnGFtmLdD8PCrN8D9C6KTY acXInD+ts3SDjPbi2L1R0qX06s2r9gHmI0gOmXvaDQACRiQyrykUhXYYolku8wXW5srh i+xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783504359; x=1784109159; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=NBi04AiFTSvUFLtcC7vl2YbJQcl1LSsfX/jfNqEazlw=; b=DR50mcbnczHGsL4cwqqWoAAMkgPUU4dTqXBxzYLICpBUwYue14OCQkxjUuziMjq54t 4TaslkXhJkqrrM4/14g8dVWl91/Wf62vW6FCkkuOCdcWzF2Q/TPw0rm9UOHjO2VcVzhs YxcRIbbAkory8mm1RhmC52iGDAwQBhFnVkzktytluViL2K6NS8GlTxRpdVuO5MtfBcl1 vH3SVs3XsjNyzhPKnmM2E6gMFlkCA3uniD6oq/wBcXUE1xNHBbZ4yvsCoUjAGch6vd0a JSRkGr/FF5nEPyNyRYoTNRDJHt2TlDMKdD9QKl5EeHIbWpo9njnTcw+BFc/tNew1UgpI pTUw== X-Forwarded-Encrypted: i=1; AHgh+Rp5+MqRupFc8qm4ncJwUgTzSun/qJJ39+CwzKUhO7jt0W2xO27AvErewlaL6r05FwaoacvE1LWwIsUARkc=@vger.kernel.org X-Gm-Message-State: AOJu0YxfuKHcORBNVg6cEr4hkIWG1mBww9neC62JOV0K5d1ixeiFPWzM V9+EqFn00CEzN5gsfjBaJ8O4VP5M9TGJJwWNJ8/5pH1i1av+UwoQ50wHhMpeCkXhohNA7/ELAVX 4rn6Hg/E= X-Gm-Gg: AfdE7clGX9re5Is99E26vlIMXu574bYmrtgc+vi37aHGqVpXn5He/NS4S4hKLrOivin N7/aklTc7ya7Cx9NaBJ6/VJySWzDjvvukIKgGd07lHZ+amsH6iAYkwecmaY2KvhOHp4cilABpBK cQb/6U6sOzpXGD0t8zdW94GzBpzODAFQt08FFEadWz9y3N8TjxzrvxtGfoI3qaBnJS3qC+HihC0 fIaW7LYs7cc90cEoDWPsluuoSCqOOWuwCi+G44tmj0V8g1P3KxUm9tqo5+an/lwhrQ8YVQamjTU HQ8Pf162I4XynPwJk0QSbLYDSf/OZxb2eqH/VlO7R4JPyL043QdcyDmOlAGOsbPj5GPiW4C0VcE oZ4Zulp7GNo0OD8H8qxl26PZVsR+n/qA6kuvTwGfMsqT3Ra+3/eQiSa0rr3t53RGkF4A+IxveX3 S/foIV1nq+HDnt7DI= X-Received: by 2002:a5d:5d0d:0:b0:478:2698:72f4 with SMTP id ffacd0b85a97d-47df07761aamr1880410f8f.45.1783504358715; Wed, 08 Jul 2026 02:52:38 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47a9e4d6da9sm40644839f8f.12.2026.07.08.02.52.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Jul 2026 02:52:38 -0700 (PDT) Date: Wed, 8 Jul 2026 11:52:36 +0200 From: Petr Mladek To: Bradley Morgan Cc: akpm@linux-foundation.org, feng.tang@linux.alibaba.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] panic: stop CPUs that lose the panic_redirect_cpu race Message-ID: References: <20260707183253.9793-1-include@grrlz.net> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260707183253.9793-1-include@grrlz.net> On Tue 2026-07-07 18:32:53, Bradley Morgan wrote: > Loser of the redirect cmpxchg must stop, not fall through to > panic_try_start(). This described the change to "return true". It does not explain why it was updated to "old_cpu != this_cpu" in v2. Also it looks like a lazy description. It does not make much sense without looking into the patch. People need to check: What is "redirect cmpxchg"? What does panic_try_start() do? I think that we could do much better job. See how Sashiko tried to describe the scenario... > This issue was found by sashiko [1]. > > [1] https://sashiko.dev/#/patchset/20260705164123.18746-1-include%40grrlz.net Link to Sashiko provides more information but it is not guaranteed to stay. The important information should be added to the commit message. Also it would be good add link to the 2nd report which affected the fix as well. Finally, it would be better to use the tags Reported-by: and Closes: Well, I would keep this patch only for fixing the 1st problem (race in cmpxchg panic_try_force_cpu(). I mean to use "return true". But I would improve the commit message, something like: The cmpxchg() in panic_try_force_cpu() makes sure that only one CPU tries to redirect panic() to the requested CPU. It is similar to the cmpxchg() in panic_try_start() which makes sure that only one CPU does the panic(). In both situations, only the winner of cmpxchg() should proceed further. Other CPUs should go offline. There is a bug in panic_try_force_cpu() because CPUs continue with vpanic() when the cmpxchg() fails. As a result, they might win in panic_try_start() instead of the requested CPU. Reported-by: Reported-by: Sashiko Closes: https://sashiko.dev/#/patchset/20260705164123.18746-1-include%40grrlz.net > Signed-off-by: Bradley Morgan > --- > kernel/panic.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > Changes since v1: a nested panic on the CPU that already holds > panic_redirect_cpu would fail its own cmpxchg and, with v1's > unconditional return true, halt itself and abandon the panic. > Check old_cpu == this_cpu so the winner of a nested reentry > falls through to panic_try_start() instead. This is important and should have been mentioned in the commit message. > diff --git a/kernel/panic.c b/kernel/panic.c > index 03f1eef07b17..e46c37b39c40 100644 > --- a/kernel/panic.c > +++ b/kernel/panic.c > @@ -400,12 +400,9 @@ static bool panic_try_force_cpu(const char *fmt, va_list args) > if (panic_in_progress()) > return false; > > - /* > - * Only one CPU can do the redirect. Use atomic cmpxchg to ensure > - * we don't race with another CPU also trying to redirect. > - */ > + /* Which CPU won the race? */ I do not like the new comment. What exactly is the "the race"? And it does not explain the "old_cpu != this_cpu" check. I would suggest to write something like, for "return true" variant: /* * Only one CPU can do the redirection. Others should go * offline. */ And update it in the 2nd patch to: /* * Only one CPU can do the redirection. Others should go * offline. Continue with panic() when ending recursively here. */ > if (!atomic_try_cmpxchg(&panic_redirect_cpu, &old_cpu, this_cpu)) > - return false; > + return old_cpu != this_cpu; Best Regards, Petr