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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5EC40C32771 for ; Wed, 21 Sep 2022 04:40:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id ADF166B0072; Wed, 21 Sep 2022 00:40:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A8D8C6B0073; Wed, 21 Sep 2022 00:40:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 97C31940007; Wed, 21 Sep 2022 00:40:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 863286B0072 for ; Wed, 21 Sep 2022 00:40:18 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 4A551A068E for ; Wed, 21 Sep 2022 04:40:18 +0000 (UTC) X-FDA: 79934840916.16.FB0568C Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf22.hostedemail.com (Postfix) with ESMTP id 66D0DC000D for ; Wed, 21 Sep 2022 04:40:17 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id 0B7F8CE1C1B; Wed, 21 Sep 2022 04:40:13 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 51E05C433C1; Wed, 21 Sep 2022 04:40:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1663735211; bh=vGuyQI+FIpieTlo0FPtCZO4Etx2WXiRfDhLbQSJCdRk=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=TRhzMDTurf2g0MLA+jsdQR2hU5WuzVsoYjoZTTP003TS3yZxFUrtflewh9u4HwOC1 0E5KMpp5cf71jk92IFfgM1Gf2dpuvCcGV73jFGwukMWpbLcjDhpl/kZtqdaz1oWpz/ akWC5JVgA+owxMuBDS1eDcK6B8X1QLiK0IIZT7NPfj7mR9r3+hwRJqP93pS+O/0w2a o+nuzEgu5EQY/HgzkEUJGMXu80ybb3Byhk0MOZnWBHtehZDjTnGjsa1ae1vSXTWruo bmW8IGbRbS0wVZous5A6bRKMXVjEEX7G2ObE0qmDBsmpteiMkMcdL4kRarV397NgtC oIvK7EppA86OA== From: Kalle Valo To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-doc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, Linus Torvalds , Andrew Morton , Ingo Molnar , David Laight , Jonathan Corbet , Andy Whitcroft , Joe Perches , Dwaipayan Ray , Lukas Bulwahn , Baoquan He , Vivek Goyal , Dave Young , Jani Nikula , Michael Ellerman , Nicholas Piggin , Christophe Leroy Subject: Re: [PATCH v1 1/3] coding-style.rst: document BUG() and WARN() rules ("do not crash the kernel") References: <20220920122302.99195-1-david@redhat.com> <20220920122302.99195-2-david@redhat.com> Date: Wed, 21 Sep 2022 07:40:00 +0300 In-Reply-To: <20220920122302.99195-2-david@redhat.com> (David Hildenbrand's message of "Tue, 20 Sep 2022 14:23:00 +0200") Message-ID: <87pmfp8hnj.fsf@kernel.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1663735217; a=rsa-sha256; cv=none; b=N2ltE1fpVH9owGp1PjAy2hFEOtbMYKgNiufxFqb8L6dXRWNe7lvvhvz4+26MYLEpgEfUip XBYSeZ64quhxBbg4FrShJtX+Ne5m+RZBCojVPwztKW/i/7PmUXC5Pks0BMIZlMOTm5N1kU 24M9Jzc/j+tbk8MAU9/Shftsh1fmlD0= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=TRhzMDTu; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf22.hostedemail.com: domain of kvalo@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=kvalo@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1663735217; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Tw5lZy7b7BOiN5YhBnwE4uran1oklA/MoDO475jdWYQ=; b=Lyupczf6Fm85PwxD3kfwh04XbfvifBtI4N8vlxzX6J2YtK0ky7693WYboKAh6Nib8U7fBZ Hl4NUJBhUXi+rjQzUv21Kur7iqMmwtKwKNemDiWSYwDMgwpJjpu3R0Uyjvr8jfVq4Lb5l3 V8eO2h73AGn94FtHd6qCp9yHN3wKC60= Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=TRhzMDTu; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf22.hostedemail.com: domain of kvalo@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=kvalo@kernel.org X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: 6ie6rr81rw57amgaz3irqb57yk8c7oxw X-Rspamd-Queue-Id: 66D0DC000D X-HE-Tag: 1663735217-189614 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: David Hildenbrand writes: > Linus notes [1] that the introduction of new code that uses VM_BUG_ON() > is just as bad as BUG_ON(), because it will crash the kernel on > distributions that enable CONFIG_DEBUG_VM (like Fedora): > > VM_BUG_ON() has the exact same semantics as BUG_ON. It is literally > no different, the only difference is "we can make the code smaller > because these are less important". [2] > > This resulted in a more generic discussion about usage of BUG() and > friends. While there might be corner cases that still deserve a BUG_ON(), > most BUG_ON() cases should simply use WARN_ON_ONCE() and implement a > recovery path if reasonable: > > The only possible case where BUG_ON can validly be used is "I have > some fundamental data corruption and cannot possibly return an > error". [2] > > As a very good approximation is the general rule: > > "absolutely no new BUG_ON() calls _ever_" [2] > > ... not even if something really shouldn't ever happen and is merely for > documenting that an invariant always has to hold. However, there are sill > exceptions where BUG_ON() may be used: > > If you have a "this is major internal corruption, there's no way we can > continue", then BUG_ON() is appropriate. [3] > > There is only one good BUG_ON(): > > Now, that said, there is one very valid sub-form of BUG_ON(): > BUILD_BUG_ON() is absolutely 100% fine. [2] > > While WARN will also crash the machine with panic_on_warn set, that's > exactly to be expected: > > So we have two very different cases: the "virtual machine with good > logging where a dead machine is fine" - use 'panic_on_warn'. And > the actual real hardware with real drivers, running real loads by > users. [4] > > The basic idea is that warnings will similarly get reported by users > and be found during testing. However, in contrast to a BUG(), there is a > way to actually influence the expected behavior (e.g., panic_on_warn) > and to eventually keep the machine alive to extract some debug info. > > Ingo notes that not all WARN_ON_ONCE cases need recovery. If we don't ever > expect this code to trigger in any case, recovery code is not really > helpful. > > I'd prefer to keep all these warnings 'simple' - i.e. no attempted > recovery & control flow, unless we ever expect these to trigger. > [5] > > There have been different rules floating around that were never properly > documented. Let's try to clarify. > > [1] https://lkml.kernel.org/r/CAHk-=wiEAH+ojSpAgx_Ep=NKPWHU8AdO3V56BXcCsU97oYJ1EA@mail.gmail.com > [2] https://lore.kernel.org/r/CAHk-=wg40EAZofO16Eviaj7mfqDhZ2gVEbvfsMf6gYzspRjYvw@mail.gmail.com > [2] https://lkml.kernel.org/r/CAHk-=wit-DmhMfQErY29JSPjFgebx_Ld+pnerc4J2Ag990WwAA@mail.gmail.com > [4] https://lore.kernel.org/r/CAHk-=wgF7K2gSSpy=m_=K3Nov4zaceUX9puQf1TjkTJLA2XC_g@mail.gmail.com > [5] https://lore.kernel.org/r/YwIW+mVeZoTOxn%2F4@gmail.com > > Signed-off-by: David Hildenbrand [...] > +Use WARN_ON_ONCE() rather than WARN() or WARN_ON() > +************************************************** > + > +WARN_ON_ONCE() is generally preferred over WARN() or WARN_ON(), because it > +is common for a given warning condition, if it occurs at all, to occur > +multiple times. This can fill up and wrap the kernel log, and can even slow > +the system enough that the excessive logging turns into its own, additional > +problem. FWIW I have had cases where WARN() messages caused a reboot, maybe mention that here? In my case the logging was so excessive that the watchdog wasn't updated and in the end the device was forcefully rebooted. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches