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=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 058DBC433E0 for ; Mon, 1 Feb 2021 21:39:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B727D64ED0 for ; Mon, 1 Feb 2021 21:39:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232448AbhBAVjQ (ORCPT ); Mon, 1 Feb 2021 16:39:16 -0500 Received: from outgoing-auth-1.mit.edu ([18.9.28.11]:58674 "EHLO outgoing.mit.edu" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S229608AbhBAVjO (ORCPT ); Mon, 1 Feb 2021 16:39:14 -0500 Received: from cwcc.thunk.org (pool-72-74-133-215.bstnma.fios.verizon.net [72.74.133.215]) (authenticated bits=0) (User authenticated as tytso@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 111LcKLj027550 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 1 Feb 2021 16:38:20 -0500 Received: by cwcc.thunk.org (Postfix, from userid 15806) id F0DAD15C39E2; Mon, 1 Feb 2021 16:38:19 -0500 (EST) Date: Mon, 1 Feb 2021 16:38:19 -0500 From: "Theodore Ts'o" To: Nick Desaulniers Cc: Vinicius Tinti , Christoph Hellwig , Andreas Dilger , Nathan Chancellor , Ext4 Developers List , LKML , clang-built-linux Subject: Re: [PATCH v2] ext4: Enable code path when DX_DEBUG is set Message-ID: References: <20210201003125.90257-1-viniciustinti@gmail.com> <20210201124924.GA3284018@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 01, 2021 at 01:16:19PM -0800, Nick Desaulniers wrote: > I agree; Vinicius, my recommendation for -Wunreachable-* with Clang > was to see whether dead code identified by this more aggressive > diagnostic (than -Wunused-function) was to ask maintainers whether > code identified by it was intentionally dead and if they would > consider removing it. If they say "no," that's fine, and doesn't need > to be pushed. It's not clear to maintainers that: > 1. this warning is not on by default > 2. we're not looking to pursue turning this on by default > > If maintainers want to keep the dead code, that's fine, let them and > move on to the next instance to see if that's interesting (or not). It should be noted that in Documenting/process/coding-style.rst, there is an expicit recommendation to code in a way that will result in dead code warnings: Within code, where possible, use the IS_ENABLED macro to convert a Kconfig symbol into a C boolean expression, and use it in a normal C conditional: .. code-block:: c if (IS_ENABLED(CONFIG_SOMETHING)) { ... } The compiler will constant-fold the conditional away, and include or exclude the block of code just as with an #ifdef, so this will not add any runtime overhead. However, this approach still allows the C compiler to see the code inside the block, and check it for correctness (syntax, types, symbol references, etc). Thus, you still have to use an #ifdef if the code inside the block references symbols that will not exist if the condition is not met. So our process documentation *explicitly* recommends against using #ifdef CONFIG_XXX ... #endif, and instead use something that will -Wunreachable-code-aggressive to cause the compiler to complain. Hence, this is not a warning that we will *ever* be able to enable unconditionally --- so why work hard to remove such warnings from the code? If the goal is to see if we can detect real bugs using this technique, well and good. If the data shows that this warning actually is useful in finding bugs, then manybe we can figure out a way that we can explicitly hint to the compiler that in *this* case, the maintainer actually knew what they were doing. But if an examination of the warnings shows that -Wunreachable-code-aggressive isn't actually finding any real bugs, then perhaps it's not worth it. Cheers, - Ted