From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) (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 E092A2C80 for ; Tue, 4 Jan 2022 17:26:02 +0000 (UTC) Received: by mail-lf1-f52.google.com with SMTP id h2so72661703lfv.9 for ; Tue, 04 Jan 2022 09:26:02 -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=eACqMGuiiu9u8VjN9/WyfyLhgQXIrt4j/JOgoFLa+Xa7vvsFkBSijDtg46D2whCjgP Oh+50uTXrjCfLfjm6vH1oEH/Ks/tjaW7FfXjyq4CNnRZ/Mv0IkMXgR62DG9IRQHnWD+P wwiEN9HfIX6JlWLWQjwY45qN8z3Sfe8XisTKfmp9AtLkY7BrwewcsTCSRc7PayseH16P NvJv4xLKlhAyawl8m9Ukh4ipp9gSL81rbngNEd4VdFycN8mrJKmMU2XERDMVwhmH4+Hs PYoKjwzG3776B5KjoHCDMXKZRVgvzWvyDYSCQ4Q5MBFuPefqERTffSXBmnFlTGc0bY1D c4ow== X-Gm-Message-State: AOAM5339XzRkSHeXExeL+XM2K8vbJhg/JZwMPDxMPgxYXsOBMgRaYYoK mAJ9IKV+opNfsZxnn7BqdFcyzmv5Q0O4CXtm41otgg== 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) Precedence: bulk X-Mailing-List: llvm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: 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 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