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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C89C3C433EF for ; Tue, 4 Jan 2022 17:26:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235426AbiADR0F (ORCPT ); Tue, 4 Jan 2022 12:26:05 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39310 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234749AbiADR0D (ORCPT ); Tue, 4 Jan 2022 12:26:03 -0500 Received: from mail-lf1-x12e.google.com (mail-lf1-x12e.google.com [IPv6:2a00:1450:4864:20::12e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 332CFC061761 for ; Tue, 4 Jan 2022 09:26:03 -0800 (PST) Received: by mail-lf1-x12e.google.com with SMTP id x7so83285337lfu.8 for ; Tue, 04 Jan 2022 09:26:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=WI7lB1h5wzR1I5amS3/J/XjMPBvz/P5sBCVcxmVEb6g=; b=OMiXrp20A1P+UWBlFyhfOkxCnq+sBHc6yZlhZnVZ7OVEOwVMMYK1ivGWdCIWQTWRip SPK2nhJjpXO5iVLF37PSpqBhEHrvkSfDM2pxQvdU/WNrLW365k+hFye1+D6eU/c/IXXo LOdMAILQQRfWDCR08GV8mnK7FmQL9geS33pEfWPecFozzwtNBooirRkMKCSaaGHOl51m riB0Eka+KFABS3v+nmXQHX+/P9eIJFTpfOZdTs304aLI1ivvjExWMVIhc235t9SdQxin PVgOih4W3xqoqysFob1zl+u7bsCqAaI1G3qswq6r/InvYOBigCwA2COhE+TUwaojfOAq S9jA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=WI7lB1h5wzR1I5amS3/J/XjMPBvz/P5sBCVcxmVEb6g=; b=PJb50RLGNal4maTgJGfShIxnsYHmQR2m/oYNlcTbB7XGlqFG/tKrl++IHoog9HMXsA 47TfIm6zS2KItqOV90Hi/qYULD4Imr2Zvy8CRbr1M7pXZNg8sndx1FtoGdkPHceponoX O2GiSduGwsXabhk0YfhEyz7r+PovWMxS7vr6aFGwqHj3VTG1B3ZoNmBqDFFT/PgeUn43 ihYRuk0SQqdw58qMtaYOtxQjAqMi7JcKb+0yFzTFRsh2ivn1UE5yWKopqX5pOgtIHOlc k043/suvBPBLx1Yo/tgge4TOeeQGte5CnrcuBGlv5EoWRYMybZyBbJcyWkHZf51PlEUf l3NA== X-Gm-Message-State: AOAM532qCVqyct1b4GKBOrIuGDFKuoKLC94TZUQZDe8VAHBErKjJ177L pKaOLl9QWMdZXcLynavwjDn7X3sdP1eIpUxzRgudcg== X-Google-Smtp-Source: ABdhPJw8CT9FUQyCSpw151cYtBBTzAW6DKyeI4S1JIsi2Z3jQSPb/+4uOHm3HO73slTwxuU1Z1CZppFcgoHYgkIhIRk= X-Received: by 2002:a05:6512:32c5:: with SMTP id f5mr41463073lfg.550.1641317160619; Tue, 04 Jan 2022 09:26:00 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Nick Desaulniers Date: Tue, 4 Jan 2022 09:25:48 -0800 Message-ID: Subject: Re: [PATCH 0000/2297] [ANNOUNCE, RFC] "Fast Kernel Headers" Tree -v1: Eliminate the Linux kernel's "Dependency Hell" To: Ingo Molnar Cc: Nathan Chancellor , Linus Torvalds , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, Andrew Morton , Peter Zijlstra , Thomas Gleixner , Greg Kroah-Hartman , "David S. Miller" , Ard Biesheuvel , Josh Poimboeuf , Jonathan Corbet , Al Viro , llvm@lists.linux.dev, ashimida , Arnd Bergmann Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 4, 2022 at 2:47 AM Ingo Molnar wrote: > > > * Nathan Chancellor wrote: > > > Hi Ingo, > > > > On Sun, Jan 02, 2022 at 10:57:35PM +0100, Ingo Molnar wrote: > > I took the series for a spin with clang and GCC on arm64 and x86_64 and > > I found a few warnings/errors. > > Thank you! > > > 1. Position of certain attributes > > > > In some commits, you move the cacheline_aligned attributes from after > > the closing brace on structures to before the struct keyword, which > > causes clang to warn (and error with CONFIG_WERROR): > > > > In file included from arch/arm64/kernel/asm-offsets.c:9: > > In file included from arch/arm64/kernel/../../../kernel/sched/per_task_= area_struct.h:33: > > In file included from ./include/linux/perf_event_api.h:17: > > In file included from ./include/linux/perf_event_types.h:41: > > In file included from ./include/linux/ftrace.h:18: > > In file included from ./arch/arm64/include/asm/ftrace.h:53: > > In file included from ./include/linux/compat.h:11: > > ./include/linux/fs_types.h:997:1: error: attribute '__aligned__' is ign= ored, place it after "struct" to apply attribute to type declaration [-Werr= or,-Wignored-attributes] > > ____cacheline_aligned > > ^ > > ./include/linux/cache.h:41:46: note: expanded from macro '____cacheline= _aligned' > > #define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTE= S))) > > Yeah, so this is a *really* stupid warning from Clang. > > Putting the attribute after 'struct' risks the hard to track down bugs wh= en > a inclusion is missing, which scenario I pointed out in > this commit: > > headers/deps: dcache: Move the ____cacheline_aligned attribute to the= head of the definition > > When changing I removed the h= eader, > which caused a couple of hundred of mysterious, somewhat obscure link= time errors: > > ld: net/sctp/tsnmap.o:(.bss+0x0): multiple definition of `____cache= line_aligned_in_smp'; init/do_mounts_rd.o:(.bss+0x0): first defined here > ld: net/sctp/tsnmap.o:(.bss+0x40): multiple definition of `____cach= eline_aligned'; init/do_mounts_rd.o:(.bss+0x40): first defined here > ld: net/sctp/debug.o:(.bss+0x0): multiple definition of `____cachel= ine_aligned_in_smp'; init/do_mounts_rd.o:(.bss+0x0): first defined here > ld: net/sctp/debug.o:(.bss+0x40): multiple definition of `____cache= line_aligned'; init/do_mounts_rd.o:(.bss+0x40): first defined here > > After a bit of head-scratching, what happened is that 'struct dentry_= operations' > has the ____cacheline_aligned attribute at the tail of the type defin= ition - > which turned into a local variable definition when wa= s not > included - which includes into indirectly. > > There were no compile time errors, only link time errors. > > Move the attribute to the head of the definition, in which case > a missing inclusion creates an immediate build failur= e: > > In file included from ./include/linux/fs.h:9, > from ./include/linux/fsverity.h:14, > from fs/verity/fsverity_private.h:18, > from fs/verity/read_metadata.c:8: > ./include/linux/dcache.h:132:22: error: expected =E2=80=98;=E2=80= =99 before =E2=80=98struct=E2=80=99 > 132 | ____cacheline_aligned > | ^ > | ; > 133 | struct dentry_operations { > | ~~~~~~ > > No change in functionality. > > Signed-off-by: Ingo Molnar > > Can this Clang warning be disabled? Clang is warning that the attribute will be ignored because of that positioning. If you disable the warning, code will probably stop working as intended. This warning has at least been helping us make the kernel coding style more consistent. This made me think of d5b421fe02827 ("docs: Explain the desired position of function attributes"), where we adding some text to Documentation/process/coding-style.rst about the positioning of __attribute__'s in function signatures, but I guess this case is data. We probably should add something to the coding style about attributes on data, too. The C standards body is also working on standardizing attributes; at the least I expect some of these things to be ironed out more soon. > > > 2. Error with CONFIG_SHADOW_CALL_STACK > > So this feature depends on Clang: > > # Supported by clang >=3D 7.0 > config CC_HAVE_SHADOW_CALL_STACK > def_bool $(cc-option, -fsanitize=3Dshadow-call-stack -ffixed-x18= ) > > No way to activate it under my GCC cross-build toolchain, right? > > But ... I hacked the build mode on with GCC using this patch: Dan Li is working on a GCC patch. If you're up for building GCC from source= : https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586204.html -- This is a really cool series Ingo. I'm sure Arnd has seen it by now, but Arnd has been thinking about this area a lot, too. I haven't but I have played with running "include what you use" on the kernel sources; Kconfig being the biggest impediment to that approach. To me, I'm most nervous about "backsliding;" let's say this work lands, at some point probably years in the future, I assume without any form of automation that we might find ourselves at a similar point of header dependencies getting all tangled again. What are your thoughts on where/how/what we could automate to try to help developers in the future keep their header dependencies simpler? (Sorry if this was already answered in the cover letter) It would be really useful if you were planning a talk at something like plumbers how you go about making these changes. I really hope once others understand your workflow that we might help with some form of automation. Nice work! -- Thanks, ~Nick Desaulniers