Re: [PATCH v2 0/2] get_maintainer: add patch-only keyword matching
On Wed, Sep 27, 2023 at 11:09 PM Joe Perches wrote: > > On Thu, 2023-09-28 at 14:31 +0900, Justin Stitt wrote: > > On Thu, Sep 28, 2023 at 2:01 PM Joe Perches wrote: > > > > > > On Thu, 2023-09-28 at 04:23 +, Justin Stitt wrote: > > > > Changes in v2: > > > > - remove formatting pass (thanks Joe) (but seriously the formatting is > > > > bad, is there opportunity to get a formatting pass in here at some > > > > point?) > > > > > > Why? What is it that makes you believe the formatting is bad? > > > > > > > Investigating further, it looked especially bad in my editor. There is > > a mixture of > > tabs and spaces and my vim tabstop is set to 4 for pl files. Setting this to > > 8 is a whole lot better. But I still see some weird spacing > > > > Yes, it's a bit odd indentation. > It's emacs default perl format. > 4 space indent with 8 space tabs, maximal tab fill. > Oh! What?! That's the most surprising convention I've ever heard of (after the GNU C coding style). Yet another thing to hold against perl I guess. :P I have my editor setup to highlight tabs vs spaces via visual cues, so that I don't mess up kernel coding style. (`git clang-format HEAD~` after a commit helps). scripts/get_maintainer.pl has some serious inconsistencies to the point where I'm not sure what it should or was meant to be. Now that you mention it, I see it, and it does seem consistent in that regard. Justin, is your formatter configurable to match that convention? Maybe it's still useful, as long as you configure it to stick to the pre-existing convention. -- Thanks, ~Nick Desaulniers
Re: [PATCH 0/3] get_maintainer: add patch-only keyword matching
On Tue, Sep 26, 2023 at 8:19 PM Justin Stitt wrote: > > This series aims to add "D:" which behaves exactly the same as "K:" but > works only on patch files. > > The goal of this is to reduce noise when folks use get_maintainer on > tree files as opposed to patches. This use case should be steered away > from [1] but "D:" should help maintainers reduce noise in their inboxes > regardless, especially when matching omnipresent keywords like [2]. In > the event of [2] Kees would be to/cc'd from folks running get_maintainer > on _any_ file containing "__counted_by". The number of these files is > rising and I fear for his inbox as his goal, as I understand it, is to > simply monitor the introduction of new __counted_by annotations to > ensure accurate semantics. Something like this (whether this series or a different approach) would be helpful to me as well; we use K: to get cc'ed on patches mentioning clang or llvm, but our ML also then ends up getting cc'ed on every follow up patch to most files. This is causing excessive posts on our ML. As a result, it's a struggle to get folks to cc themselves to the ML, which puts the code review burden on fewer people. Whether it's a new D: or refinement to the behavior of K:, I applaud the effort. Hopefully we can find an approach that works for everyone. And may God have mercy on your soul for having to touch that much perl. :-P > > See [3/3] for an illustrative example. > > This series also includes a formatting pass over get_maintainer because > I personally found it difficult to parse with the human eye. > > [1]: https://lore.kernel.org/all/20230726151515.1650519-1-k...@kernel.org/ > [2]: https://lore.kernel.org/all/20230925172037.work.853-k...@kernel.org/ > > Signed-off-by: Justin Stitt > --- > Justin Stitt (3): > MAINTAINERS: add documentation for D: > get_maintainer: run perltidy > get_maintainer: add patch-only pattern matching type > > MAINTAINERS |3 + > scripts/get_maintainer.pl | 3334 > +++-- > 2 files changed, 1718 insertions(+), 1619 deletions(-) > --- > base-commit: 6465e260f48790807eef06b583b38ca9789b6072 > change-id: 20230926-get_maintainer_add_d-07424a814e72 > > Best regards, > -- > Justin Stitt > -- Thanks, ~Nick Desaulniers
Re: [PATCH 01/15] objtool: Find a destination for jumps beyond the section end
On Tue, Apr 20, 2021 at 3:57 PM Josh Poimboeuf wrote: > > On Tue, Apr 20, 2021 at 01:25:43PM -0700, Sami Tolvanen wrote: > > On Tue, Apr 20, 2021 at 11:14 AM Josh Poimboeuf wrote: > > > > > > On Fri, Apr 16, 2021 at 01:38:30PM -0700, Sami Tolvanen wrote: > > > > With -ffunction-sections, Clang can generate a jump beyond the end of > > > > a section when the section ends in an unreachable instruction. > > > > > > Why? Can you show an example? > > > > Here's the warning I'm seeing when building allyesconfig + CFI: > > > > vmlinux.o: warning: objtool: > > rockchip_spi_transfer_one.f088382d97b74759d70e27e891fe8f1c()+0x149: > > can't find jump dest instruction at > > .text.rockchip_spi_transfer_one.f088382d97b74759d70e27e891fe8f1c+0x7dc > > > > $ objdump -d -r -j > > .text.rockchip_spi_transfer_one.f088382d97b74759d70e27e891fe8f1c > > vmlinux.o > > > > : > > ... > > 149: 0f 85 8d 06 00 00 jne7dc <.compoundliteral.4> > > ... > > 7d7: e8 00 00 00 00 callq 7dc <.compoundliteral.4> > > 7d8: R_X86_64_PLT32 __stack_chk_fail-0x4 > > Instead of silencing the warning by faking the jump destination, I'd > rather improve the warning to something like > > "warning: rockchip_spi_transfer_one() falls through to the next function" > > which is what we normally do in this type of situation. > > It may be caused by UB, or a compiler bug, but either way we should > figure out the root cause. We probably want to creduce or cvise this. IIRC we still have outstanding issues with switch statements with user-annotated unreachable branches not getting eliminated. -- Thanks, ~Nick Desaulniers
[PATCH v3] arm64: vdso32: drop -no-integrated-as flag
Clang can assemble these files just fine; this is a relic from the top level Makefile conditionally adding this. We no longer need --prefix, --gcc-toolchain, or -Qunused-arguments flags either with this change, so remove those too. To test building: $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 LLVM_IAS=1 \ defconfig arch/arm64/kernel/vdso32/ Suggested-by: Nathan Chancellor Signed-off-by: Nick Desaulniers Reviewed-by: Nathan Chancellor Reviewed-by: Vincenzo Frascino --- Changes V2 -> V3: * Pick up reviewed by tags. Changes V1 -> V2: * Remove --prefix, --gcc-toolchain, COMPAT_GCC_TOOLCHAIN, and COMPAT_GCC_TOOLCHAIN_DIR as per Nathan. * Credit Nathan with Suggested-by tag. * Remove -Qunused-arguments. * Update commit message. arch/arm64/kernel/vdso32/Makefile | 8 1 file changed, 8 deletions(-) diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 789ad420f16b..3dba0c4f8f42 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -10,15 +10,7 @@ include $(srctree)/lib/vdso/Makefile # Same as cc-*option, but using CC_COMPAT instead of CC ifeq ($(CONFIG_CC_IS_CLANG), y) -COMPAT_GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE_COMPAT)elfedit)) -COMPAT_GCC_TOOLCHAIN := $(realpath $(COMPAT_GCC_TOOLCHAIN_DIR)/..) - CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%)) -CC_COMPAT_CLANG_FLAGS += --prefix=$(COMPAT_GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE_COMPAT)) -CC_COMPAT_CLANG_FLAGS += -no-integrated-as -Qunused-arguments -ifneq ($(COMPAT_GCC_TOOLCHAIN),) -CC_COMPAT_CLANG_FLAGS += --gcc-toolchain=$(COMPAT_GCC_TOOLCHAIN) -endif CC_COMPAT ?= $(CC) CC_COMPAT += $(CC_COMPAT_CLANG_FLAGS) -- 2.31.1.368.gbe11c130af-goog
Re: [PATCH 00/13] [RFC] Rust support
On Fri, Apr 16, 2021 at 10:39 AM Willy Tarreau wrote: > > resources usage, I'm really not convinced at all it's suited for > low-level development. I understand the interest of the experiment > to help the language evolve into that direction, but I fear that > the kernel will soon be as bloated and insecure as a browser, and > that's really not to please me. Dunno, I don't think the introduction of Rust made Firefox _more_ insecure. https://wiki.mozilla.org/Oxidation#Within_Firefox I pray no executives ever see Dmitry Vyukov's 2019 Linux Plumbers Conf talk "Reflections on kernel quality, development process and testing." https://www.youtube.com/watch?v=iAfrrNdl2f4 or his 2018 Linux Security Summit talk "Syzbot and the Tale of Thousand Kernel Bugs" https://www.youtube.com/watch?v=qrBVXxZDVQY (and they're just fuzzing the syscall interface and USB devices. Imagine once folks can more easily craft malformed bluetooth and wifi packets.) I'd imagine the first term that comes to mind for them might be "liability." They are quite sensitive to these vulnerabilities with silly names, logos, and websites. There are many of us that believe an incremental approach of introducing a memory safe language to our existing infrastructure at the very least to attempt to improve the quality of drivers for those that choose to use such tools is a better approach. I think a lot of the current discussion picking nits in syntax, format of docs, ease of installation, or theoretical memory models for which no language (not even the one the kernel is implemented in) provides all rightly should still be added to a revised RFC under "Why not [Rust]?" but perhaps are severely overlooking the benefits. A tradeoff for sure though. Really, a key point is that a lot of common mistakes in C are compile time errors in Rust. I know no "true" kernel dev would make such mistakes in C, but is there nothing we can do to help our peers writing drivers? The point is to transfer cost from runtime to compile time to avoid costs at runtime; like all of the memory safety bugs which are costing our industry. Curiously recurring statistics: https://www.zdnet.com/article/microsoft-70-percent-of-all-security-bugs-are-memory-safety-issues/ "Microsoft security engineer Matt Miller said that over the last 12 years, around 70 percent of all Microsoft patches were fixes for memory safety bugs." https://www.chromium.org/Home/chromium-security/memory-safety "The Chromium project finds that around 70% of our serious security bugs are memory safety problems." https://security.googleblog.com/2021/01/data-driven-security-hardening-in.html (59% of Critical and High severity vulnerabilities fixed in Android Security Bulletins in 2019 are classified as "Memory," FWIW) https://hacks.mozilla.org/2019/02/rewriting-a-browser-component-in-rust/ "If we’d had a time machine and could have written this component in Rust from the start, 51 (73.9%) of these bugs would not have been possible." -- Thanks, ~Nick Desaulniers
Re: [PATCH 00/13] [RFC] Rust support
On Fri, Apr 16, 2021 at 11:47 AM Paul E. McKenney wrote: > > On Thu, Apr 15, 2021 at 11:04:37PM -0700, Nick Desaulniers wrote: > > On Thu, Apr 15, 2021 at 9:27 PM Boqun Feng wrote: > > > > > > But I think the Rust Community still wants to have a good memory model, > > > and they are open to any kind of suggestion and input. I think we (LKMM > > > people) should really get involved, because the recent discussion on > > > RISC-V's atomics shows that if we didn't people might get a "broken" > > > design because they thought C11 memory model is good enough: > > > > > > https://lore.kernel.org/lkml/YGyZPCxJYGOvqYZQ@boqun-archlinux/ > > > > > > And the benefits are mutual: a) Linux Kernel Memory Model (LKMM) is > > > defined by combining the requirements of developers and the behavior of > > > hardwares, it's pratical and can be a very good input for memory model > > > designing in Rust; b) Once Rust has a better memory model, the compiler > > > technologies whatever Rust compilers use to suppor the memory model can > > > be adopted to C compilers and we can get that part for free. > > > > Yes, I agree; I think that's a very good approach. Avoiding the ISO > > WG14 is interesting; at least the merits could be debated in the > > public and not behind closed doors. > > WG14 (C) and WG21 (C++) are at least somewhat open. Here are some of > the proposals a few of us have in flight: Wow, the working groups have been busy. Thank you Paul and Boqun (and anyone else on thread) for authoring many of the proposals listed below. Looks like I have a lot of reading to do to catch up. Have any of those been accepted yet, or led to amendments to either language's specs? Where's the best place to track that? My point has more to do with _participation_. Rust's RFC process is well documented (https://rust-lang.github.io/rfcs/introduction.html) and is done via github pull requests[0]. This is a much different process than drafts thrown over the wall. What hope do any kernel contributors have to participate in the ISO WGs, other than hoping their contributions to a draft foresee/address any concerns members of the committee might have? How do members of the ISO WG communicate with folks interested in the outcomes of their decisions? The two processes are quite different; one doesn't require "joining a national body" (which I assume involves some monetary transaction, if not for the individual participant, for their employer) for instance. (http://www.open-std.org/jtc1/sc22/wg14/www/contributing which links to https://www.iso.org/members.html; I wonder if we have kernel contributors in those grayed out countries?) It was always very ironic to me that the most important body of free software was subject to decisions made by ISO, for better or for worse. I would think Rust's RFC process would be more accessible to kernel developers, modulo the anti-github crowd, but with the Rust's community's values in inclusion I'm sure they'd be happy to accomodate folks for the RFC process without requiring github. I'm not sure ISO can be as flexible for non-members. Either way, I think Rust's RFC process is something worth adding to the list of benefits under the heading "Why Rust?" in this proposed RFC. > > P2055R0 A Relaxed Guide to memory_order_relaxed > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2055r0.pdf > P0124R7 Linux-Kernel Memory Model (vs. that of C/C++) > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p0124r7.html > P1726R4 Pointer lifetime-end zap > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1726r4.pdf > > https://docs.google.com/document/d/1MfagxTa6H0rTxtq9Oxyh4X53NzKqOt7y3hZBVzO_LMk/edit?usp=sharing > P1121R2 Hazard Pointers: Proposed Interface and Wording for Concurrency TS 2 > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1121r2.pdf > P1382R1 volatile_load and volatile_store > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1382r1.pdf > P1122R2 Proposed Wording for Concurrent Data Structures: Read-Copy-Update > (RCU) > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1122r2.pdf > > https://docs.google.com/document/d/1MfagxTa6H0rTxtq9Oxyh4X53NzKqOt7y3hZBVzO_LMk/edit?usp=sharing > P0190R4 Proposal for New memory order consume Definition > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0190r4.pdf > P0750R1 Consume > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0750r1.html Does wg14 not participate in these discussions? (Or, is there a lot of overlap between participants in both?) http://93.90.116.65/JTC1/SC22/WG14/www/docs/ seems like a list of proposals
Re: [PATCH 00/13] [RFC] Rust support
On Thu, Apr 15, 2021 at 9:27 PM Boqun Feng wrote: > > [Copy LKMM people, Josh, Nick and Wedson] > > On Thu, Apr 15, 2021 at 08:58:16PM +0200, Peter Zijlstra wrote: > > On Wed, Apr 14, 2021 at 08:45:51PM +0200, oj...@kernel.org wrote: > > > > > Rust is a systems programming language that brings several key > > > advantages over C in the context of the Linux kernel: > > > > > > - No undefined behavior in the safe subset (when unsafe code is > > > sound), including memory safety and the absence of data races. > > > > And yet I see not a single mention of the Rust Memory Model and how it > > aligns (or not) with the LKMM. The C11 memory model for example is a > > really poor fit for LKMM. > > > > I think Rust currently uses C11 memory model as per: > > https://doc.rust-lang.org/nomicon/atomics.html > > , also I guess another reason that they pick C11 memory model is because > LLVM has the support by default. > > But I think the Rust Community still wants to have a good memory model, > and they are open to any kind of suggestion and input. I think we (LKMM > people) should really get involved, because the recent discussion on > RISC-V's atomics shows that if we didn't people might get a "broken" > design because they thought C11 memory model is good enough: > > https://lore.kernel.org/lkml/YGyZPCxJYGOvqYZQ@boqun-archlinux/ > > And the benefits are mutual: a) Linux Kernel Memory Model (LKMM) is > defined by combining the requirements of developers and the behavior of > hardwares, it's pratical and can be a very good input for memory model > designing in Rust; b) Once Rust has a better memory model, the compiler > technologies whatever Rust compilers use to suppor the memory model can > be adopted to C compilers and we can get that part for free. Yes, I agree; I think that's a very good approach. Avoiding the ISO WG14 is interesting; at least the merits could be debated in the public and not behind closed doors. > > At least I personally is very intereted to help Rust on a complete and > pratical memory model ;-) > > Josh, I think it's good if we can connect to the people working on Rust > memoryg model, I think the right person is Ralf Jung and the right place > is https://github.com/rust-lang/unsafe-code-guidelines, but you > cerntainly know better than me ;-) Or maybe we can use Rust-for-Linux or > linux-toolchains list to discuss. > > [...] > > > - Boqun Feng is working hard on the different options for > > > threading abstractions and has reviewed most of the `sync` PRs. > > > > Boqun, I know you're familiar with LKMM, can you please talk about how > > Rust does things and how it interacts? > > As Wedson said in the other email, currently there is no code requiring > synchronization between C side and Rust side, so we are currently fine. > But in the longer term, we need to teach Rust memory model about the > "design patterns" used in Linux kernel for parallel programming. > > What I have been doing so far is reviewing patches which have memory > orderings in Rust-for-Linux project, try to make sure we don't include > memory ordering bugs for the beginning. > > Regards, > Boqun -- Thanks, ~Nick Desaulniers
Re: [PATCH 04/13] Kbuild: Rust support
On Wed, Apr 14, 2021 at 5:43 PM Miguel Ojeda wrote: > > On Thu, Apr 15, 2021 at 1:19 AM Nick Desaulniers > wrote: > > > > -Oz in clang typically generates larger kernel code than -Os; LLVM > > seems to aggressively emit libcalls even when the setup for a call > > would be larger than the inlined call itself. Is z smaller than s for > > the existing rust examples? > > I will check if the `s`/`z` flags have the exact same semantics as > they do in Clang, but as a quick test (quite late here, sorry!), yes, > it seems `z` is smaller: > > text data bssdec hex filename > > 1265688 104 126680 1eed8 drivers/android/rust_binder.o [s] > 1229238 104 123035 1e09b drivers/android/rust_binder.o [z] > > 2123510 0 212351 33d7f rust/core.o [s] > 2076530 0 207653 32b25 rust/core.o [z] cool, thanks for verifying. LGTM > > This is a mess; who thought it would be a good idea to support > > compiling the rust code at a different optimization level than the > > rest of the C code in the kernel? Do we really need that flexibility > > for Rust kernel code, or can we drop this feature? > > I did :P > > The idea is that, since it seemed to work out of the box when I tried, > it could be nice to keep for debugging and for having another degree > of freedom when testing the compiler/nightlies etc. > > Also, it is not intended for users, which is why I put it in the > "hacking" menu -- users should still only modify the usual global > option. > > However, it is indeed strange for the kernel and I don't mind dropping > it if people want to see it out (one could still do it manually if > needed...). > > (Related: from what I have been told, the kernel does not support > lower levels in C just due to old problems with compilers; but those > may be gone now). IIRC the kernel (or at least x86_64 defconfig) cannot be built at -O0, which is too bad if developers were myopically focused on build times. It would have been nice to have something like CONFIG_CC_OPTIMIZE_FOR_COMPILE_TIME to join CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE and CONFIG_CC_OPTIMIZE_FOR_SIZE, but maybe it's still possible to support one day. (¿Por qué no los tres? Perhaps a false-trichotomy? Sorry, but those 3 are somewhat at odds for compilation). Until then, I don't see why we need to permit developers to express such flexibility for just the Rust code, or have it differ from the intent of the C code. Does it make sense to set RUST_OPT_LEVEL_3 and CC_OPTIMIZE_FOR_SIZE? I doubt it. That doesn't seem like a development feature, but a mistake. YAGNI. Instead developers should clarify what they care about in terms of high level intent; if someone wants to micromanage optimization level flags in their forks they don't need a Kconfig to do it (they're either going to hack KBUILD_CFLAGS, CFLAGS_*.o, or KCFLAGS), and there's probably better mechanisms for fine-tooth precision of optimizing actually hot code or not via PGO and AutoFDO. https://lore.kernel.org/lkml/20210407211704.367039-1-mo...@google.com/ -- Thanks, ~Nick Desaulniers
Re: [PATCH] X86: Makefile: Replace -pg with CC_FLAGS_FTRACE
On Thu, Apr 15, 2021 at 2:43 AM zhaoxiao wrote: > > In preparation for x86 supporting ftrace built on other compiler > options, let's have the x86 Makefiles remove the $(CC_FLAGS_FTRACE) > flags, whatever these may be, rather than assuming '-pg'. > > There should be no functional change as a result of this patch. > > Signed-off-by: zhaoxiao > --- > arch/x86/kernel/Makefile | 16 > arch/x86/lib/Makefile| 2 +- I see additional CFLAGS_REMOVE_* = -pg in - arch/x86/mm/Makefile - arch/x86/kernel/cpu/Makefile - arch/x86/entry/vdso/Makefile - arch/x86/um/vdso/Makefile - arch/x86/xen/Makefile Would this same change be appropriate to all of the above? Seeing the additional possible values of CC_FLAGS_FTRACE (`-mrecord-mcount`, `-mnop-mcount`, `-mfentry`) makes we wonder if those are currently broken for these files as they are not removed, or if only `-pg` is problematic? Thank you for the patch. > 2 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile > index 2ddf08351f0b..2811fc6a17ba 100644 > --- a/arch/x86/kernel/Makefile > +++ b/arch/x86/kernel/Makefile > @@ -13,14 +13,14 @@ CPPFLAGS_vmlinux.lds += -U$(UTS_MACHINE) > > ifdef CONFIG_FUNCTION_TRACER > # Do not profile debug and lowlevel utilities > -CFLAGS_REMOVE_tsc.o = -pg > -CFLAGS_REMOVE_paravirt-spinlocks.o = -pg > -CFLAGS_REMOVE_pvclock.o = -pg > -CFLAGS_REMOVE_kvmclock.o = -pg > -CFLAGS_REMOVE_ftrace.o = -pg > -CFLAGS_REMOVE_early_printk.o = -pg > -CFLAGS_REMOVE_head64.o = -pg > -CFLAGS_REMOVE_sev-es.o = -pg > +CFLAGS_REMOVE_tsc.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_paravirt-spinlocks.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_pvclock.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_kvmclock.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_ftrace.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_early_printk.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_head64.o = $(CC_FLAGS_FTRACE) > +CFLAGS_REMOVE_sev-es.o = $(CC_FLAGS_FTRACE) > endif > > KASAN_SANITIZE_head$(BITS).o := n > diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile > index bad4dee4f0e4..0aa71b8a5bc1 100644 > --- a/arch/x86/lib/Makefile > +++ b/arch/x86/lib/Makefile > @@ -21,7 +21,7 @@ KASAN_SANITIZE_cmdline.o := n > KCSAN_SANITIZE_cmdline.o := n > > ifdef CONFIG_FUNCTION_TRACER > -CFLAGS_REMOVE_cmdline.o = -pg > +CFLAGS_REMOVE_cmdline.o = $(CC_FLAGS_FTRACE) > endif > > CFLAGS_cmdline.o := -fno-stack-protector -fno-jump-tables > -- > 2.20.1 > > > -- Thanks, ~Nick Desaulniers
Re: [PATCH v2] arm64: vdso32: drop -no-integrated-as flag
On Thu, Apr 15, 2021 at 6:31 AM Vincenzo Frascino wrote: > > > > On 4/14/21 10:45 PM, Nick Desaulniers wrote: > > Clang can assemble these files just fine; this is a relic from the top > > level Makefile conditionally adding this. We no longer need --prefix, > > --gcc-toolchain, or -Qunused-arguments flags either with this change, so > > remove those too. > > > > To test building: > > $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ > > CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 LLVM_IAS=1 \ > > defconfig arch/arm64/kernel/vdso32/ > > > > Suggested-by: Nathan Chancellor > > Signed-off-by: Nick Desaulniers > > The patch looks fine, but I have one question: the kernel requires as a > minimum > Clang/LLVM version 10.0.1. Did you verify that with that version compat vDSOs > still builds and works correctly? Hi Vincenzo, Great question, let's check. $ cd path/to/llvm-project $ git checkout origin/release/10.x $ cd llvm/build && ninja $ cd path/to/linux $ b4 am https://lore.kernel.org/lkml/20210413230609.3114365-1-ndesaulni...@google.com/ -o - | git am -3 We can't generally build ARCH=arm64 defconfig with LLVM_IAS=1 with clang-10, but dropping LLVM_IAS=1 it looks like we can still build without that. $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 -j72 defconfig clean all $ ls -l arch/arm64/kernel/vdso32 total 116 -rw-r- 1 ndesaulniers primarygroup 7534 Apr 14 14:41 Makefile -rw-r- 1 ndesaulniers primarygroup 387 Mar 31 10:47 note.c -rw-r- 1 ndesaulniers primarygroup 2544 Apr 15 09:48 note.o -rw-r- 1 ndesaulniers primarygroup 4552 Apr 15 09:48 vdso.lds -rw-r- 1 ndesaulniers primarygroup 1587 Apr 1 12:55 vdso.lds.S -rw--- 1 ndesaulniers primarygroup 3576 Apr 15 09:48 vdso.so -rw--- 1 ndesaulniers primarygroup 24380 Apr 15 09:48 vdso.so.dbg -rwxr-x--- 1 ndesaulniers primarygroup 24380 Apr 15 09:48 vdso.so.raw -rw-r- 1 ndesaulniers primarygroup 828 Apr 1 12:55 vgettimeofday.c -rw-r- 1 ndesaulniers primarygroup 29084 Apr 15 09:48 vgettimeofday.o FWIW, clang-10 was missing support for R_AARCH64_PREL32, which affects building arch/arm64/kvm/hyp/nvhe/hyp-reloc.S. > > Otherwise: > > Reviewed-by: Vincenzo Frascino > > > --- > > Changes V1 -> V2: > > * Remove --prefix, --gcc-toolchain, COMPAT_GCC_TOOLCHAIN, and > > COMPAT_GCC_TOOLCHAIN_DIR as per Nathan. > > * Credit Nathan with Suggested-by tag. > > * Remove -Qunused-arguments. > > * Update commit message. > > > > arch/arm64/kernel/vdso32/Makefile | 8 > > 1 file changed, 8 deletions(-) > > > > diff --git a/arch/arm64/kernel/vdso32/Makefile > > b/arch/arm64/kernel/vdso32/Makefile > > index 789ad420f16b..3dba0c4f8f42 100644 > > --- a/arch/arm64/kernel/vdso32/Makefile > > +++ b/arch/arm64/kernel/vdso32/Makefile > > @@ -10,15 +10,7 @@ include $(srctree)/lib/vdso/Makefile > > > > # Same as cc-*option, but using CC_COMPAT instead of CC > > ifeq ($(CONFIG_CC_IS_CLANG), y) > > -COMPAT_GCC_TOOLCHAIN_DIR := $(dir $(shell which > > $(CROSS_COMPILE_COMPAT)elfedit)) > > -COMPAT_GCC_TOOLCHAIN := $(realpath $(COMPAT_GCC_TOOLCHAIN_DIR)/..) > > - > > CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%)) > > -CC_COMPAT_CLANG_FLAGS += --prefix=$(COMPAT_GCC_TOOLCHAIN_DIR)$(notdir > > $(CROSS_COMPILE_COMPAT)) > > -CC_COMPAT_CLANG_FLAGS += -no-integrated-as -Qunused-arguments > > -ifneq ($(COMPAT_GCC_TOOLCHAIN),) > > -CC_COMPAT_CLANG_FLAGS += --gcc-toolchain=$(COMPAT_GCC_TOOLCHAIN) > > -endif > > > > CC_COMPAT ?= $(CC) > > CC_COMPAT += $(CC_COMPAT_CLANG_FLAGS) > > > > -- > Regards, > Vincenzo -- Thanks, ~Nick Desaulniers
Re: [PATCH 09/13] Samples: Rust examples
On Thu, Apr 15, 2021 at 12:10 AM Greg Kroah-Hartman wrote: > > On Wed, Apr 14, 2021 at 04:24:45PM -0700, Nick Desaulniers wrote: > > On Wed, Apr 14, 2021 at 12:35 PM Linus Torvalds > > wrote: > > > > > > On Wed, Apr 14, 2021 at 11:47 AM wrote: > > > > > > > > From: Miguel Ojeda > > > > > > > > A set of Rust modules that showcase how Rust modules look like > > > > and how to use the abstracted kernel features. > > > > > > Honestly, I'd like to see a real example. This is fine for testing, > > > but I'd like to see something a bit more real, and a bit less special > > > than the Android "binder" WIP that comes a few patches later. > > > > > > Would there be some kind of real driver or something that people could > > > use as a example of a real piece of code that actually does something > > > meaningful? > > > > Are you suggesting that they "rewrite it in Rust?" :^P *ducks* > > Well, that's what they are doing here with the binder code :) I know, but imagine the meme magic if Linus said literally that! Missed opportunity. > Seriously, binder is not a "normal" driver by any means, the only way > you can squint at it and consider it a driver is that it has a char > device node that it uses to talk to userspace with. Other than that, > it's very stand-alone and does crazy things to kernel internals, which > makes it a good canidate for a rust rewrite in that it is easy to > benchmark and no one outside of one ecosystem relies on it. > > The binder code also shows that there is a bunch of "frameworks" that > need to be ported to rust to get it to work, I think the majority of the > rust code for binder is just trying to implement core kernel things like > linked lists and the like. That will need to move into the rust kernel > core eventually. > > The binder rewrite here also is missing a number of features that the > in-kernel binder code has gotten over the years, so it is not > feature-complete by any means yet, it's still a "toy example". > > > (sorry, I couldn't help myself) Perhaps it would be a good exercise to > > demonstrate some of the benefits of using Rust for driver work? > > I've been talking with the developers here about doing a "real" driver, > as the interaction between the rust code which has one set of lifetime > rules, and the driver core/model which has a different set of lifetime > rules, is going to be what shows if this actually can be done or not. > If the two can not successfully be "bridged" together, then there will > be major issues. > > Matthew points out that a nvme driver would be a good example, and I > have a few other thoughts about what would be good to start with for > some of the basics that driver authors deal with on a daily basis > (platform driver, gpio driver, pcspkr driver, /dev/zero replacement), or > that might be more suited for a "safety critical language use-case" like > the HID parser or maybe the ACPI parser (but that falls into the rewrite > category that we want to stay away from for now...) Sage advice, and it won't hurt to come back with more examples. Perhaps folks in the Rust community who have been itching to get involved in developing their favorite operating system might be interested? One technique for new language adoption I've seen at Mozilla and Google has been a moratorium that any code in needs to have a fallback in in case doesn't work out. Perhaps that would be a good policy to consider; you MAY rewrite existing drivers in Rust, but you MUST provide a C implementation or ensure one exists as fallback until further notice. That might also allay targetability concerns. > Let's see what happens here, this patchset is a great start that > provides the core "here's how to build rust in the kernel build system", > which was a non-trivial engineering effort. Hats off to them that "all" > I had to do was successfully install the proper rust compiler on my > system (not these developers fault), and then building the kernel code > here did "just work". That's a major achievement. For sure, kudos folks and thanks Greg for taking the time to try it out and provide feedback plus ideas for more interesting drivers. -- Thanks, ~Nick Desaulniers
Re: [PATCH 00/13] [RFC] Rust support
On Wed, Apr 14, 2021 at 11:47 AM wrote: > > From: Miguel Ojeda > > Some of you have noticed the past few weeks and months that > a serious attempt to bring a second language to the kernel was > being forged. We are finally here, with an RFC that adds support > for Rust to the Linux kernel. > > This cover letter is fairly long, since there are quite a few topics > to describe, but I hope it answers as many questions as possible > before the discussion starts. > > If you are interested in following this effort, please join us > in the mailing list at: > > rust-for-li...@vger.kernel.org > > and take a look at the project itself at: > > https://github.com/Rust-for-Linux Looks like Wedson's writeup is now live. Nice job Wedson! https://security.googleblog.com/2021/04/rust-in-linux-kernel.html -- Thanks, ~Nick Desaulniers
Re: [PATCH 02/13] kallsyms: Increase maximum kernel symbol length to 512
rr(format, ...) fprintf (stderr, format, ## __VA_ARGS__)
> #define pr_warn pr_err
> diff --git a/tools/lib/perf/include/perf/event.h
> b/tools/lib/perf/include/perf/event.h
> index d82054225fcc..f5c40325b441 100644
> --- a/tools/lib/perf/include/perf/event.h
> +++ b/tools/lib/perf/include/perf/event.h
> @@ -93,7 +93,7 @@ struct perf_record_throttle {
> };
>
> #ifndef KSYM_NAME_LEN
> -#define KSYM_NAME_LEN 256
> +#define KSYM_NAME_LEN 512
> #endif
>
> struct perf_record_ksymbol {
> diff --git a/tools/lib/symbol/kallsyms.h b/tools/lib/symbol/kallsyms.h
> index 72ab9870454b..542f9b059c3b 100644
> --- a/tools/lib/symbol/kallsyms.h
> +++ b/tools/lib/symbol/kallsyms.h
> @@ -7,7 +7,7 @@
> #include
>
> #ifndef KSYM_NAME_LEN
> -#define KSYM_NAME_LEN 256
> +#define KSYM_NAME_LEN 512
> #endif
>
> static inline u8 kallsyms2elf_binding(char type)
> --
> 2.17.1
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH 03/13] Makefile: Generate CLANG_FLAGS even in GCC builds
On Wed, Apr 14, 2021 at 11:48 AM wrote:
>
> From: Miguel Ojeda
>
> To support Rust under GCC-built kernels, we need to save the flags that
> would have been passed if the kernel was being compiled with Clang.
>
> The reason is that bindgen -- the tool we use to generate Rust bindings
> to the C side of the kernel -- relies on libclang to parse C. Ideally:
>
> - bindgen would support a GCC backend (requested at [1]),
>
> - or the Clang driver would be perfectly compatible with GCC,
> including plugins. Unlikely, of course, but perhaps a big
> subset of configs may be possible to guarantee to be kept
> compatible nevertheless.
>
> This is also the reason why GCC builds are very experimental and some
> configurations may not work (e.g. GCC_PLUGIN_RANDSTRUCT). However,
> we keep GCC builds working (for some example configs) in the CI
> to avoid diverging/regressing further, so that we are better prepared
> for the future when a solution might become available.
>
> [1] https://github.com/rust-lang/rust-bindgen/issues/1949
>
> Link: https://github.com/Rust-for-Linux/linux/issues/167
>
> Co-developed-by: Alex Gaynor
> Signed-off-by: Alex Gaynor
> Co-developed-by: Geoffrey Thomas
> Signed-off-by: Geoffrey Thomas
> Co-developed-by: Finn Behrens
> Signed-off-by: Finn Behrens
> Co-developed-by: Adam Bratschi-Kaye
> Signed-off-by: Adam Bratschi-Kaye
> Co-developed-by: Wedson Almeida Filho
> Signed-off-by: Wedson Almeida Filho
> Signed-off-by: Miguel Ojeda
> ---
> Makefile | 27 ---
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d4784d181123..9c75354324ed 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -559,26 +559,31 @@ ifdef building_out_of_srctree
> { echo "# this is build directory, ignore it"; echo "*"; } >
> .gitignore
> endif
>
> -# The expansion should be delayed until arch/$(SRCARCH)/Makefile is included.
> -# Some architectures define CROSS_COMPILE in arch/$(SRCARCH)/Makefile.
> -# CC_VERSION_TEXT is referenced from Kconfig (so it needs export),
> -# and from include/config/auto.conf.cmd to detect the compiler upgrade.
> -CC_VERSION_TEXT = $(shell $(CC) --version 2>/dev/null | head -n 1 | sed
> 's/\#//g')
> +TENTATIVE_CLANG_FLAGS := -Werror=unknown-warning-option
>
> -ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
> ifneq ($(CROSS_COMPILE),)
> -CLANG_FLAGS+= --target=$(notdir $(CROSS_COMPILE:%-=%))
> +TENTATIVE_CLANG_FLAGS += --target=$(notdir $(CROSS_COMPILE:%-=%))
> GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
> -CLANG_FLAGS+= --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
> +TENTATIVE_CLANG_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir
> $(CROSS_COMPILE))
> GCC_TOOLCHAIN := $(realpath $(GCC_TOOLCHAIN_DIR)/..)
> endif
> ifneq ($(GCC_TOOLCHAIN),)
> -CLANG_FLAGS+= --gcc-toolchain=$(GCC_TOOLCHAIN)
> +TENTATIVE_CLANG_FLAGS += --gcc-toolchain=$(GCC_TOOLCHAIN)
> endif
> ifneq ($(LLVM_IAS),1)
> -CLANG_FLAGS+= -no-integrated-as
> +TENTATIVE_CLANG_FLAGS += -no-integrated-as
> endif
> -CLANG_FLAGS+= -Werror=unknown-warning-option
> +
> +export TENTATIVE_CLANG_FLAGS
I'm ok with this approach, but I'm curious:
If the user made a copy of the CLANG_FLAGS variable and modified its
copy, would TENTATIVE_CLANG_FLAGS even be necessary? IIUC,
TENTATIVE_CLANG_FLAGS is used to filter out certain flags before
passing them to bindgen?
Or, I'm curious whether you even need to rename this variable (or
create a new variable) at all? Might make for a shorter diff if you
just keep the existing identifier (CLANG_FLAGS), but create them
unconditionally (at least not conditional on CC=clang).
> +
> +# The expansion should be delayed until arch/$(SRCARCH)/Makefile is included.
> +# Some architectures define CROSS_COMPILE in arch/$(SRCARCH)/Makefile.
> +# CC_VERSION_TEXT is referenced from Kconfig (so it needs export),
> +# and from include/config/auto.conf.cmd to detect the compiler upgrade.
> +CC_VERSION_TEXT = $(shell $(CC) --version 2>/dev/null | head -n 1 | sed
> 's/\#//g')
> +
> +ifneq ($(findstring clang,$(CC_VERSION_TEXT)),)
> +CLANG_FLAGS+= $(TENTATIVE_CLANG_FLAGS)
> KBUILD_CFLAGS += $(CLANG_FLAGS)
> KBUILD_AFLAGS += $(CLANG_FLAGS)
> export CLANG_FLAGS
> --
> 2.17.1
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH 09/13] Samples: Rust examples
On Wed, Apr 14, 2021 at 12:35 PM Linus Torvalds wrote: > > On Wed, Apr 14, 2021 at 11:47 AM wrote: > > > > From: Miguel Ojeda > > > > A set of Rust modules that showcase how Rust modules look like > > and how to use the abstracted kernel features. > > Honestly, I'd like to see a real example. This is fine for testing, > but I'd like to see something a bit more real, and a bit less special > than the Android "binder" WIP that comes a few patches later. > > Would there be some kind of real driver or something that people could > use as a example of a real piece of code that actually does something > meaningful? Are you suggesting that they "rewrite it in Rust?" :^P *ducks* (sorry, I couldn't help myself) Perhaps it would be a good exercise to demonstrate some of the benefits of using Rust for driver work? -- Thanks, ~Nick Desaulniers
Re: [PATCH 04/13] Kbuild: Rust support
mv $(obj)/$(subst .o,,$(notdir $@)).d $(depfile); \
> + sed -i '/^\#/d' $(depfile)
> +
> +$(obj)/%.o: $(src)/%.rs FORCE
> + $(call if_changed_dep,rustc_o_rs)
> +
> # Compile assembler sources (.S)
> # ---
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 8cd67b1b6d15..bd6cb3562fb4 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -8,6 +8,7 @@ ldflags-y += $(EXTRA_LDFLAGS)
> # flags that take effect in current and sub directories
> KBUILD_AFLAGS += $(subdir-asflags-y)
> KBUILD_CFLAGS += $(subdir-ccflags-y)
> +KBUILD_RUSTCFLAGS += $(subdir-rustcflags-y)
>
> # Figure out what we need to build from the various variables
> # ===
> @@ -122,6 +123,10 @@ _c_flags = $(filter-out
> $(CFLAGS_REMOVE_$(target-stem).o), \
> $(filter-out $(ccflags-remove-y), \
> $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(ccflags-y)) \
> $(CFLAGS_$(target-stem).o))
> +_rustc_flags= $(filter-out $(RUSTCFLAGS_REMOVE_$(target-stem).o), \
> + $(filter-out $(rustcflags-remove-y), \
> + $(KBUILD_RUSTCFLAGS) $(rustcflags-y)) \
> + $(RUSTCFLAGS_$(target-stem).o))
> _a_flags = $(filter-out $(AFLAGS_REMOVE_$(target-stem).o), \
> $(filter-out $(asflags-remove-y), \
> $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) $(asflags-y)) \
> @@ -191,6 +196,11 @@ modkern_cflags =
> \
> $(KBUILD_CFLAGS_MODULE) $(CFLAGS_MODULE), \
> $(KBUILD_CFLAGS_KERNEL) $(CFLAGS_KERNEL) $(modfile_flags))
>
> +modkern_rustcflags = \
> + $(if $(part-of-module), \
> + $(KBUILD_RUSTCFLAGS_MODULE) $(RUSTCFLAGS_MODULE), \
> + $(KBUILD_RUSTCFLAGS_KERNEL) $(RUSTCFLAGS_KERNEL))
> +
> modkern_aflags = $(if $(part-of-module), \
> $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE), \
> $(KBUILD_AFLAGS_KERNEL) $(AFLAGS_KERNEL))
> @@ -200,6 +210,8 @@ c_flags= -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS)
> $(LINUXINCLUDE) \
> $(_c_flags) $(modkern_cflags) \
> $(basename_flags) $(modname_flags)
>
> +rustc_flags = $(_rustc_flags) $(modkern_rustcflags)
> @$(objtree)/include/generated/rustc_cfg
> +
> a_flags= -Wp,-MMD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \
> $(_a_flags) $(modkern_aflags)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 2568dbe16ed6..a83d646ecef5 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -637,6 +637,56 @@ static struct conf_printer kconfig_printer_cb =
> .print_comment = kconfig_print_comment,
> };
>
> +/*
> + * rustc cfg printer
> + *
> + * This printer is used when generating the resulting rustc configuration
> + * after kconfig invocation and `defconfig` files.
> + */
> +static void rustc_cfg_print_symbol(FILE *fp, struct symbol *sym, const char
> *value, void *arg)
> +{
> + const char *str;
> +
> + switch (sym->type) {
> + case S_INT:
> + case S_HEX:
> + case S_BOOLEAN:
> + case S_TRISTATE:
> + str = sym_escape_string_value(value);
> +
> + /*
> +* We don't care about disabled ones, i.e. no need for
> +* what otherwise are "comments" in other printers.
> +*/
> + if (*value == 'n')
> + return;
> +
> + /*
> +* To have similar functionality to the C macro `IS_ENABLED()`
> +* we provide an empty `--cfg CONFIG_X` here in both `y`
> +* and `m` cases.
> +*
> +* Then, the common `fprintf()` below will also give us
> +* a `--cfg CONFIG_X="y"` or `--cfg CONFIG_X="m"`, which can
> +* be used as the equivalent of `IS_BUILTIN()`/`IS_MODULE()`.
> +*/
> + if (*value == 'y' || *value == 'm')
> + fprintf(fp, "--cfg=%s%s\n", CONFIG_, sym->name);
> +
> + break;
> + default:
> + str = value;
> + break;
> + }
> +
> + fprintf(fp, "--cfg=%s%s=%s\n", CONFIG_, sym->name, str);
> +}
> +
> +static struct conf_printer rustc_cfg_printer_cb =
> +{
> + .print_symbol = rustc_cfg_print_symbol,
> +};
> +
> /*
> * Header printer
> *
> @@ -1044,7 +1094,7 @@ int conf_write_autoconf(int overwrite)
> struct symbol *sym;
> const char *name;
> const char *autoconf_name = conf_get_autoconfig_name();
> - FILE *out, *out_h;
> + FILE *out, *out_h, *out_rustc_cfg;
> int i;
>
> if (!overwrite && is_present(autoconf_name))
> @@ -1065,6 +1115,13 @@ int conf_write_autoconf(int overwrite)
> return 1;
> }
>
> + out_rustc_cfg = fopen(".tmp_rustc_cfg", "w");
> + if (!out_rustc_cfg) {
> + fclose(out);
> + fclose(out_h);
> + return 1;
> + }
> +
> conf_write_heading(out, _printer_cb, NULL);
> conf_write_heading(out_h, _printer_cb, NULL);
>
> @@ -1076,9 +1133,11 @@ int conf_write_autoconf(int overwrite)
> /* write symbols to auto.conf and autoconf.h */
> conf_write_symbol(out, sym, _printer_cb, (void *)1);
> conf_write_symbol(out_h, sym, _printer_cb, NULL);
> + conf_write_symbol(out_rustc_cfg, sym, _cfg_printer_cb,
> NULL);
> }
> fclose(out);
> fclose(out_h);
> + fclose(out_rustc_cfg);
>
> name = getenv("KCONFIG_AUTOHEADER");
> if (!name)
> @@ -1097,6 +1156,12 @@ int conf_write_autoconf(int overwrite)
> if (rename(".tmpconfig", autoconf_name))
> return 1;
>
> + name = "include/generated/rustc_cfg";
> + if (make_parent_dir(name))
> + return 1;
> + if (rename(".tmp_rustc_cfg", name))
> + return 1;
> +
> return 0;
> }
>
> diff --git a/scripts/rust-version.sh b/scripts/rust-version.sh
> new file mode 100755
> index ..67b6d31688e2
> --- /dev/null
> +++ b/scripts/rust-version.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# rust-version rust-command
> +#
> +# Print the compiler version of `rust-command' in a 5 or 6-digit form
> +# such as `14502' for rustc-1.45.2 etc.
> +#
> +# Returns 0 if not found (so that Kconfig does not complain)
> +compiler="$*"
> +
> +if [ ${#compiler} -eq 0 ]; then
> + echo "Error: No compiler specified." >&2
> + printf "Usage:\n\t$0 \n" >&2
> + exit 1
> +fi
> +
> +if ! command -v $compiler >/dev/null 2>&1; then
> + echo 0
> + exit 0
> +fi
> +
> +VERSION=$($compiler --version | cut -f2 -d' ')
> +
> +# Cut suffix if any (e.g. `-dev`)
> +VERSION=$(echo $VERSION | cut -f1 -d'-')
> +
> +MAJOR=$(echo $VERSION | cut -f1 -d'.')
> +MINOR=$(echo $VERSION | cut -f2 -d'.')
> +PATCHLEVEL=$(echo $VERSION | cut -f3 -d'.')
> +printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL
> --
> 2.17.1
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH 11/13] MAINTAINERS: Rust
On Wed, Apr 14, 2021 at 11:50 AM wrote:
>
> From: Miguel Ojeda
>
> Miguel, Alex and Wedson will be maintaining the Rust support.
>
> Co-developed-by: Alex Gaynor
> Signed-off-by: Alex Gaynor
> Co-developed-by: Wedson Almeida Filho
> Signed-off-by: Wedson Almeida Filho
> Signed-off-by: Miguel Ojeda
> ---
> MAINTAINERS | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e876927c60d..de32aaa5cabd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15547,6 +15547,20 @@ L: linux-r...@vger.kernel.org
...
> +F: rust/
> +F: samples/rust/
> +F: Documentation/rust/
Probably some other files, too? Like the target.json files under
arch/{arch}/rust/ ?
--
Thanks,
~Nick Desaulniers
Re: [PATCH 10/13] Documentation: Rust general information
can > install > +the component manually:: > + > +rustup component add rustdoc > + > +The standalone installers also come with ``rustdoc``. > + > + > +Configuration > +- > + > +``Rust support`` (``CONFIG_RUST``) needs to be enabled in the ``General > setup`` > +menu. The option is only shown if the build system can locate ``rustc``. > +In turn, this will make visible the rest of options that depend on Rust. > + > +Afterwards, go to:: > + > +Kernel hacking > + -> Sample kernel code > + -> Rust samples > + > +And enable some sample modules either as built-in or as loadable. > + > + > +Building > + > + > +Building a kernel with Clang or a complete LLVM toolchain is the best > supported > +setup at the moment. That is:: > + > +make ARCH=... CROSS_COMPILE=... CC=clang -j... > + > +or:: > + > +make ARCH=... CROSS_COMPILE=... LLVM=1 -j... Please reorder; prefer LLVM=1 to CC=clang. Probably worth another cross reference to :ref:`kbuild_llvm`. > + > +Using GCC also works for some configurations, but it is *very* experimental > at > +the moment. > + > + > +Hacking > +--- > + > +If you want to dive deeper, take a look at the source code of the samples > +at ``samples/rust/``, the Rust support code under ``rust/`` and > +the ``Rust hacking`` menu under ``Kernel hacking``. > + > +If you use GDB/Binutils and Rust symbols aren't getting demangled, the reason > +is your toolchain doesn't support Rust's new v0 mangling scheme yet. There > are "new" as in changed, or "new" as in Rust previously did not mangle symbols? > +a few ways out: > + > + - If you don't mind building your own tools, we provide the following fork > +with the support cherry-picked from GCC on top of very recent releases: > + > + > https://github.com/Rust-for-Linux/binutils-gdb/releases/tag/gdb-10.1-release-rust > + > https://github.com/Rust-for-Linux/binutils-gdb/releases/tag/binutils-2_35_1-rust > + > + - If you only need GDB and can enable ``CONFIG_DEBUG_INFO``, do so: > +some versions of GDB (e.g. vanilla GDB 10.1) are able to use > +the pre-demangled names embedded in the debug info. > + > + - If you don't need loadable module support, you may compile without > +the ``-Zsymbol-mangling-version=v0`` flag. However, we don't maintain > +support for that, so avoid it unless you are in a hurry. > -- > 2.17.1 > -- Thanks, ~Nick Desaulniers
Re: [PATCH 11/13] MAINTAINERS: Rust
On Wed, Apr 14, 2021 at 11:50 AM wrote: > > From: Miguel Ojeda > > Miguel, Alex and Wedson will be maintaining the Rust support. > > Co-developed-by: Alex Gaynor > Signed-off-by: Alex Gaynor > Co-developed-by: Wedson Almeida Filho > Signed-off-by: Wedson Almeida Filho > Signed-off-by: Miguel Ojeda > --- > MAINTAINERS | 14 ++ > 1 file changed, 14 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9e876927c60d..de32aaa5cabd 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15547,6 +15547,20 @@ L: linux-r...@vger.kernel.org > S: Maintained > F: drivers/infiniband/ulp/rtrs/ > > +RUST > +M: Miguel Ojeda > +M: Alex Gaynor > +M: Wedson Almeida Filho > +L: rust-for-li...@vger.kernel.org > +S: Maintained Assuming this will at least be part of Wedson's core responsibilities, shouldn't this be "Supported." Per Maintainers: 87 S: *Status*, one of the following: 88 Supported: Someone is actually paid to look after this. 89 Maintained: Someone actually looks after it. Either way, Acked-by: Nick Desaulniers > +W: https://github.com/Rust-for-Linux/linux > +B: https://github.com/Rust-for-Linux/linux/issues > +T: git https://github.com/Rust-for-Linux/linux.git rust-next > +F: rust/ > +F: samples/rust/ > +F: Documentation/rust/ > +K: \b(?i:rust)\b > + > RXRPC SOCKETS (AF_RXRPC) > M: David Howells > L: linux-...@lists.infradead.org > -- > 2.17.1 > -- Thanks, ~Nick Desaulniers
[PATCH v2] arm64: vdso32: drop -no-integrated-as flag
Clang can assemble these files just fine; this is a relic from the top level Makefile conditionally adding this. We no longer need --prefix, --gcc-toolchain, or -Qunused-arguments flags either with this change, so remove those too. To test building: $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 LLVM_IAS=1 \ defconfig arch/arm64/kernel/vdso32/ Suggested-by: Nathan Chancellor Signed-off-by: Nick Desaulniers --- Changes V1 -> V2: * Remove --prefix, --gcc-toolchain, COMPAT_GCC_TOOLCHAIN, and COMPAT_GCC_TOOLCHAIN_DIR as per Nathan. * Credit Nathan with Suggested-by tag. * Remove -Qunused-arguments. * Update commit message. arch/arm64/kernel/vdso32/Makefile | 8 1 file changed, 8 deletions(-) diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 789ad420f16b..3dba0c4f8f42 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -10,15 +10,7 @@ include $(srctree)/lib/vdso/Makefile # Same as cc-*option, but using CC_COMPAT instead of CC ifeq ($(CONFIG_CC_IS_CLANG), y) -COMPAT_GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE_COMPAT)elfedit)) -COMPAT_GCC_TOOLCHAIN := $(realpath $(COMPAT_GCC_TOOLCHAIN_DIR)/..) - CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%)) -CC_COMPAT_CLANG_FLAGS += --prefix=$(COMPAT_GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE_COMPAT)) -CC_COMPAT_CLANG_FLAGS += -no-integrated-as -Qunused-arguments -ifneq ($(COMPAT_GCC_TOOLCHAIN),) -CC_COMPAT_CLANG_FLAGS += --gcc-toolchain=$(COMPAT_GCC_TOOLCHAIN) -endif CC_COMPAT ?= $(CC) CC_COMPAT += $(CC_COMPAT_CLANG_FLAGS) -- 2.31.1.295.g9ea45b61b8-goog
Re: [PATCH] arm64: alternatives: Move length validation in alternative_{insn,endif}
On Tue, Apr 13, 2021 at 5:09 PM Nathan Chancellor wrote:
>
> After commit 2decad92f473 ("arm64: mte: Ensure TIF_MTE_ASYNC_FAULT is
> set atomically"), LLVM's integrated assembler fails to build entry.S:
>
> :5:7: error: expected assembly-time absolute expression
> .org . - (664b-663b) + (662b-661b)
> ^
> :6:7: error: expected assembly-time absolute expression
> .org . - (662b-661b) + (664b-663b)
> ^
>
> The root cause is LLVM's assembler has a one-pass design, meaning it
> cannot figure out these instruction lengths when the .org directive is
> outside of the subsection that they are in, which was changed by the
> .arch_extension directive added in the above commit.
>
> Apply the same fix from commit 966a0acce2fc ("arm64/alternatives: move
> length validation inside the subsection") to the alternative_endif
> macro, shuffling the .org directives so that the length validation
> happen will always happen in the same subsections. alternative_insn has
> not shown any issue yet but it appears that it could have the same issue
> in the future so just preemptively change it.
Thanks Nathan.
Reviewed-by: Nick Desaulniers
Tested-by: Nick Desaulniers
I did some additional disassembly comparison. In case we ever need it
again, I'll copy it below for posterity.
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu make LLVM=1 LLVM_IAS=1
-j72 O=/tmp/a defconfig all
$ b4 am https://lore.kernel.org/lkml/20210414000803.662534-1-nat...@kernel.org/
-o - | git am -3
$ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu make LLVM=1 LLVM_IAS=1
-j72 O=/tmp/b defconfig all
$ for f in $(find /tmp/a/arch/arm64 -name \*.o); do llvm-objdump -dr
$f > $f.txt; done
$ for f in $(find /tmp/b/arch/arm64 -name \*.o); do llvm-objdump -dr
$f > $f.txt; done
$ for f in $(find /tmp/a/arch/arm64 -name \*.o); do diff -u $f.txt
$(echo $f.txt|sed 's/a/b/'); done | less
For no difference. You can check more sections than .text by changing
`-d` to `-D` for llvm-objdump, though you're going to get a lot of
noise related to changes in .strtab and relocations referring to debug
info (.debug_str). But if I drop your patch, rebuild, and recompare,
I see the same differences.
>
> Cc: sta...@vger.kernel.org
> Fixes: f7b93d42945c ("arm64/alternatives: use subsections for replacement
> sequences")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1347
> Signed-off-by: Nathan Chancellor
> ---
>
> Apologies if my explanation or terminology is off, I am only now getting
> more familiar with assembly.
>
> arch/arm64/include/asm/alternative-macros.h | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/include/asm/alternative-macros.h
> b/arch/arm64/include/asm/alternative-macros.h
> index 5df500dcc627..8a078fc662ac 100644
> --- a/arch/arm64/include/asm/alternative-macros.h
> +++ b/arch/arm64/include/asm/alternative-macros.h
> @@ -97,9 +97,9 @@
> .popsection
> .subsection 1
> 663: \insn2
> -664: .previous
> - .org. - (664b-663b) + (662b-661b)
> +664: .org. - (664b-663b) + (662b-661b)
> .org. - (662b-661b) + (664b-663b)
> + .previous
> .endif
> .endm
>
> @@ -169,11 +169,11 @@
> */
> .macro alternative_endif
> 664:
> + .org. - (664b-663b) + (662b-661b)
> + .org. - (662b-661b) + (664b-663b)
> .if .Lasm_alt_mode==0
> .previous
> .endif
> - .org. - (664b-663b) + (662b-661b)
> - .org. - (662b-661b) + (664b-663b)
> .endm
>
> /*
>
> base-commit: 738fa58ee1328481d1d7889e7c430b3401c571b9
> --
> 2.31.1.272.g89b43f80a5
>
--
Thanks,
~Nick Desaulniers
[PATCH] arm64: vdso32: drop -no-integrated-as flag
Clang can assemble these files just fine; this is a relic from the top level Makefile conditionally adding this. To test building: $ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ CROSS_COMPILE_COMPAT=arm-linux-gnueabi- make LLVM=1 LLVM_IAS=1 \ defconfig arch/arm64/kernel/vdso32/ Signed-off-by: Nick Desaulniers --- arch/arm64/kernel/vdso32/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile index 789ad420f16b..7812717f8b79 100644 --- a/arch/arm64/kernel/vdso32/Makefile +++ b/arch/arm64/kernel/vdso32/Makefile @@ -15,7 +15,7 @@ COMPAT_GCC_TOOLCHAIN := $(realpath $(COMPAT_GCC_TOOLCHAIN_DIR)/..) CC_COMPAT_CLANG_FLAGS := --target=$(notdir $(CROSS_COMPILE_COMPAT:%-=%)) CC_COMPAT_CLANG_FLAGS += --prefix=$(COMPAT_GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE_COMPAT)) -CC_COMPAT_CLANG_FLAGS += -no-integrated-as -Qunused-arguments +CC_COMPAT_CLANG_FLAGS += -Qunused-arguments ifneq ($(COMPAT_GCC_TOOLCHAIN),) CC_COMPAT_CLANG_FLAGS += --gcc-toolchain=$(COMPAT_GCC_TOOLCHAIN) endif -- 2.31.1.295.g9ea45b61b8-goog
[PATCH v3] gcov: clang: drop support for clang-10 and older
LLVM changed the expected function signatures for llvm_gcda_start_file()
and llvm_gcda_emit_function() in the clang-11 release. Drop the older
implementations and require folks to upgrade their compiler if they're
interested in GCOV support.
Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
Link: https://lkml.kernel.org/r/20210312224132.3413602-3-ndesaulni...@google.com
Signed-off-by: Nick Desaulniers
Suggested-by: Nathan Chancellor
Acked-by: Peter Oberparleiter
Reviewed-by: Nathan Chancellor
Reviewed-by: Fangrui Song
Cc: Prasad Sodagudi
Cc: Johannes Berg
---
This patch is
https://lore.kernel.org/lkml/20210412214210.6e1ecca9cdc5.I24459763acf0591d5e6b31c7e3a59890d802f79c@changeid/
and
https://lore.kernel.org/lkml/20210312224132.3413602-3-ndesaulni...@google.com/
rolled into one, then rebased on linux-next with
https://lore.kernel.org/lkml/20210412214210.6e1ecca9cdc5.I24459763acf0591d5e6b31c7e3a59890d802f79c@changeid/
applied. I merged the Reviewed-by and Acked-by tags. It was
simpler to drop v1
https://www.spinics.net/lists/mm-commits/msg154861.html in order to land
commit 9562fd132985 ("gcov: re-fix clang-11+ support").
kernel/gcov/Kconfig | 1 +
kernel/gcov/clang.c | 103
2 files changed, 1 insertion(+), 103 deletions(-)
diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index f62de2dea8a3..58f87a3092f3 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -4,6 +4,7 @@ menu "GCOV-based kernel profiling"
config GCOV_KERNEL
bool "Enable gcov-based kernel profiling"
depends on DEBUG_FS
+ depends on !CC_IS_CLANG || CLANG_VERSION >= 11
select CONSTRUCTORS
default n
help
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index d43ffd0c5ddb..cbb0bed958ab 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -69,16 +69,10 @@ struct gcov_fn_info {
u32 ident;
u32 checksum;
-#if CONFIG_CLANG_VERSION < 11
- u8 use_extra_checksum;
-#endif
u32 cfg_checksum;
u32 num_counters;
u64 *counters;
-#if CONFIG_CLANG_VERSION < 11
- const char *function_name;
-#endif
};
static struct gcov_info *current_info;
@@ -108,16 +102,6 @@ void llvm_gcov_init(llvm_gcov_callback writeout,
llvm_gcov_callback flush)
}
EXPORT_SYMBOL(llvm_gcov_init);
-#if CONFIG_CLANG_VERSION < 11
-void llvm_gcda_start_file(const char *orig_filename, const char version[4],
- u32 checksum)
-{
- current_info->filename = orig_filename;
- memcpy(_info->version, version, sizeof(current_info->version));
- current_info->checksum = checksum;
-}
-EXPORT_SYMBOL(llvm_gcda_start_file);
-#else
void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
{
current_info->filename = orig_filename;
@@ -125,28 +109,7 @@ void llvm_gcda_start_file(const char *orig_filename, u32
version, u32 checksum)
current_info->checksum = checksum;
}
EXPORT_SYMBOL(llvm_gcda_start_file);
-#endif
-#if CONFIG_CLANG_VERSION < 11
-void llvm_gcda_emit_function(u32 ident, const char *function_name,
- u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
-{
- struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
-
- if (!info)
- return;
-
- INIT_LIST_HEAD(>head);
- info->ident = ident;
- info->checksum = func_checksum;
- info->use_extra_checksum = use_extra_checksum;
- info->cfg_checksum = cfg_checksum;
- if (function_name)
- info->function_name = kstrdup(function_name, GFP_KERNEL);
-
- list_add_tail(>head, _info->functions);
-}
-#else
void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
{
struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
@@ -160,7 +123,6 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
u32 cfg_checksum)
info->cfg_checksum = cfg_checksum;
list_add_tail(>head, _info->functions);
}
-#endif
EXPORT_SYMBOL(llvm_gcda_emit_function);
void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
@@ -291,16 +253,8 @@ int gcov_info_is_compatible(struct gcov_info *info1,
struct gcov_info *info2)
!list_is_last(_ptr2->head, >functions)) {
if (fn_ptr1->checksum != fn_ptr2->checksum)
return false;
-#if CONFIG_CLANG_VERSION < 11
- if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
- return false;
- if (fn_ptr1->use_extra_checksum &&
- fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
- return false;
-#else
if (fn_ptr1-&
Re: [PATCH] gcov: clang: fix clang-11+ build
On Mon, Apr 12, 2021 at 12:42 PM Johannes Berg
wrote:
>
> From: Johannes Berg
>
> With clang-11+, the code is broken due to my kvmalloc() conversion
> (which predated the clang-11 support code) leaving one vmalloc()
> in place. Fix that.
>
> Signed-off-by: Johannes Berg
> ---
> This fixes a clang-11 build issue reported against current
> linux-next since the clang-11+ support landed in v5.12-rc5
Thanks for the patch, and noticing the breakage so quickly. I
recently added GCOV to our CI, but I've been fighting enough fires
this morning that I haven't had time to check this particular build!
Reviewed-by: Nick Desaulniers
Tested-by: Nick Desaulniers
> and my kvmalloc() conversion patch is in akpm/linux-next.
> I guess this should be folded into
>
> gcov: use kvmalloc()
>
> If desired, I can send a combined patch instead.
> ---
> kernel/gcov/clang.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
> index 04a65443005d..d43ffd0c5ddb 100644
> --- a/kernel/gcov/clang.c
> +++ b/kernel/gcov/clang.c
> @@ -368,7 +368,7 @@ static struct gcov_fn_info *gcov_fn_info_dup(struct
> gcov_fn_info *fn)
> INIT_LIST_HEAD(_dup->head);
>
> cv_size = fn->num_counters * sizeof(fn->counters[0]);
> - fn_dup->counters = vmalloc(cv_size);
> + fn_dup->counters = kvmalloc(cv_size, GFP_KERNEL);
> if (!fn_dup->counters) {
> kfree(fn_dup);
> return NULL;
> --
> 2.30.2
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH] crypto: arm/curve25519 - Move '.fpu' after '.arch'
On Fri, Apr 9, 2021 at 3:12 PM Nathan Chancellor wrote:
>
> Debian's clang carries a patch that makes the default FPU mode
> 'vfp3-d16' instead of 'neon' for 'armv7-a' to avoid generating NEON
> instructions on hardware that does not support them:
>
> https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/raw/5a61ca6f21b4ad8c6ac4970e5ea5a7b5b4486d22/debian/patches/clang-arm-default-vfp3-on-armv7a.patch
> https://bugs.debian.org/841474
> https://bugs.debian.org/842142
> https://bugs.debian.org/914268
Another good link would be the one from Jessica describing more
precisely what the ARM targets for Debian are:
https://wiki.debian.org/ArchitectureSpecificsMemo#armel
>
> This results in the following build error when clang's integrated
> assembler is used because the '.arch' directive overrides the '.fpu'
> directive:
>
> arch/arm/crypto/curve25519-core.S:25:2: error: instruction requires: NEON
> vmov.i32 q0, #1
> ^
> arch/arm/crypto/curve25519-core.S:26:2: error: instruction requires: NEON
> vshr.u64 q1, q0, #7
> ^
> arch/arm/crypto/curve25519-core.S:27:2: error: instruction requires: NEON
> vshr.u64 q0, q0, #8
> ^
> arch/arm/crypto/curve25519-core.S:28:2: error: instruction requires: NEON
> vmov.i32 d4, #19
> ^
>
> Shuffle the order of the '.arch' and '.fpu' directives so that the code
> builds regardless of the default FPU mode. This has been tested against
> both clang with and without Debian's patch and GCC.
>
> Cc: sta...@vger.kernel.org
> Fixes: d8f1308a025f ("crypto: arm/curve25519 - wire up NEON implementation")
> Link: https://github.com/ClangBuiltLinux/continuous-integration2/issues/118
> Reported-by: Arnd Bergmann
> Suggested-by: Arnd Bergmann
> Suggested-by: Jessica Clarke
> Signed-off-by: Nathan Chancellor
Great work tracking down that Debian was carrying patches! Thank you!
I've run this through the same 3 assemblers.
Reviewed-by: Nick Desaulniers
Tested-by: Nick Desaulniers
> ---
> arch/arm/crypto/curve25519-core.S | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/crypto/curve25519-core.S
> b/arch/arm/crypto/curve25519-core.S
> index be18af52e7dc..b697fa5d059a 100644
> --- a/arch/arm/crypto/curve25519-core.S
> +++ b/arch/arm/crypto/curve25519-core.S
> @@ -10,8 +10,8 @@
> #include
>
> .text
> -.fpu neon
> .arch armv7-a
> +.fpu neon
> .align 4
>
> ENTRY(curve25519_neon)
>
> base-commit: e49d033bddf5b565044e2abe4241353959bc9120
> --
> 2.31.1.189.g2e36527f23
>
--
Thanks,
~Nick Desaulniers
Re: static_branch/jump_label vs branch merging
On Fri, Apr 9, 2021 at 4:18 AM Peter Zijlstra wrote:
>
> On Fri, Apr 09, 2021 at 12:55:18PM +0200, Florian Weimer wrote:
> > * Ard Biesheuvel:
> >
> > > Wouldn't that require the compiler to interpret the contents of the
> > > asm() block?
> >
> > Yes and no. It would require proper toolchain support, so in this case
> > a new ELF relocation type, with compiler, assembler, and linker support
> > to generate those relocations and process them. As far as I understand
> > it, the kernel doesn't do things this way.
>
> I don't think that all is needed. All we need is for the compiler to
> recognise that:
>
> if (cond) {
> stmt-A;
> }
> if (cond) {
> stmt-B;
> }
>
> both cond are equivalent and hence can merge the blocks like:
>
> if (cond) {
> stmt-A;
> stmt-B;
> }
>
> But because @cond is some super opaque asm crap, the compiler throws up
> it's imaginry hands and says it cannot possibly tell and leaves them as
> is.
Right, because if `cond` has side effects (such as is implied by asm
statements that are volatile qualified), suddenly those side effects
would only occur once whereas previously they occurred twice.
Since asm goto is implicitly volatile qualified, it sounds like
removing the implicit volatile qualifier from asm goto might help?
Then if there were side effects but you forgot to inform the compiler
that there were via an explicit volatile qualifier, and it performed
the suggested merge, oh well.
--
Thanks,
~Nick Desaulniers
Re: [PATCH -next] lib: zstd: Make symbol 'HUF_compressWeights_wksp' static
On Thu, Apr 8, 2021 at 5:55 AM Zhao Xuehui wrote:
>
> The symbol 'HUF_compressWeights_wksp' is not used outside of
> huf_compress.c, so this commit marks it static.
Reviewed-by: Nick Desaulniers
Quite a few other functions are declared in a header, but I don't see
any existing callers in tree. I wonder if the maintainer could
consider cleaning these up so that we don't retain them in binaries
without dead code elimination enabled, or if there's a need to keep
this code in line with an external upstream codebase?
>
> Signed-off-by: Zhao Xuehui
> ---
> lib/zstd/huf_compress.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/zstd/huf_compress.c b/lib/zstd/huf_compress.c
> index fd32838c185f..1e5e001c3d41 100644
> --- a/lib/zstd/huf_compress.c
> +++ b/lib/zstd/huf_compress.c
> @@ -79,7 +79,8 @@ unsigned HUF_optimalTableLog(unsigned maxTableLog, size_t
> srcSize, unsigned maxS
> * Note : all elements within weightTable are supposed to be <=
> HUF_TABLELOG_MAX.
> */
> #define MAX_FSE_TABLELOG_FOR_HUFF_HEADER 6
> -size_t HUF_compressWeights_wksp(void *dst, size_t dstSize, const void
> *weightTable, size_t wtSize, void *workspace, size_t workspaceSize)
> +static size_t HUF_compressWeights_wksp(void *dst, size_t dstSize, const void
> *weightTable,
> + size_t wtSize, void *workspace, size_t
> workspaceSize)
> {
> BYTE *const ostart = (BYTE *)dst;
> BYTE *op = ostart;
>
--
Thanks,
~Nick Desaulniers
[PATCH v2] gcov: re-fix clang-11+ support
LLVM changed the expected function signature for
llvm_gcda_emit_function() in the clang-11 release. Users of clang-11 or
newer may have noticed their kernels producing invalid coverage
information:
$ llvm-cov gcov -a -c -u -f -b .gcda -- gcno=.gcno
1 : checksum mismatch, \
(, ) != (, )
2 Invalid .gcda File!
...
Fix up the function signatures so calling this function interprets its
parameters correctly and computes the correct cfg checksum. In
particular, in clang-11, the additional checksum is no longer optional.
Link: https://reviews.llvm.org/rG25544ce2df0daa4304c07e64b9c8b0f7df60c11d
Cc: sta...@vger.kernel.org #5.4+
Reported-by: Prasad Sodagudi
Tested-by: Prasad Sodagudi
Signed-off-by: Nick Desaulniers
Reviewed-by: Nathan Chancellor
---
Changes V1 -> V2:
* Carried Nathan's reviewed-by tag.
* Rebased on mainline, as per Andrew.
* Left off patch 2/2 from the series
https://lore.kernel.org/lkml/20210407185456.41943-1-ndesaulni...@google.com/
I assume that dropping support for clang-10+GCOV will be held
separately for -next for 5.13, while this will be sent for 5.12?
kernel/gcov/clang.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index 8743150db2ac..c466c7fbdece 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -70,7 +70,9 @@ struct gcov_fn_info {
u32 ident;
u32 checksum;
+#if CONFIG_CLANG_VERSION < 11
u8 use_extra_checksum;
+#endif
u32 cfg_checksum;
u32 num_counters;
@@ -145,10 +147,8 @@ void llvm_gcda_emit_function(u32 ident, const char
*function_name,
list_add_tail(>head, _info->functions);
}
-EXPORT_SYMBOL(llvm_gcda_emit_function);
#else
-void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
- u8 use_extra_checksum, u32 cfg_checksum)
+void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
{
struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
@@ -158,12 +158,11 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
INIT_LIST_HEAD(>head);
info->ident = ident;
info->checksum = func_checksum;
- info->use_extra_checksum = use_extra_checksum;
info->cfg_checksum = cfg_checksum;
list_add_tail(>head, _info->functions);
}
-EXPORT_SYMBOL(llvm_gcda_emit_function);
#endif
+EXPORT_SYMBOL(llvm_gcda_emit_function);
void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
{
@@ -293,11 +292,16 @@ int gcov_info_is_compatible(struct gcov_info *info1,
struct gcov_info *info2)
!list_is_last(_ptr2->head, >functions)) {
if (fn_ptr1->checksum != fn_ptr2->checksum)
return false;
+#if CONFIG_CLANG_VERSION < 11
if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
return false;
if (fn_ptr1->use_extra_checksum &&
fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
return false;
+#else
+ if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
+ return false;
+#endif
fn_ptr1 = list_next_entry(fn_ptr1, head);
fn_ptr2 = list_next_entry(fn_ptr2, head);
}
@@ -529,17 +533,22 @@ static size_t convert_to_gcda(char *buffer, struct
gcov_info *info)
list_for_each_entry(fi_ptr, >functions, head) {
u32 i;
- u32 len = 2;
-
- if (fi_ptr->use_extra_checksum)
- len++;
pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
- pos += store_gcov_u32(buffer, pos, len);
+#if CONFIG_CLANG_VERSION < 11
+ pos += store_gcov_u32(buffer, pos,
+ fi_ptr->use_extra_checksum ? 3 : 2);
+#else
+ pos += store_gcov_u32(buffer, pos, 3);
+#endif
pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
+#if CONFIG_CLANG_VERSION < 11
if (fi_ptr->use_extra_checksum)
pos += store_gcov_u32(buffer, pos,
fi_ptr->cfg_checksum);
+#else
+ pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
+#endif
pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
base-commit: 3fb4f979b4fa1f92a02b538ae86e725b73e703d0
--
2.31.1.295.g9ea45b61b8-goog
Re: [PATCH v4] lib/string: Introduce sysfs_streqcase
On Thu, Apr 8, 2021 at 7:52 AM Gioh Kim wrote: > > On Thu, Apr 8, 2021 at 3:14 PM Jinpu Wang wrote: > > > > On Thu, Apr 8, 2021 at 3:06 PM Gioh Kim wrote: > > > > > > As the name shows, it checks if strings are equal in case insensitive > > > manner. > > > > > > For example, drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c uses > > > strncasecmp to check that the input via sysfs is "mi". But it would > > > work even-if the input is "min-wrongcommand". > > > > > > I found some more cases using strncasecmp to check the entire string > > > such as rtrs-clt-sysfs.c does. drivers/pnp/interface.c checks > > > "disable" command with strncasecmp but it would also work if the > > > command is "disable-wrong". > > > > > > Signed-off-by: Gioh Kim v4 LGTM, thanks. Reviewed-by: Nick Desaulniers > > you should add the > > Reported-by: kernel test robot > > > --- > > you can add the changelog here after the --- > > v4->v3: removed #ifdef CONFIG_SYSFS ~ #endif. > > > > The string comparison doesn't depends on CONFIG_SYSFS at all. > > > > It looks good to me. > > Reviewed-by: Jack Wang > > > > > > Yes, I got two build error reports for v3. > Should I send v5 including "Reported-by: kernel test robot " > tag? I don't think that's necessary. I would use that tag if I was fixing an issue reported by the bot; but v4 is basically the same as v2 in regards to the issue 0day bot reported with v3. v3 just demonstrates that there are drivers with possibly incorrect Kconfig dependencies (missing a dependency on SYSFS perhaps). So the underlying problem was not reported by 0day bot; 0day bot just helped avoid issues from v3. Fixing the Kconfig dependencies would be nice to have, but not a requirement IMO to this feature/functionality. -- Thanks, ~Nick Desaulniers
Re: [PATCH] x86/kernel: remove unneeded dead-store initialization
On Wed, Apr 7, 2021 at 12:07 PM Borislav Petkov wrote: > > On Wed, Apr 07, 2021 at 09:03:28PM +0200, Borislav Petkov wrote: > > On Wed, Apr 07, 2021 at 10:41:26AM -0700, Nick Desaulniers wrote: > > > You do have clang-tidy installed right? `which clang-tidy`? > > Btw, for user convenience, that "clang-analyzer" Makefile target could > check for the presence of clang-tidy and fail if it is not there, with > an informative error message. Methinks. Yes, that's a good idea; we had a similar discussion recently about what happens if you enable CONFIG_DEBUG_INFO_BTF and don't have pahole installed. This is very much in the same vein. I've filed https://github.com/ClangBuiltLinux/linux/issues/1342 to follow up on. Should be a good beginner bug for folks looking to get started contributing. -- Thanks, ~Nick Desaulniers
Re: [PATCH 1/2] gcov: re-fix clang-11+ support
On Wed, Apr 7, 2021 at 2:21 PM Andrew Morton wrote:
>
> On Wed, 7 Apr 2021 11:54:55 -0700 Nick Desaulniers
> wrote:
>
> > LLVM changed the expected function signature for
> > llvm_gcda_emit_function() in the clang-11 release. Users of clang-11 or
> > newer may have noticed their kernels producing invalid coverage
> > information:
> >
> > $ llvm-cov gcov -a -c -u -f -b .gcda -- gcno=.gcno
> > 1 : checksum mismatch, \
> > (, ) != (, )
> > 2 Invalid .gcda File!
> > ...
> >
> > Fix up the function signatures so calling this function interprets its
> > parameters correctly and computes the correct cfg checksum. In
> > particular, in clang-11, the additional checksum is no longer optional.
>
> Which tree is this against? I'm seeing quite a lot of rejects against
> Linus's current.
Today's linux-next; the only recent changes to this single source file
since my last patches were:
commit b3c4e66c908b ("gcov: combine common code")
commit 17d0508a080d ("gcov: use kvmalloc()")
both have your sign off, so I assume those are in your tree?
--
Thanks,
~Nick Desaulniers
Re: [PATCH] mailmap: update Andrey Konovalov's email address
err..with Marco's real email address this time On Wed, Apr 7, 2021 at 1:14 PM Nick Desaulniers wrote: > > (replying to > https://lore.kernel.org/lkml/ead0e9c32a2f70e0bde6f63b3b9470e0ef13d2ee.1616107969.git.andreyk...@google.com/) > > Just got the bounceback, RIP. :( > > Marco is updating your epitaph. > > Acked-by: Nick Desaulniers -- Thanks, ~Nick Desaulniers
Re: [PATCH] mailmap: update Andrey Konovalov's email address
(replying to https://lore.kernel.org/lkml/ead0e9c32a2f70e0bde6f63b3b9470e0ef13d2ee.1616107969.git.andreyk...@google.com/) Just got the bounceback, RIP. :( Marco is updating your epitaph. Acked-by: Nick Desaulniers
Re: [PATCH] lib/string: Introduce sysfs_streqcase
On Tue, Apr 6, 2021 at 11:15 PM Gioh Kim wrote:
>
> As the name shows, it checks if strings are equal in case insensitive
> manner.
>
> For example, drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c uses
> strncasecmp to check that the input via sysfs is "mi". But it would
> work even-if the input is "min-wrongcommand".
>
> I found some more cases using strncasecmp to check the entire string
> such as rtrs-clt-sysfs.c does. drivers/pnp/interface.c checks
> "disable" command with strncasecmp but it would also work if the
> command is "disable-wrong".
Reviewed-by: Nick Desaulniers
I do wonder if these (sysfs_streqcase and sysfs_streq) could or should
be conditionally available on CONFIG_SYSFS=y; don't pay for what you
don't use (without needing CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y)?
Also, it might be nice to share the second half of the function with
sysfs_streq via a new static function. Though it will just get
inlined in both for CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y, it might
help the compiler if CONFIG_CC_OPTIMIZE_FOR_SIZE=y was instead chosen
if the compiler cannot outline/deduplicate the shared code. At the
least, there's less duplication between two very similar functions; if
one changes then authors may need to be careful to update both.
Are either of those concerns worth a v3? ¯\_(ツ)_/¯
>
> Signed-off-by: Gioh Kim
> ---
> include/linux/string.h | 1 +
> lib/string.c | 23 +++
> 2 files changed, 24 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 4fcfb56abcf5..36d00ff8013e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -184,6 +184,7 @@ extern char **argv_split(gfp_t gfp, const char *str, int
> *argcp);
> extern void argv_free(char **argv);
>
> extern bool sysfs_streq(const char *s1, const char *s2);
> +extern bool sysfs_streqcase(const char *s1, const char *s2);
> extern int kstrtobool(const char *s, bool *res);
> static inline int strtobool(const char *s, bool *res)
> {
> diff --git a/lib/string.c b/lib/string.c
> index 7548eb715ddb..5e6bc0d3d5c6 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -714,6 +714,29 @@ bool sysfs_streq(const char *s1, const char *s2)
> }
> EXPORT_SYMBOL(sysfs_streq);
>
> +/**
> + * sysfs_streqcase - same to sysfs_streq and case insensitive
> + * @s1: one string
> + * @s2: another string
> + *
> + */
> +bool sysfs_streqcase(const char *s1, const char *s2)
> +{
> + while (*s1 && tolower(*s1) == tolower(*s2)) {
> + s1++;
> + s2++;
> + }
> +
> + if (*s1 == *s2)
> + return true;
> + if (!*s1 && *s2 == '\n' && !s2[1])
> + return true;
> + if (*s1 == '\n' && !s1[1] && !*s2)
> + return true;
> + return false;
> +}
> +EXPORT_SYMBOL(sysfs_streqcase);
> +
> /**
> * match_string - matches given string in an array
> * @array: array of strings
> --
> 2.25.1
>
--
Thanks,
~Nick Desaulniers
[PATCH 2/2] gcov: re-drop support for clang-10
LLVM changed the expected function signatures for
llvm_gcda_emit_function() in the clang-11 release. Drop the older
implementations and require folks to upgrade their compiler if they're
interested in GCOV support.
Signed-off-by: Nick Desaulniers
---
kernel/gcov/clang.c | 40
1 file changed, 40 deletions(-)
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index 1747204541bf..78c4dc751080 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -69,9 +69,6 @@ struct gcov_fn_info {
u32 ident;
u32 checksum;
-#if CONFIG_CLANG_VERSION < 11
- u8 use_extra_checksum;
-#endif
u32 cfg_checksum;
u32 num_counters;
@@ -113,23 +110,6 @@ void llvm_gcda_start_file(const char *orig_filename, u32
version, u32 checksum)
}
EXPORT_SYMBOL(llvm_gcda_start_file);
-#if CONFIG_CLANG_VERSION < 11
-void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
- u8 use_extra_checksum, u32 cfg_checksum)
-{
- struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
-
- if (!info)
- return;
-
- INIT_LIST_HEAD(>head);
- info->ident = ident;
- info->checksum = func_checksum;
- info->use_extra_checksum = use_extra_checksum;
- info->cfg_checksum = cfg_checksum;
- list_add_tail(>head, _info->functions);
-}
-#else
void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
{
struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
@@ -143,7 +123,6 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
u32 cfg_checksum)
info->cfg_checksum = cfg_checksum;
list_add_tail(>head, _info->functions);
}
-#endif
EXPORT_SYMBOL(llvm_gcda_emit_function);
void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
@@ -274,16 +253,8 @@ int gcov_info_is_compatible(struct gcov_info *info1,
struct gcov_info *info2)
!list_is_last(_ptr2->head, >functions)) {
if (fn_ptr1->checksum != fn_ptr2->checksum)
return false;
-#if CONFIG_CLANG_VERSION < 11
- if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
- return false;
- if (fn_ptr1->use_extra_checksum &&
- fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
- return false;
-#else
if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
return false;
-#endif
fn_ptr1 = list_next_entry(fn_ptr1, head);
fn_ptr2 = list_next_entry(fn_ptr2, head);
}
@@ -403,21 +374,10 @@ size_t convert_to_gcda(char *buffer, struct gcov_info
*info)
u32 i;
pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
-#if CONFIG_CLANG_VERSION < 11
- pos += store_gcov_u32(buffer, pos,
- fi_ptr->use_extra_checksum ? 3 : 2);
-#else
pos += store_gcov_u32(buffer, pos, 3);
-#endif
pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
-#if CONFIG_CLANG_VERSION < 11
- if (fi_ptr->use_extra_checksum)
- pos += store_gcov_u32(buffer, pos,
fi_ptr->cfg_checksum);
-#else
pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
-#endif
-
pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
for (i = 0; i < fi_ptr->num_counters; i++)
--
2.31.1.295.g9ea45b61b8-goog
[PATCH 1/2] gcov: re-fix clang-11+ support
LLVM changed the expected function signature for
llvm_gcda_emit_function() in the clang-11 release. Users of clang-11 or
newer may have noticed their kernels producing invalid coverage
information:
$ llvm-cov gcov -a -c -u -f -b .gcda -- gcno=.gcno
1 : checksum mismatch, \
(, ) != (, )
2 Invalid .gcda File!
...
Fix up the function signatures so calling this function interprets its
parameters correctly and computes the correct cfg checksum. In
particular, in clang-11, the additional checksum is no longer optional.
Link: https://reviews.llvm.org/rG25544ce2df0daa4304c07e64b9c8b0f7df60c11d
Cc: sta...@vger.kernel.org #5.4+
Reported-by: Prasad Sodagudi
Tested-by: Prasad Sodagudi
Signed-off-by: Nick Desaulniers
---
kernel/gcov/clang.c | 38 +-
1 file changed, 33 insertions(+), 5 deletions(-)
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index d41f5ecda9db..1747204541bf 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -69,7 +69,9 @@ struct gcov_fn_info {
u32 ident;
u32 checksum;
+#if CONFIG_CLANG_VERSION < 11
u8 use_extra_checksum;
+#endif
u32 cfg_checksum;
u32 num_counters;
@@ -111,6 +113,7 @@ void llvm_gcda_start_file(const char *orig_filename, u32
version, u32 checksum)
}
EXPORT_SYMBOL(llvm_gcda_start_file);
+#if CONFIG_CLANG_VERSION < 11
void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
u8 use_extra_checksum, u32 cfg_checksum)
{
@@ -126,6 +129,21 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
info->cfg_checksum = cfg_checksum;
list_add_tail(>head, _info->functions);
}
+#else
+void llvm_gcda_emit_function(u32 ident, u32 func_checksum, u32 cfg_checksum)
+{
+ struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
+
+ if (!info)
+ return;
+
+ INIT_LIST_HEAD(>head);
+ info->ident = ident;
+ info->checksum = func_checksum;
+ info->cfg_checksum = cfg_checksum;
+ list_add_tail(>head, _info->functions);
+}
+#endif
EXPORT_SYMBOL(llvm_gcda_emit_function);
void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
@@ -256,11 +274,16 @@ int gcov_info_is_compatible(struct gcov_info *info1,
struct gcov_info *info2)
!list_is_last(_ptr2->head, >functions)) {
if (fn_ptr1->checksum != fn_ptr2->checksum)
return false;
+#if CONFIG_CLANG_VERSION < 11
if (fn_ptr1->use_extra_checksum != fn_ptr2->use_extra_checksum)
return false;
if (fn_ptr1->use_extra_checksum &&
fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
return false;
+#else
+ if (fn_ptr1->cfg_checksum != fn_ptr2->cfg_checksum)
+ return false;
+#endif
fn_ptr1 = list_next_entry(fn_ptr1, head);
fn_ptr2 = list_next_entry(fn_ptr2, head);
}
@@ -378,17 +401,22 @@ size_t convert_to_gcda(char *buffer, struct gcov_info
*info)
list_for_each_entry(fi_ptr, >functions, head) {
u32 i;
- u32 len = 2;
-
- if (fi_ptr->use_extra_checksum)
- len++;
pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
- pos += store_gcov_u32(buffer, pos, len);
+#if CONFIG_CLANG_VERSION < 11
+ pos += store_gcov_u32(buffer, pos,
+ fi_ptr->use_extra_checksum ? 3 : 2);
+#else
+ pos += store_gcov_u32(buffer, pos, 3);
+#endif
pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
pos += store_gcov_u32(buffer, pos, fi_ptr->checksum);
+#if CONFIG_CLANG_VERSION < 11
if (fi_ptr->use_extra_checksum)
pos += store_gcov_u32(buffer, pos,
fi_ptr->cfg_checksum);
+#else
+ pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
+#endif
pos += store_gcov_u32(buffer, pos, GCOV_TAG_COUNTER_BASE);
pos += store_gcov_u32(buffer, pos, fi_ptr->num_counters * 2);
--
2.31.1.295.g9ea45b61b8-goog
[PATCH 0/2] gcov: re-fix clang-11 support
LLVM changed the expected function signature for llvm_gcda_emit_function() in the clang-11 release. Users of clang-11 or newer may have noticed their kernels producing invalid coverage information: $ llvm-cov gcov -a -c -u -f -b .gcda -- gcno=.gcno 1 : checksum mismatch, \ (, ) != (, ) 2 Invalid .gcda File! ... Similar to the last series, the patch is broken in two. The first is tagged for inclusion in stable in order to continue supporting newer versions of clang (clang-11+) for that tree, then the second drops the older implementations to keep one and only support clang-11+. This same pattern was done recently in: https://lore.kernel.org/lkml/20210312224132.3413602-1-ndesaulni...@google.com/ We've since added CI coverage of CONFIG_GCOV https://github.com/ClangBuiltLinux/continuous-integration2/pull/107 but need to find a better way to test validating the coverage info in userspace. Nick Desaulniers (2): gcov: re-fix clang-11+ support gcov: re-drop support for clang-10 kernel/gcov/clang.c | 20 1 file changed, 4 insertions(+), 16 deletions(-) -- 2.31.1.295.g9ea45b61b8-goog
Re: arm-linux-gnueabi-ld: warning: orphan section `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' being placed in section `__cpuidle_method_of_table'
On Wed, Apr 7, 2021 at 11:23 AM Arnd Bergmann wrote: > > On Wed, Apr 7, 2021 at 8:07 PM Nick Desaulniers > wrote: > > > > On Wed, Apr 7, 2021 at 3:52 AM kernel test robot wrote: > > > > > > Hi Kees, > > > > > > FYI, the error/warning still remains. > > > > > > tree: > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > > head: 2d743660786ec51f5c1fefd5782bbdee7b227db0 > > > commit: 5a17850e251a55bba6d65aefbfeacfa9888cd2cd arm/build: Warn on > > > orphan section placement > > > date: 7 months ago > > > config: arm-randconfig-r033-20210407 (attached as .config) > > > compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 > > > reproduce (this is a W=1 build): > > > wget > > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross > > > -O ~/bin/make.cross > > > chmod +x ~/bin/make.cross > > > # > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5a17850e251a55bba6d65aefbfeacfa9888cd2cd > > > git remote add linus > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > > git fetch --no-tags linus master > > > git checkout 5a17850e251a55bba6d65aefbfeacfa9888cd2cd > > > # save the attached .config to linux build tree > > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > > > ARCH=arm > > > > > > If you fix the issue, kindly add following tag as appropriate > > > Reported-by: kernel test robot > > > > > > All warnings (new ones prefixed by >>): > > > > > > >> arm-linux-gnueabi-ld: warning: orphan section > > > >> `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' > > > >> being placed in section `__cpuidle_method_of_table' > > > >> arm-linux-gnueabi-ld: warning: orphan section > > > >> `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' > > > >> being placed in section `__cpuidle_method_of_table' > > > >> arm-linux-gnueabi-ld: warning: orphan section > > > >> `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' > > > >> being placed in section `__cpuidle_method_of_table' > > > > Looks like arch/arm/include/asm/cpuidle.h defines > > `CPUIDLE_METHOD_OF_DECLARE` to create a static struct in such a > > section. Only arch/arm/mach-omap2/pm33xx-core.c uses that macro. > > Nick, Kees, > > Should I resend my patch, or are you taking care of it? > > https://lore.kernel.org/lkml/20201230155506.1085689-1-a...@kernel.org/T/#u Your patch looks like it has multiple reviewed-by tags, so it should be ready to be submitted/merged? Waiting on anything else? -- Thanks, ~Nick Desaulniers
Re: arm-linux-gnueabi-ld: warning: orphan section `__cpuidle_method_of_table' from `arch/arm/mach-omap2/pm33xx-core.o' being placed in section `__cpuidle_method_of_table'
On Wed, Apr 7, 2021 at 3:52 AM kernel test robot wrote: > > Hi Kees, > > FYI, the error/warning still remains. > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > master > head: 2d743660786ec51f5c1fefd5782bbdee7b227db0 > commit: 5a17850e251a55bba6d65aefbfeacfa9888cd2cd arm/build: Warn on orphan > section placement > date: 7 months ago > config: arm-randconfig-r033-20210407 (attached as .config) > compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5a17850e251a55bba6d65aefbfeacfa9888cd2cd > git remote add linus > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > git fetch --no-tags linus master > git checkout 5a17850e251a55bba6d65aefbfeacfa9888cd2cd > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > ARCH=arm > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All warnings (new ones prefixed by >>): > > >> arm-linux-gnueabi-ld: warning: orphan section `__cpuidle_method_of_table' > >> from `arch/arm/mach-omap2/pm33xx-core.o' being placed in section > >> `__cpuidle_method_of_table' > >> arm-linux-gnueabi-ld: warning: orphan section `__cpuidle_method_of_table' > >> from `arch/arm/mach-omap2/pm33xx-core.o' being placed in section > >> `__cpuidle_method_of_table' > >> arm-linux-gnueabi-ld: warning: orphan section `__cpuidle_method_of_table' > >> from `arch/arm/mach-omap2/pm33xx-core.o' being placed in section > >> `__cpuidle_method_of_table' Looks like arch/arm/include/asm/cpuidle.h defines `CPUIDLE_METHOD_OF_DECLARE` to create a static struct in such a section. Only arch/arm/mach-omap2/pm33xx-core.c uses that macro. > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org -- Thanks, ~Nick Desaulniers
Re: [PATCH] x86/kernel: remove unneeded dead-store initialization
On Wed, Apr 7, 2021 at 5:02 AM Borislav Petkov wrote: > > On Wed, Mar 31, 2021 at 04:00:24PM +0800, Yang Li wrote: > > make clang-analyzer on x86_64 defconfig caught my attention with: > > I can't trigger this here using: > > make CC=clang-11 -j16 clang-analyzer > > I get all kinds of missing python scripts: > FileNotFoundError: [Errno 2] No such file or directory: 'clang-tidy' You do have clang-tidy installed right? `which clang-tidy`? -- Thanks, ~Nick Desaulniers
[PATCH] MIPS: select ARCH_KEEP_MEMBLOCK unconditionally
While removing allnoconfig_y from Kconfig, ARCH=mips allnoconfig builds
started failing with the error:
WARNING: modpost: vmlinux.o(.text+0x9c70): Section mismatch in reference
from the function reserve_exception_space() to the function
.meminit.text:memblock_reserve()
The function reserve_exception_space() references the function __meminit
memblock_reserve().
This is often because reserve_exception_space lacks a __meminit
annotation or the annotation of memblock_reserve is wrong.
ERROR: modpost: Section mismatches detected.
Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.
allnoconfig disables DEBUG_KERNEL and thus ARCH_KEEP_MEMBLOCK, which
changes __init_memblock to be equivalent to __meminit triggering the
above error.
Link:
https://lore.kernel.org/linux-kbuild/20210313194836.372585-11-masahi...@kernel.org/
Fixes: commit a8c0f1c634507 ("MIPS: Select ARCH_KEEP_MEMBLOCK if
DEBUG_KERNEL to enable sysfs memblock debug")
Cc: Masahiro Yamada
Reported-by: Guenter Roeck
Signed-off-by: Nick Desaulniers
---
arch/mips/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index e9893cd34992..702648f60e41 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -12,7 +12,7 @@ config MIPS
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARCH_HAS_GCOV_PROFILE_ALL
- select ARCH_KEEP_MEMBLOCK if DEBUG_KERNEL
+ select ARCH_KEEP_MEMBLOCK
select ARCH_SUPPORTS_UPROBES
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF if 64BIT
--
2.31.1.295.g9ea45b61b8-goog
Re: Usage of CXX in tools directory
On Sun, Apr 4, 2021 at 8:00 AM Sedat Dilek wrote: > > [ Please CC me I am not subscribed to all mailing-lists ] > [ Please CC some more folks if you like ] > > Hi, > > when dealing/experimenting with BPF together with pahole/dwarves and > dwarf-v5 and clang-lto I fell over that there is usage of CXX in tools > directory. > Especially, I wanted to build and run test_progs from BPF selftests. > One BPF selftest called "test_cpp" used GNU/g++ (and even /usr/bin/ld) > and NOT LLVM/clang++. > > For details see the linux-bpf/dwarves thread "[PATCH dwarves] > dwarf_loader: handle DWARF5 DW_OP_addrx properly" in [1]. > > Lemme check: > > $ git grep CXX tools/ > tools/build/Build.include:cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ > $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXXFLAGS_$(basetarget).o) > $(CXXFLAGS_$(obj)) > tools/build/Makefile.build:quiet_cmd_cxx_o_c = CXX $@ > tools/build/Makefile.build: cmd_cxx_o_c = $(CXX) $(cxx_flags) -c -o $@ $< > tools/build/Makefile.feature: feature-$(1) := $(shell $(MAKE) > OUTPUT=$(OUTPUT_FEATURES) CC="$(CC)" CXX="$(CXX)" > CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" > CXXFLAGS="$(EXTRA_CXXFLAGS) $(FEATURE_CHECK_CXXFLAGS-$(1))" > LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C $(feature_dir) > $(OUTPUT_FEATURES)test-$1.bin >/dev/nu > ll 2>/dev/null && echo 1 || echo 0) > tools/build/feature/Makefile:__BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall > -Werror -o $@ $(patsubst %.bin,%.cpp,$(@F)) $(LDFLAGS) > ... > tools/perf/Makefile.config:USE_CXX = 0 > tools/perf/Makefile.config:CXXFLAGS += > -DHAVE_LIBCLANGLLVM_SUPPORT -I$(shell $(LLVM_CONFIG) --includedir) > tools/perf/Makefile.config:$(call detected,CONFIG_CXX) > tools/perf/Makefile.config: USE_CXX = 1 > tools/perf/Makefile.perf:export srctree OUTPUT RM CC CXX LD AR CFLAGS > CXXFLAGS V BISON FLEX AWK > tools/perf/Makefile.perf:ifeq ($(USE_CXX), 1) > tools/perf/util/Build:perf-$(CONFIG_CXX) += c++/ > ... > tools/scripts/Makefile.include:$(call allow-override,CXX,$(CROSS_COMPILE)g++) > ... > tools/testing/selftests/bpf/Makefile:CXX ?= $(CROSS_COMPILE)g++ > tools/testing/selftests/bpf/Makefile: $(call msg,CXX,,$@) > tools/testing/selftests/bpf/Makefile: $(Q)$(CXX) $(CFLAGS) $^ $(LDLIBS) -o > $@ > > The problem is if you pass LLVM=1 there is no clang(++) assigned to > CXX automagically. > > [2] says: > > LLVM has substitutes for GNU binutils utilities. Kbuild supports > LLVM=1 to enable them. > > make LLVM=1 > They can be enabled individually. The full list of the parameters: > > make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \ > OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \ > HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld > > [ EndOfQuote ] > > So you need to pass CXX=clang++ manually when playing in tools directory: Yes, CXX is not set by LLVM=1 in the top level Makefile. CXX is not exported by the top level Makefile. I suspect that tools/scripts/Mafefile.include (and possible testing/selftests/bpf/Makefile) needs to check for LLVM=1 and set CXX=clang++ explicitly. > > MAKE="make V=1 > MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang > CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1" > MAKE_OPTS="MAKE_OPTS $PAHOLE=/opt/pahole/bin/pahole" > > $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ clean > $ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/ > > Unsure, if tools needs a special treatment in things of CXX or LLVM=1 > needs to be enhanced with CCX=clang++. > If we have HOSTCXX why not have a CXX in toplevel Makefile? > > In "tools: Factor Clang, LLC and LLVM utils definitions" (see [3]) I > did some factor-ing. > > For the records: Here Linus Git is my base. > > Ideas? > > Thanks. > > Regards, > - Sedat - > > P.S.: Just a small note: I know there is less usage of CXX code in the > linux-kernel. > > [1] > https://lore.kernel.org/bpf/CA+icZUWh6YOkCKG72SndqUbQNwG+iottO4=cPyRRVjaHD2=0...@mail.gmail.com/T/#m22907f838d2d27be24e8959a53473a62f21cecea > [2] https://www.kernel.org/doc/html/latest/kbuild/llvm.html#llvm-utilities > [3] https://git.kernel.org/linus/211a741cd3e124bffdc13ee82e7e65f204e53f60 -- Thanks, ~Nick Desaulniers
Re: [PATCH] lib/string: Introduce sysfs_streqcase
Thanks for the patch!
+ akpm (please remember to run ./scripts/get_maintainer.pl on your patch files)
On Fri, Apr 2, 2021 at 2:41 AM Gioh Kim wrote:
>
> As the name shows, it checks if strings are equal in case insensitive
> manner. I found some cases using strncasecmp to check the entire
> strings and they would not work as intended.
>
> For example, drivers/infiniband/ulp/rtrs/rtrs-clt-sysfs.c uses
> strncasecmp to check that the input via sysfs is "mi". But it would
> work even-if the input is "min-wrongcommand".
> And also drivers/pnp/interface.c checks "disable" command with
> strncasecmp but it would also work if the command is "disable-wrong".
Perhaps those callers should be using strcasecmp then, rather than strncasecmp?
Also, if they're being liberal in accepting either case, I don't see
why the sysfs nodes should be strict in rejecting trailing input at
that point.
>
> Signed-off-by: Gioh Kim
> ---
> lib/string.c | 23 +++
> 1 file changed, 23 insertions(+)
>
> diff --git a/lib/string.c b/lib/string.c
> index 7548eb715ddb..5e6bc0d3d5c6 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -714,6 +714,29 @@ bool sysfs_streq(const char *s1, const char *s2)
> }
> EXPORT_SYMBOL(sysfs_streq);
>
> +/**
> + * sysfs_streqcase - same to sysfs_streq and case insensitive
> + * @s1: one string
> + * @s2: another string
> + *
> + */
> +bool sysfs_streqcase(const char *s1, const char *s2)
> +{
> + while (*s1 && tolower(*s1) == tolower(*s2)) {
> + s1++;
> + s2++;
> + }
> +
> + if (*s1 == *s2)
> + return true;
> + if (!*s1 && *s2 == '\n' && !s2[1])
> + return true;
> + if (*s1 == '\n' && !s1[1] && !*s2)
> + return true;
> + return false;
> +}
> +EXPORT_SYMBOL(sysfs_streqcase);
This should be declared in
include/linux/string.h
in order for others to use this (as 0day bot notes).
> +
> /**
> * match_string - matches given string in an array
> * @array: array of strings
> --
> 2.25.1
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH] usb: isp1301-omap: Add missing gpiod_add_lookup_table function
On Thu, Apr 1, 2021 at 9:21 AM Maciej Falkowski
wrote:
>
> The gpiod table was added without any usage making it unused
> as reported by Clang compilation from omap1_defconfig on linux-next:
>
> arch/arm/mach-omap1/board-h2.c:347:34: warning: unused variable
> 'isp1301_gpiod_table' [-Wunused-variable]
> static struct gpiod_lookup_table isp1301_gpiod_table = {
> ^
> 1 warning generated.
>
> The patch adds the missing gpiod_add_lookup_table() function.
>
> Signed-off-by: Maciej Falkowski
> Fixes: f3ef38160e3d ("usb: isp1301-omap: Convert to use GPIO descriptors")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1325
Looks consistent to me with other callers of gpiod_add_lookup_table
from .init_machine callbacks.
Reviewed-by: Nick Desaulniers
> ---
> arch/arm/mach-omap1/board-h2.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap1/board-h2.c b/arch/arm/mach-omap1/board-h2.c
> index c40cf5ef8607..977b0b744c22 100644
> --- a/arch/arm/mach-omap1/board-h2.c
> +++ b/arch/arm/mach-omap1/board-h2.c
> @@ -320,7 +320,7 @@ static int tps_setup(struct i2c_client *client, void
> *context)
> {
> if (!IS_BUILTIN(CONFIG_TPS65010))
> return -ENOSYS;
> -
> +
> tps65010_config_vregs1(TPS_LDO2_ENABLE | TPS_VLDO2_3_0V |
> TPS_LDO1_ENABLE | TPS_VLDO1_3_0V);
>
> @@ -394,6 +394,8 @@ static void __init h2_init(void)
> BUG_ON(gpio_request(H2_NAND_RB_GPIO_PIN, "NAND ready") < 0);
> gpio_direction_input(H2_NAND_RB_GPIO_PIN);
>
> + gpiod_add_lookup_table(_gpiod_table);
> +
> omap_cfg_reg(L3_1610_FLASH_CS2B_OE);
> omap_cfg_reg(M8_1610_FLASH_CS2B_WE);
>
> --
--
Thanks,
~Nick Desaulniers
Re: [PATCH] ARM: OMAP: Fix use of possibly uninitialized irq variable
On Thu, Apr 1, 2021 at 9:12 AM Maciej Falkowski
wrote:
>
> The current control flow of IRQ number assignment to `irq` variable
> allows a request of IRQ of unspecified value,
> generating a warning under Clang compilation with omap1_defconfig on
> linux-next:
>
> arch/arm/mach-omap1/pm.c:656:11: warning: variable 'irq' is used
> uninitialized whenever
> 'if' condition is false [-Wsometimes-uninitialized]
> else if (cpu_is_omap16xx())
> ^
> ./arch/arm/mach-omap1/include/mach/soc.h:123:30: note: expanded from macro
> 'cpu_is_omap16xx'
> ^
> arch/arm/mach-omap1/pm.c:658:18: note: uninitialized use occurs here
> if (request_irq(irq, omap_wakeup_interrupt, 0, "peripheral wakeup",
> ^~~
> arch/arm/mach-omap1/pm.c:656:7: note: remove the 'if' if its condition is
> always true
> else if (cpu_is_omap16xx())
> ^~
> arch/arm/mach-omap1/pm.c:611:9: note: initialize the variable 'irq' to
> silence this warning
> int irq;
>^
> = 0
> 1 warning generated.
Ooh, yeah if cpu_is_omap15xx() then irq is unused uninitialized; I
don't see any INT_1610_WAKE_UP_REQ-equlivalent for
INT_15XX_WAKE_UP_REQ.
Ok, LGTM.
Reviewed-by: Nick Desaulniers
I agree with Nathan on the Fixes tag.
>
> The patch provides a default value to the `irq` variable
> along with a validity check.
>
> Signed-off-by: Maciej Falkowski
> Link: https://github.com/ClangBuiltLinux/linux/issues/1324
> ---
> arch/arm/mach-omap1/pm.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
> index 2c1e2b32b9b3..a745d64d4699 100644
> --- a/arch/arm/mach-omap1/pm.c
> +++ b/arch/arm/mach-omap1/pm.c
> @@ -655,9 +655,13 @@ static int __init omap_pm_init(void)
> irq = INT_7XX_WAKE_UP_REQ;
> else if (cpu_is_omap16xx())
> irq = INT_1610_WAKE_UP_REQ;
> - if (request_irq(irq, omap_wakeup_interrupt, 0, "peripheral wakeup",
> - NULL))
> - pr_err("Failed to request irq %d (peripheral wakeup)\n", irq);
> + else
> + irq = -1;
> +
> + if (irq >= 0) {
> + if (request_irq(irq, omap_wakeup_interrupt, 0, "peripheral
> wakeup", NULL))
> + pr_err("Failed to request irq %d (peripheral
> wakeup)\n", irq);
> + }
>
> /* Program new power ramp-up time
> * (0 for most boards since we don't lower voltage when in deep sleep)
> --
--
Thanks,
~Nick Desaulniers
Re: [PATCH] ARM: OMAP1: ams-delta: remove unused function ams_delta_camera_power
On Thu, Apr 1, 2021 at 9:05 AM Maciej Falkowski
wrote:
>
> The ams_delta_camera_power() function is unused as reports
> Clang compilation with omap1_defconfig on linux-next:
>
> arch/arm/mach-omap1/board-ams-delta.c:462:12: warning: unused function
> 'ams_delta_camera_power' [-Wunused-function]
> static int ams_delta_camera_power(struct device *dev, int power)
>^
> 1 warning generated.
>
> The soc_camera support was dropped without removing
> ams_delta_camera_power() function, making it unused.
>
> Signed-off-by: Maciej Falkowski
Thanks for the patch!
Reviewed-by: Nick Desaulniers
> Fixes: ce548396a433 ("media: mach-omap1: board-ams-delta.c: remove soc_camera
> dependencies")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1326
> ---
> arch/arm/mach-omap1/board-ams-delta.c | 14 --
> 1 file changed, 14 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c
> b/arch/arm/mach-omap1/board-ams-delta.c
> index 2ee527c00284..1026a816dcc0 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -458,20 +458,6 @@ static struct gpiod_lookup_table leds_gpio_table = {
>
> #ifdef CONFIG_LEDS_TRIGGERS
> DEFINE_LED_TRIGGER(ams_delta_camera_led_trigger);
> -
> -static int ams_delta_camera_power(struct device *dev, int power)
> -{
> - /*
> -* turn on camera LED
> -*/
> - if (power)
> - led_trigger_event(ams_delta_camera_led_trigger, LED_FULL);
> - else
> - led_trigger_event(ams_delta_camera_led_trigger, LED_OFF);
> - return 0;
> -}
> -#else
> -#define ams_delta_camera_power NULL
> #endif
>
> static struct platform_device ams_delta_audio_device = {
> --
> 2.26.3
>
> --
> You received this message because you are subscribed to the Google Groups
> "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to clang-built-linux+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/clang-built-linux/20210401160434.7655-1-maciej.falkowski9%40gmail.com.
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 2/3] kbuild: check the minimum assembler version in Kconfig
# it has been already checked by
> > scripts/cc-version.sh.
> > + echo LLVM 0
> > + exit 0
> > + fi
> > + shift
> > + done
> > +}
> > +
> > +check_integrated_as "$@"
> > +
> > +orig_args="$@"
> > +
> > +# Get the first line of the --version output.
> > +IFS='
> > +'
> > +set -- $(LC_ALL=C "$@" -Wa,--version -c -x assembler /dev/null -o
> > /dev/null 2>&1)
> > +
> > +# Split the line on spaces.
> > +IFS=' '
> > +set -- $1
> > +
> > +min_tool_version=$(dirname $0)/min-tool-version.sh
> > +
> > +if [ "$1" = GNU -a "$2" = assembler ]; then
> > + shift $(($# - 1))
> > + version=$1
> > + min_version=$($min_tool_version binutils)
> > + name=GNU
> > +else
> > + echo "$orig_args: unknown assembler invoked" >&2
> > + exit 1
> > +fi
> > +
> > +# Some distributions append a package release number, as in 2.34-4.fc32
> > +# Trim the hyphen and any characters that follow.
> > +version=${version%-*}
> > +
> > +cversion=$(get_canonical_version $version)
> > +min_cversion=$(get_canonical_version $min_version)
> > +
> > +if [ "$cversion" -lt "$min_cversion" ]; then
> > + echo >&2 "***"
> > + echo >&2 "*** Assembler is too old."
> > + echo >&2 "*** Your $name assembler version:$version"
> > + echo >&2 "*** Minimum $name assembler version: $min_version"
> > + echo >&2 "***"
> > + exit 1
> > +fi
> > +
> > +echo $name $cversion
> > --
> > 2.27.0
> >
> > --
--
Thanks,
~Nick Desaulniers
Re: [PATCH] blk-mq: fix alignment mismatch.
On Wed, Mar 31, 2021 at 2:58 PM Nathan Chancellor wrote: > > On Wed, Mar 31, 2021 at 02:27:03PM -0700, Jian Cai wrote: > > > > I just realized you already proposed solutions for skipping the check > > in > > https://lore.kernel.org/linux-block/20210310225240.4epj2mdmzt4vurr3@archlinux-ax161/#t. > > Do you have any plans to send them for review? > > I was hoping to gather some feedback on which option would be preferred > by Jens and the other ClangBuiltLinux folks before I sent them along. I > can send the first just to see what kind of feedback I can gather. Either approach is fine by me. The smaller might be easier to get accepted into stable. The larger approach will probably become more useful in the future (having the diag infra work properly with clang). I think the ball is kind of in Jens' court to decide. Would doing both be appropriate, Jens? Have the smaller patch tagged for stable disabling it for the whole file, then another commit on top not tagged for stable that adds the diag infra, and a third on top replacing the file level warning disablement with local diags to isolate this down to one case? It's a fair but small amount of churn IMO; but if Jens is not opposed it seems fine? -- Thanks, ~Nick Desaulniers
Re: [PATCH 11/13] kconfig: do not use allnoconfig_y option
On Wed, Mar 31, 2021 at 10:12 AM Guenter Roeck wrote:
>
> On Sun, Mar 14, 2021 at 04:48:34AM +0900, Masahiro Yamada wrote:
> > allnoconfig_y is a bad hack that sets a symbol to 'y' by allnoconfig.
> >
> > allnoconfig does not mean a minimum set of CONFIG options because a
> > bunch of prompts are hidden by 'if EMBEDDED' or 'if EXPERT', but I do
> > not like to do a workaround this way.
> >
> > Use the pre-existing feature, KCONFIG_ALLCONFIG, to provide a one
> > liner config fragment. CONFIG_EMBEDDED=y is still forced under
> > allnoconfig.
> >
> > No change in the .config file produced by 'make tinyconfig'.
> >
> > The output of 'make allnoconfig' will be changed; we will get
> > CONFIG_EMBEDDED=n because allnoconfig literally sets all symbols to n.
> >
> > Signed-off-by: Masahiro Yamada
>
> With this patch in place, mips:allnoconfig fails to build with
> the following error.
>
> Error log:
> WARNING: modpost: vmlinux.o(.text+0x9c70): Section mismatch in reference from
> the function reserve_exception_space() to the function
> .meminit.text:memblock_reserve()
> The function reserve_exception_space() references
> the function __meminit memblock_reserve().
> This is often because reserve_exception_space lacks a __meminit
> annotation or the annotation of memblock_reserve is wrong.
> ERROR: modpost: Section mismatches detected.
> Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.
> make[2]: *** [scripts/Makefile.modpost:62: vmlinux.symvers] Error 1
> make[2]: *** Deleting file 'vmlinux.symvers'
> make[1]: *** [Makefile:1292: vmlinux] Error 2
> make: *** [Makefile:222: __sub-make] Error 2
Thanks for the report. I suspect this is related to allnoconfig
disabling CONFIG_ARCH_KEEP_MEMBLOCK, which changes the definition of
__init_memblock in include/linux/memblock.h.
So allnoconfig would unselect CONFIG_ARCH_KEEP_MEMBLOCK, making
__init_memblock equivalent to __meminit triggering the above warning.
arch/mips/Kconfig
14: select ARCH_KEEP_MEMBLOCK if DEBUG_KERNEL
so DEBUG_KERNEL is probably also disabled by allnoconfig.
commit a8c0f1c634507 ("MIPS: Select ARCH_KEEP_MEMBLOCK if DEBUG_KERNEL
to enable sysfs memblock debug")
probably should drop the `if DEBUG_KERNEL` part.
>
> Guenter
>
> > ---
> >
> > init/Kconfig| 1 -
> > kernel/configs/tiny-base.config | 1 +
> > scripts/kconfig/Makefile| 3 ++-
> > 3 files changed, 3 insertions(+), 2 deletions(-)
> > create mode 100644 kernel/configs/tiny-base.config
> >
> > diff --git a/init/Kconfig b/init/Kconfig
> > index 46b87ad73f6a..beb8314fdf96 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1769,7 +1769,6 @@ config DEBUG_RSEQ
> >
> > config EMBEDDED
> > bool "Embedded system"
> > - option allnoconfig_y
> > select EXPERT
> > help
> > This option should be enabled if compiling the kernel for
> > diff --git a/kernel/configs/tiny-base.config
> > b/kernel/configs/tiny-base.config
> > new file mode 100644
> > index ..2f0e6bf6db2c
> > --- /dev/null
> > +++ b/kernel/configs/tiny-base.config
> > @@ -0,0 +1 @@
> > +CONFIG_EMBEDDED=y
> > diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> > index 7df3c0e4c52e..46f2465177f0 100644
> > --- a/scripts/kconfig/Makefile
> > +++ b/scripts/kconfig/Makefile
> > @@ -102,7 +102,8 @@ configfiles=$(wildcard $(srctree)/kernel/configs/$@
> > $(srctree)/arch/$(SRCARCH)/c
> >
> > PHONY += tinyconfig
> > tinyconfig:
> > - $(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
> > + $(Q)KCONFIG_ALLCONFIG=kernel/configs/tiny-base.config $(MAKE) -f
> > $(srctree)/Makefile allnoconfig
> > + $(Q)$(MAKE) -f $(srctree)/Makefile tiny.config
> >
> > # CHECK: -o cache_dir= working?
> > PHONY += testconfig
> > --
> > 2.27.0
> >
--
Thanks,
~Nick Desaulniers
Re: [PATCH 9/9] kbuild: remove CONFIG_MODULE_COMPRESS
On Wed, Mar 31, 2021 at 6:39 AM Masahiro Yamada wrote: Should the online be Kconfig rather than Kbuild, for a commit that only changes Kconfigs? > > CONFIG_MODULE_COMPRESS is only used to activate the choice for module > compression algorithm. It will be simpler to make the choice visible > all the time by adding CONFIG_MODULE_COMPRESS_NONE to allow the user to > disable module compression. > > This is more consistent with the "Kernel compression mode" and "Built-in > initramfs compression mode" choices. > > CONFIG_KERNEL_UNCOMPRESSED and CONFIG_INITRAMFS_COMPRESSION_NONE are > available to choose to not compress the kernel, initrd, respectively. > > Signed-off-by: Masahiro Yamada > --- > > init/Kconfig | 45 ++--- > 1 file changed, 26 insertions(+), 19 deletions(-) > > diff --git a/init/Kconfig b/init/Kconfig > index 019c1874e609..3ca1ffd219c4 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -2225,40 +2225,47 @@ config MODULE_SIG_HASH > default "sha384" if MODULE_SIG_SHA384 > default "sha512" if MODULE_SIG_SHA512 > > -config MODULE_COMPRESS The top level Makefile has comments and code that refer to this choice which is now removed. I think you'll want to fix that up in this change as well? Ah, patch 7 in the series does that: https://lore.kernel.org/linux-kbuild/20210331133811.3221540-7-masahi...@kernel.org/ Ok then this LGTM. Reviewed-by: Nick Desaulniers > - bool "Compress modules on installation" > +choice > + prompt "Module compression mode" > help > + This option allows you to choose the algorithm which will be used to > + compress modules when 'make modules_install' is run. (or, you can > + choose to not compress modules at all.) > > - Compresses kernel modules when 'make modules_install' is run; gzip > or > - xz depending on "Compression algorithm" below. > + External modules will also be compressed in the same way during the > + installation. > > - module-init-tools MAY support gzip, and kmod MAY support gzip and > xz. > + For modules inside an initrd or initramfs, it's more efficient to > + compress the whole initrd or initramfs instead. > > - Out-of-tree kernel modules installed using Kbuild will also be > - compressed upon installation. > + This is fully compatible with signed modules. > > - Note: for modules inside an initrd or initramfs, it's more efficient > - to compress the whole initrd or initramfs instead. > + Please note that the tool used to load modules needs to support the > + corresponding algorithm. module-init-tools MAY support gzip, and > kmod > + MAY support gzip and xz. > > - Note: This is fully compatible with signed modules. > + Your build system needs to provide the appropriate compression tool > + to compress the modules. > > - If in doubt, say N. > + If in doubt, select 'None'. > > -choice > - prompt "Compression algorithm" > - depends on MODULE_COMPRESS > - default MODULE_COMPRESS_GZIP > +config MODULE_COMPRESS_NONE > + bool "None" > help > - This determines which sort of compression will be used during > - 'make modules_install'. > - > - GZIP (default) and XZ are supported. > + Do not compress modules. The installed modules are suffixed > + with .ko. > > config MODULE_COMPRESS_GZIP > bool "GZIP" > + help > + Compress modules with XZ. The installed modules are suffixed > + with .ko.gz. > > config MODULE_COMPRESS_XZ > bool "XZ" > + help > + Compress modules with XZ. The installed modules are suffixed > + with .ko.xz. > > endchoice > > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
Re: [PATCH 5/9] kbuild: rename extmod-prefix to extmod_prefix
On Wed, Mar 31, 2021 at 6:38 AM Masahiro Yamada wrote:
>
> This seems to be useful in sub-make as well. As a preparation of
> exporting it, rename extmod-prefix to extmod_prefix because exported
> variables cannot contain hyphens.
>
> Signed-off-by: Masahiro Yamada
Reviewed-by: Nick Desaulniers
> ---
>
> Makefile | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b5ff4753eba8..e3c2bd1b6f42 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -919,7 +919,7 @@ endif
> ifdef CONFIG_LTO_CLANG
> ifdef CONFIG_LTO_CLANG_THIN
> CC_FLAGS_LTO := -flto=thin -fsplit-lto-unit
> -KBUILD_LDFLAGS += --thinlto-cache-dir=$(extmod-prefix).thinlto-cache
> +KBUILD_LDFLAGS += --thinlto-cache-dir=$(extmod_prefix).thinlto-cache
> else
> CC_FLAGS_LTO := -flto
> endif
> @@ -1141,9 +1141,9 @@ endif # CONFIG_BPF
>
> PHONY += prepare0
>
> -extmod-prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
> -export MODORDER := $(extmod-prefix)modules.order
> -export MODULES_NSDEPS := $(extmod-prefix)modules.nsdeps
> +extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
> +export MODORDER := $(extmod_prefix)modules.order
> +export MODULES_NSDEPS := $(extmod_prefix)modules.nsdeps
>
> ifeq ($(KBUILD_EXTMOD),)
> core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
> @@ -1742,7 +1742,7 @@ build-dirs := $(KBUILD_EXTMOD)
> $(MODORDER): descend
> @:
>
> -compile_commands.json: $(extmod-prefix)compile_commands.json
> +compile_commands.json: $(extmod_prefix)compile_commands.json
> PHONY += compile_commands.json
>
> clean-dirs := $(KBUILD_EXTMOD)
> @@ -1832,12 +1832,12 @@ endif
>
> PHONY += single_modpost
> single_modpost: $(single-no-ko) modules_prepare
> - $(Q){ $(foreach m, $(single-ko), echo $(extmod-prefix)$m;) } >
> $(MODORDER)
> + $(Q){ $(foreach m, $(single-ko), echo $(extmod_prefix)$m;) } >
> $(MODORDER)
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
>
> KBUILD_MODULES := 1
>
> -export KBUILD_SINGLE_TARGETS := $(addprefix $(extmod-prefix),
> $(single-no-ko))
> +export KBUILD_SINGLE_TARGETS := $(addprefix $(extmod_prefix),
> $(single-no-ko))
>
> # trim unrelated directories
> build-dirs := $(foreach d, $(build-dirs), \
> @@ -1906,12 +1906,12 @@ nsdeps: modules
> quiet_cmd_gen_compile_commands = GEN $@
>cmd_gen_compile_commands = $(PYTHON3) $< -a $(AR) -o $@ $(filter-out
> $<, $(real-prereqs))
>
> -$(extmod-prefix)compile_commands.json:
> scripts/clang-tools/gen_compile_commands.py \
> +$(extmod_prefix)compile_commands.json:
> scripts/clang-tools/gen_compile_commands.py \
> $(if $(KBUILD_EXTMOD),,$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS))
> \
> $(if $(CONFIG_MODULES), $(MODORDER)) FORCE
> $(call if_changed,gen_compile_commands)
>
> -targets += $(extmod-prefix)compile_commands.json
> +targets += $(extmod_prefix)compile_commands.json
>
> PHONY += clang-tidy clang-analyzer
>
> @@ -1919,7 +1919,7 @@ ifdef CONFIG_CC_IS_CLANG
> quiet_cmd_clang_tools = CHECK $<
>cmd_clang_tools = $(PYTHON3)
> $(srctree)/scripts/clang-tools/run-clang-tools.py $@ $<
>
> -clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
> +clang-tidy clang-analyzer: $(extmod_prefix)compile_commands.json
> $(call cmd,clang_tools)
> else
> clang-tidy clang-analyzer:
> --
> 2.27.0
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH] x86/kernel: remove unneeded dead-store initialization
On Wed, Mar 31, 2021 at 1:00 AM Yang Li wrote:
>
> make clang-analyzer on x86_64 defconfig caught my attention with:
>
> arch/x86/kernel/cpu/cacheinfo.c:880:24: warning: Value stored to
> 'this_cpu_ci' during its initialization is never read
> [clang-analyzer-deadcode.DeadStores]
> struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
> ^
>
> So, simply remove this unneeded dead-store initialization to make
> clang-analyzer happy.
>
> As compilers will detect this unneeded assignment and optimize this anyway,
> the resulting object code is identical before and after this change.
>
> No functional change. No change to object code.
Reviewed-by: Nick Desaulniers
Looks like this is from when this code was introduced in
commit 0d55ba46bfbe ("x86/cacheinfo: Move cacheinfo sysfs code to
generic infrastructure")
though this file was moved from arch/x86/kernel/cpu/intel_cacheinfo.c
to arch/x86/kernel/cpu/cacheinfo.c in
commit 1d200c078d0e ("x86/CPU: Rename intel_cacheinfo.c to cacheinfo.c")
(So I don't think a Fixes tag for 0d55ba46bfbe would be appropriate).
Thanks for the patch!
>
> Reported-by: Abaci Robot
> Signed-off-by: Yang Li
> ---
> arch/x86/kernel/cpu/cacheinfo.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
> index 3ca9be4..d66af29 100644
> --- a/arch/x86/kernel/cpu/cacheinfo.c
> +++ b/arch/x86/kernel/cpu/cacheinfo.c
> @@ -877,7 +877,7 @@ void init_intel_cacheinfo(struct cpuinfo_x86 *c)
> static int __cache_amd_cpumap_setup(unsigned int cpu, int index,
> struct _cpuid4_info_regs *base)
> {
> - struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
> + struct cpu_cacheinfo *this_cpu_ci;
> struct cacheinfo *this_leaf;
> int i, sibling;
>
> --
> 1.8.3.1
>
--
Thanks,
~Nick Desaulniers
[PATCH v2] ARM: kprobes: test-thumb: fix for LLVM_IAS=1
There's a few instructions that GAS infers operands but Clang doesn't;
from what I can tell the Arm ARM doesn't say these are optional.
F5.1.257 TBB, TBH T1 Halfword variant
F5.1.238 STREXD T1 variant
F5.1.84 LDREXD T1 variant
Link: https://github.com/ClangBuiltLinux/linux/issues/1309
Signed-off-by: Nick Desaulniers
---
See:
https://lore.kernel.org/linux-arm-kernel/CAMj1kXE5uw4+zV3JVpfA2drOD5TZVMs5a_E5wrrnzjEYc=e...@mail.gmail.com/
for what I'd consider V1. The previous issues with .w suffixes have been
fixed or have fixes pending in LLVM:
* BL+DBG: https://reviews.llvm.org/D97236
* ORN/ORNS: https://reviews.llvm.org/D99538
* RSB/RSBS: https://reviews.llvm.org/D99542
I'd have expected the Arm ARM to use curly braces to denote optional
operands (see also "F5.1.167 RSB, RSBS (register)" for an example).
arch/arm/probes/kprobes/test-thumb.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/probes/kprobes/test-thumb.c
b/arch/arm/probes/kprobes/test-thumb.c
index 456c181a7bfe..4e11f0b760f8 100644
--- a/arch/arm/probes/kprobes/test-thumb.c
+++ b/arch/arm/probes/kprobes/test-thumb.c
@@ -441,21 +441,21 @@ void kprobe_thumb32_test_cases(void)
"3: mvn r0, r0 \n\t"
"2: nop \n\t")
- TEST_RX("tbh[pc, r",7, (9f-(1f+4))>>1,"]",
+ TEST_RX("tbh[pc, r",7, (9f-(1f+4))>>1,", lsl #1]",
"9: \n\t"
".short (2f-1b-4)>>1\n\t"
".short (3f-1b-4)>>1\n\t"
"3: mvn r0, r0 \n\t"
"2: nop \n\t")
- TEST_RX("tbh[pc, r",12, ((9f-(1f+4))>>1)+1,"]",
+ TEST_RX("tbh[pc, r",12, ((9f-(1f+4))>>1)+1,", lsl #1]",
"9: \n\t"
".short (2f-1b-4)>>1\n\t"
".short (3f-1b-4)>>1\n\t"
"3: mvn r0, r0 \n\t"
"2: nop \n\t")
- TEST_RRX("tbh [r",1,9f, ", r",14,1,"]",
+ TEST_RRX("tbh [r",1,9f, ", r",14,1,", lsl #1]",
"9: \n\t"
".short (2f-1b-4)>>1\n\t"
".short (3f-1b-4)>>1\n\t"
@@ -468,10 +468,10 @@ void kprobe_thumb32_test_cases(void)
TEST_UNSUPPORTED("strexbr0, r1, [r2]")
TEST_UNSUPPORTED("strexhr0, r1, [r2]")
- TEST_UNSUPPORTED("strexdr0, r1, [r2]")
+ TEST_UNSUPPORTED("strexdr0, r1, r2, [r2]")
TEST_UNSUPPORTED("ldrexbr0, [r1]")
TEST_UNSUPPORTED("ldrexhr0, [r1]")
- TEST_UNSUPPORTED("ldrexdr0, [r1]")
+ TEST_UNSUPPORTED("ldrexdr0, r1, [r1]")
TEST_GROUP("Data-processing (shifted register) and (modified
immediate)")
--
2.31.0.291.g576ba9dcdaf-goog
Re: [PATCH v2] ARM: kprobes: rewrite test-[arm|thumb].c in UAL
On Fri, Jan 29, 2021 at 1:40 AM Ard Biesheuvel wrote:
>
> On Fri, 29 Jan 2021 at 01:22, Nick Desaulniers
> wrote:
> >
> > > On Thu, 28 Jan 2021 at 20:34, Nick Desaulniers
> > > wrote:
> > > > + TEST_RX("tbh[pc, r",7, (9f-(1f+4))>>1,", lsl #1]",
> > > >
> > > On Thu, Jan 28, 2021 at 1:03 PM Ard Biesheuvel wrote:
> > > Why is this change needed? Are the resulting opcodes equivalent? Does
> > > GAS infer the lsl #1 but Clang doesn't?
> >
> > Yes; it seems if you serialize/deserialize this using GNU `as` and
> > objdump, that's the canonical form (GNU objdump seems to print in UAL
> > form, IIUC). I didn't see anything specifically about `tbh` in
> > https://developer.arm.com/documentation/dui0473/c/writing-arm-assembly-language/assembly-language-changes-after-rvctv2-1?lang=en
> > but it's what GNU objdump produces and what clang's integrated
> > assembler accepts.
> >
>
> This matches the ARM ARM: TBB and TBH are indexed table lookups, where
> the table consists of 16-bit quantities in the latter case, and so the
> LSL #1 is implied.
(Sorry, more time to look into thumb2 now, revisiting this)
Then I would have expected it to be stated that the operand is
implicit in the Arm ARM or use the curly brace notation for the
optional operands (see "F5.1.167 RSB, RSBS (register)" for an example
of what I'm talking about. AFAICT, there's no such language in the Arm
ARM about TBH. This is also what GNU binutils' objdump spits out when
disassembled. And TBB differs from TBH in regards to this operand;
TBH has it, TBB does not. It's not clear to me whether these
shorthands are intentional (pseudo ops) vs unintentional (parsing
bugs) in GAS.
>
> > > >
> > > > #define _DATA_PROCESSING32_DNM(op,s,val)
> > > > \
> > > > - TEST_RR(op s".w r0, r",1, VAL1,", r",2, val, "")
> > > > \
> > > > + TEST_RR(op s" r0, r",1, VAL1,", r",2, val, "")
> > > > \
> > >
> > > What is wrong with these .w suffixes? Shouldn't the assembler accept
> > > these even on instructions that only exist in a wide encoding?
> >
> > Yeah, I'm not sure these have anything to do with UAL. Looking at
> > LLVM's sources and IIRC, LLVM has "InstAlias"es it uses for .w
> > suffixes. I think I need to fix those in LLVM for a couple
> > instructions, rather than modify these in kernel sources. I'll split
> > off the arm-test.c and thumb-test.c into separate patches, fix LLVM,
> > and drop the .w suffix changes to thumb-test.c.
> >
>
> The ARM ARM (DDI0487G.a F1.2) clearly specifies that any instruction
> documented as supporting the optional {q} suffix (which is almost all
> if not all of them) can be issued with the .w appended, even if doing
> so is redundant (note that it even documents this as being supported
> for the A32 ISA, which GAS does not implement today either)
>
> Given how we often use this suffix to ensure that the opcode fills the
> expected amount of space (for things like jump tables, etc), we cannot
> simply drop these left and right and expect things to still work.
I understand fixed in:
* BL+DBG: https://reviews.llvm.org/D97236
* ORN/ORNS: https://reviews.llvm.org/D99538
* RSB/RSBS: https://reviews.llvm.org/D99542
--
Thanks,
~Nick Desaulniers
Re: [PATCH 2/3] riscv: Workaround mcount name prior to clang-13
On Thu, Mar 25, 2021 at 3:38 PM Nathan Chancellor wrote:
>
> Prior to clang 13.0.0, the RISC-V name for the mcount symbol was
> "mcount", which differs from the GCC version of "_mcount", which results
> in the following errors:
>
> riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_level':
> main.c:(.text+0xe): undefined reference to `mcount'
> riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_start':
> main.c:(.text+0x4e): undefined reference to `mcount'
> riscv64-linux-gnu-ld: init/main.o: in function `__traceiter_initcall_finish':
> main.c:(.text+0x92): undefined reference to `mcount'
> riscv64-linux-gnu-ld: init/main.o: in function `.LBB32_28':
> main.c:(.text+0x30c): undefined reference to `mcount'
> riscv64-linux-gnu-ld: init/main.o: in function `free_initmem':
> main.c:(.text+0x54c): undefined reference to `mcount'
>
> This has been corrected in https://reviews.llvm.org/D98881 but the
> minimum supported clang version is 10.0.1. To avoid build errors and to
> gain a working function tracer, adjust the name of the mcount symbol for
> older versions of clang in mount.S and recordmcount.pl.
>
> Cc: sta...@vger.kernel.org
> Link: https://github.com/ClangBuiltLinux/linux/issues/1331
> Signed-off-by: Nathan Chancellor
Thanks for keeping this alive on clang-10, and resolving it for future releases!
Reviewed-by: Nick Desaulniers
> ---
> arch/riscv/include/asm/ftrace.h | 14 --
> arch/riscv/kernel/mcount.S | 10 +-
> scripts/recordmcount.pl | 2 +-
> 3 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 845002cc2e57..04dad3380041 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -13,9 +13,19 @@
> #endif
> #define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
>
> +/*
> + * Clang prior to 13 had "mcount" instead of "_mcount":
> + * https://reviews.llvm.org/D98881
> + */
> +#if defined(CONFIG_CC_IS_GCC) || CONFIG_CLANG_VERSION >= 13
> +#define MCOUNT_NAME _mcount
> +#else
> +#define MCOUNT_NAME mcount
> +#endif
> +
> #define ARCH_SUPPORTS_FTRACE_OPS 1
> #ifndef __ASSEMBLY__
> -void _mcount(void);
> +void MCOUNT_NAME(void);
> static inline unsigned long ftrace_call_adjust(unsigned long addr)
> {
> return addr;
> @@ -36,7 +46,7 @@ struct dyn_arch_ftrace {
> * both auipc and jalr at the same time.
> */
>
> -#define MCOUNT_ADDR((unsigned long)_mcount)
> +#define MCOUNT_ADDR((unsigned long)MCOUNT_NAME)
> #define JALR_SIGN_MASK (0x0800)
> #define JALR_OFFSET_MASK (0x0fff)
> #define AUIPC_OFFSET_MASK (0xf000)
> diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S
> index 8a5593ff9ff3..6d462681c9c0 100644
> --- a/arch/riscv/kernel/mcount.S
> +++ b/arch/riscv/kernel/mcount.S
> @@ -47,8 +47,8 @@
>
> ENTRY(ftrace_stub)
> #ifdef CONFIG_DYNAMIC_FTRACE
> - .global _mcount
> - .set_mcount, ftrace_stub
> + .global MCOUNT_NAME
> + .setMCOUNT_NAME, ftrace_stub
> #endif
> ret
> ENDPROC(ftrace_stub)
> @@ -78,7 +78,7 @@ ENDPROC(return_to_handler)
> #endif
>
> #ifndef CONFIG_DYNAMIC_FTRACE
> -ENTRY(_mcount)
> +ENTRY(MCOUNT_NAME)
> la t4, ftrace_stub
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> la t0, ftrace_graph_return
> @@ -124,6 +124,6 @@ do_trace:
> jalrt5
> RESTORE_ABI_STATE
> ret
> -ENDPROC(_mcount)
> +ENDPROC(MCOUNT_NAME)
> #endif
> -EXPORT_SYMBOL(_mcount)
> +EXPORT_SYMBOL(MCOUNT_NAME)
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index a36df04cfa09..7b83a1aaec98 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -392,7 +392,7 @@ if ($arch eq "x86_64") {
> $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
> } elsif ($arch eq "riscv") {
> $function_regex = "^([0-9a-fA-F]+)\\s+<([^.0-9][0-9a-zA-Z_\\.]+)>:";
> -$mcount_regex = "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL(_PLT)?\\s_mcount\$";
> +$mcount_regex =
> "^\\s*([0-9a-fA-F]+):\\sR_RISCV_CALL(_PLT)?\\s_?mcount\$";
> $type = ".quad";
> $alignment = 2;
> } elsif ($arch eq "nds32") {
> --
> 2.31.0
>
> --
> You received this message because you are subscribed to the Google Groups
> "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to clang-built-linux+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/clang-built-linux/20210325223807.2423265-3-nathan%40kernel.org.
--
Thanks,
~Nick Desaulniers
Re: [PATCH] lockdep: address clang -Wformat warning printing for %hd
On Mon, Mar 22, 2021 at 4:55 AM Arnd Bergmann wrote:
>
> From: Arnd Bergmann
>
> Clang doesn't like format strings that truncate a 32-bit
> value to something shorter:
>
> kernel/locking/lockdep.c:709:4: error: format specifies type 'short' but the
> argument has type 'int' [-Werror,-Wformat]
>
> In this case, the warning is a slightly questionable, as it could realize
> that both class->wait_type_outer and class->wait_type_inner are in fact
> 8-bit struct members, even though the result of the ?: operator becomes an
> 'int'.
>
> However, there is really no point in printing the number as a 16-bit
> 'short' rather than either an 8-bit or 32-bit number, so just change
> it to a normal %d.
Thanks for the patch!
Reviewed-by: Nick Desaulniers
>
> Fixes: de8f5e4f2dc1 ("lockdep: Introduce wait-type checks")
> Signed-off-by: Arnd Bergmann
> ---
> kernel/locking/lockdep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index 70bf3e48eae3..bb3b0bc6ee17 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -705,7 +705,7 @@ static void print_lock_name(struct lock_class *class)
>
> printk(KERN_CONT " (");
> __print_lock_name(class);
> - printk(KERN_CONT "){%s}-{%hd:%hd}", usage,
> + printk(KERN_CONT "){%s}-{%d:%d}", usage,
> class->wait_type_outer ?: class->wait_type_inner,
> class->wait_type_inner);
> }
> --
> 2.29.2
>
--
Thanks,
~Nick Desaulniers
Re: drivers/gpu/drm/i915/gvt/gtt.c:267:19: error: unused function 'get_pt_type'
On Wed, Mar 24, 2021 at 2:12 AM Zhenyu Wang wrote:
>
> On 2021.03.23 15:15:29 -0700, Nick Desaulniers wrote:
> > On Fri, Mar 19, 2021 at 11:45 PM kernel test robot wrote:
> > >
> > > Hi Nick,
> > >
> > > FYI, the error/warning still remains.
> > >
> > > tree:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > > head: 1c273e10bc0cc7efb933e0ca10e260cdfc9f0b8c
> > > commit: 9f4069b055d1508c833115df7493b6e0001e5c9b drm/i915: re-disable
> > > -Wframe-address
> >
> > This in unrelated to my change.
> >
> > + Changbin, Zhenyu (authors of 3aff3512802) and Zhi (author of
> > 054f4eba2a298) in case there's any interest in fixing this up.
> > Otherwise I don't think these tiny helpful functions were meant to be
> > used somewhere but are not, so there's not much value in cleaning them
> > up.
>
> I'll check that, should be some left over last big gtt code refactor.
> Looks lkp guys don't apply -Wunused-function for gvt tree build test...
Thanks, yeah the report from the bot mentions it had `make W=1 ...` on.
>
> Thanks
>
> >
> > > date: 11 months ago
> > > config: x86_64-randconfig-a016-20210319 (attached as .config)
> > > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project
> > > fcc1ce00931751ac02498986feb37744e9ace8de)
> > > reproduce (this is a W=1 build):
^ hidden note about W=1, easy to miss.
> > > wget
> > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> > > -O ~/bin/make.cross
> > > chmod +x ~/bin/make.cross
> > > # install x86_64 cross compiling tool for clang build
> > > # apt-get install binutils-x86-64-linux-gnu
> > > #
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f4069b055d1508c833115df7493b6e0001e5c9b
> > > git remote add linus
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > > git fetch --no-tags linus master
> > > git checkout 9f4069b055d1508c833115df7493b6e0001e5c9b
> > > # save the attached .config to linux build tree
> > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> > > ARCH=x86_64
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > > >> drivers/gpu/drm/i915/gvt/gtt.c:267:19: error: unused function
> > > >> 'get_pt_type' [-Werror,-Wunused-function]
> > >static inline int get_pt_type(int type)
> > > ^
> > > >> drivers/gpu/drm/i915/gvt/gtt.c:590:20: error: unused function
> > > >> 'ppgtt_set_guest_root_entry' [-Werror,-Wunused-function]
> > >static inline void ppgtt_set_guest_root_entry(struct intel_vgpu_mm *mm,
> > > ^
> > >2 errors generated.
> > >
> > >
> > > vim +/get_pt_type +267 drivers/gpu/drm/i915/gvt/gtt.c
> > >
> > > 2707e44466881d6 Zhi Wang 2016-03-28 266
> > > 054f4eba2a2985b Zhi Wang 2017-10-10 @267 static inline int
> > > get_pt_type(int type)
> > > 054f4eba2a2985b Zhi Wang 2017-10-10 268 {
> > > 054f4eba2a2985b Zhi Wang 2017-10-10 269 return
> > > gtt_type_table[type].pt_type;
> > > 054f4eba2a2985b Zhi Wang 2017-10-10 270 }
> > > 054f4eba2a2985b Zhi Wang 2017-10-10 271
> > >
> > > :: The code at line 267 was first introduced by commit
> > > :: 054f4eba2a2985b1db43353b7b5ce90e96cf9bb9 drm/i915/gvt: Introduce
> > > page table type of current level in GTT type enumerations
> > >
> > > :: TO: Zhi Wang
> > > :: CC: Zhenyu Wang
> > >
> > > ---
> > > 0-DAY CI Kernel Test Service, Intel Corporation
> > > https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
> >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
Re: drivers/gpu/drm/i915/gvt/gtt.c:267:19: error: unused function 'get_pt_type'
On Fri, Mar 19, 2021 at 11:45 PM kernel test robot wrote:
>
> Hi Nick,
>
> FYI, the error/warning still remains.
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> master
> head: 1c273e10bc0cc7efb933e0ca10e260cdfc9f0b8c
> commit: 9f4069b055d1508c833115df7493b6e0001e5c9b drm/i915: re-disable
> -Wframe-address
This in unrelated to my change.
+ Changbin, Zhenyu (authors of 3aff3512802) and Zhi (author of
054f4eba2a298) in case there's any interest in fixing this up.
Otherwise I don't think these tiny helpful functions were meant to be
used somewhere but are not, so there's not much value in cleaning them
up.
> date: 11 months ago
> config: x86_64-randconfig-a016-20210319 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project
> fcc1ce00931751ac02498986feb37744e9ace8de)
> reproduce (this is a W=1 build):
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install x86_64 cross compiling tool for clang build
> # apt-get install binutils-x86-64-linux-gnu
> #
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f4069b055d1508c833115df7493b6e0001e5c9b
> git remote add linus
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout 9f4069b055d1508c833115df7493b6e0001e5c9b
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot
>
> All errors (new ones prefixed by >>):
>
> >> drivers/gpu/drm/i915/gvt/gtt.c:267:19: error: unused function
> >> 'get_pt_type' [-Werror,-Wunused-function]
>static inline int get_pt_type(int type)
> ^
> >> drivers/gpu/drm/i915/gvt/gtt.c:590:20: error: unused function
> >> 'ppgtt_set_guest_root_entry' [-Werror,-Wunused-function]
>static inline void ppgtt_set_guest_root_entry(struct intel_vgpu_mm *mm,
> ^
>2 errors generated.
>
>
> vim +/get_pt_type +267 drivers/gpu/drm/i915/gvt/gtt.c
>
> 2707e44466881d6 Zhi Wang 2016-03-28 266
> 054f4eba2a2985b Zhi Wang 2017-10-10 @267 static inline int get_pt_type(int
> type)
> 054f4eba2a2985b Zhi Wang 2017-10-10 268 {
> 054f4eba2a2985b Zhi Wang 2017-10-10 269return
> gtt_type_table[type].pt_type;
> 054f4eba2a2985b Zhi Wang 2017-10-10 270 }
> 054f4eba2a2985b Zhi Wang 2017-10-10 271
>
> :: The code at line 267 was first introduced by commit
> :: 054f4eba2a2985b1db43353b7b5ce90e96cf9bb9 drm/i915/gvt: Introduce page
> table type of current level in GTT type enumerations
>
> :: TO: Zhi Wang
> :: CC: Zhenyu Wang
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
--
Thanks,
~Nick Desaulniers
Re: [PATCH] irqchip/gic-v3: fix OF_BAD_ADDR error handling
On Tue, Mar 23, 2021 at 6:18 AM Arnd Bergmann wrote:
>
> From: Arnd Bergmann
>
> When building with extra warnings enabled, clang points out a
> mistake in the error handling:
>
> drivers/irqchip/irq-gic-v3-mbi.c:306:21: error: result of comparison of
> constant 18446744073709551615 with expression of type 'phys_addr_t' (aka
> 'unsigned int') is always false
> [-Werror,-Wtautological-constant-out-of-range-compare]
Looks like based on CONFIG_PHYS_ADDR_T_64BIT, phys_addr_t can be u64
or u32, but of_translate_address always returns a u64. This is fine
for the current value of OF_BAD_ADDR, but I think there's a risk of
losing the top 32b of the return value of of_translate_address() here?
> if (mbi_phys_base == OF_BAD_ADDR) {
>
> Truncate the constant to the same type as the variable it gets compared
> to, to shut make the check work and void the warning.
>
> Fixes: 505287525c24 ("irqchip/gic-v3: Add support for Message Based
> Interrupts as an MSI controller")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/irqchip/irq-gic-v3-mbi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3-mbi.c
> b/drivers/irqchip/irq-gic-v3-mbi.c
> index 563a9b366294..e81e89a81cb5 100644
> --- a/drivers/irqchip/irq-gic-v3-mbi.c
> +++ b/drivers/irqchip/irq-gic-v3-mbi.c
> @@ -303,7 +303,7 @@ int __init mbi_init(struct fwnode_handle *fwnode, struct
> irq_domain *parent)
> reg = of_get_property(np, "mbi-alias", NULL);
> if (reg) {
> mbi_phys_base = of_translate_address(np, reg);
> - if (mbi_phys_base == OF_BAD_ADDR) {
> + if (mbi_phys_base == (phys_addr_t)OF_BAD_ADDR) {
> ret = -ENXIO;
> goto err_free_mbi;
> }
> --
> 2.29.2
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v3 09/17] treewide: Change list_sort to use const pointers
On Tue, Mar 23, 2021 at 1:40 PM Sami Tolvanen wrote: > > list_sort() internally casts the comparison function passed to it > to a different type with constant struct list_head pointers, and > uses this pointer to call the functions, which trips indirect call > Control-Flow Integrity (CFI) checking. > > Instead of removing the consts, this change defines the > list_cmp_func_t type and changes the comparison function types of > all list_sort() callers to use const pointers, thus avoiding type > mismatches. > > Suggested-by: Nick Desaulniers > Signed-off-by: Sami Tolvanen > --- > arch/arm64/kvm/vgic/vgic-its.c | 8 > arch/arm64/kvm/vgic/vgic.c | 3 ++- > block/blk-mq-sched.c| 3 ++- > block/blk-mq.c | 3 ++- > drivers/acpi/nfit/core.c| 3 ++- > drivers/acpi/numa/hmat.c| 3 ++- > drivers/clk/keystone/sci-clk.c | 4 ++-- > drivers/gpu/drm/drm_modes.c | 3 ++- > drivers/gpu/drm/i915/gt/intel_engine_user.c | 3 ++- > drivers/gpu/drm/i915/gvt/debugfs.c | 2 +- > drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 ++- > drivers/gpu/drm/radeon/radeon_cs.c | 4 ++-- > .../hw/usnic/usnic_uiom_interval_tree.c | 3 ++- > drivers/interconnect/qcom/bcm-voter.c | 2 +- > drivers/md/raid5.c | 3 ++- > drivers/misc/sram.c | 4 ++-- > drivers/nvme/host/core.c| 3 ++- > .../pci/controller/cadence/pcie-cadence-host.c | 3 ++- > drivers/spi/spi-loopback-test.c | 3 ++- > fs/btrfs/raid56.c | 3 ++- > fs/btrfs/tree-log.c | 3 ++- > fs/btrfs/volumes.c | 3 ++- > fs/ext4/fsmap.c | 4 ++-- > fs/gfs2/glock.c | 3 ++- > fs/gfs2/log.c | 2 +- > fs/gfs2/lops.c | 3 ++- > fs/iomap/buffered-io.c | 3 ++- > fs/ubifs/gc.c | 7 --- > fs/ubifs/replay.c | 4 ++-- > fs/xfs/scrub/bitmap.c | 4 ++-- > fs/xfs/xfs_bmap_item.c | 4 ++-- > fs/xfs/xfs_buf.c| 6 +++--- > fs/xfs/xfs_extent_busy.c| 4 ++-- > fs/xfs/xfs_extent_busy.h| 3 ++- > fs/xfs/xfs_extfree_item.c | 4 ++-- > fs/xfs/xfs_refcount_item.c | 4 ++-- > fs/xfs/xfs_rmap_item.c | 4 ++-- > include/linux/list_sort.h | 7 --- > lib/list_sort.c | 17 ++--- > lib/test_list_sort.c| 3 ++- > net/tipc/name_table.c | 4 ++-- > 41 files changed, 90 insertions(+), 72 deletions(-) This looks like all of the call sites I could find. Yes, this looks much better, thank you for taking the time to fix all of these. I ran this through some build tests of ARCH=arm64 defconfig, allyesconfig, and same for my host (x86_64). All LGTM. Reviewed-by: Nick Desaulniers Tested-by: Nick Desaulniers -- Thanks, ~Nick Desaulniers
Re: [PATCH] scripts: stable: add script to validate backports
On Tue, Mar 23, 2021 at 12:05 PM Greg Kroah-Hartman wrote: > > The only time git gets involved is when we do a -rc release or when we > do a "real" release, and then we use 'git quiltimport' on the whole > stack. > > Here's a script that I use (much too slow, I know), for checking this > type of thing and I try to remember to run it before every cycle of -rc > releases: > https://github.com/gregkh/commit_tree/blob/master/find_fixes_in_queue > > It's a hack, and picks up more things than is really needed, but I would > rather it error on that side than the other. Yes, my script is similar. Looks like yours also runs on a git tree. I noticed that id_fixed_in runs `git grep -l --threads=3 ` to find fixes; that's neat, I didn't know about `--threads=`. I tried it with ae46578b963f manually: $ git grep -l --threads=3 ae46578b963f $ Should it have found a7889c6320b9 and 773e0c402534? Perhaps `git log --grep=` should be used instead? I thought `git grep` only greps files in the archive, not commit history? -- Thanks, ~Nick Desaulniers
Re: [PATCH] scripts: stable: add script to validate backports
On Tue, Mar 23, 2021 at 6:56 AM Greg Kroah-Hartman wrote: > > On Tue, Mar 16, 2021 at 02:31:33PM -0700, Nick Desaulniers wrote: > > A common recurring mistake made when backporting patches to stable is > > forgetting to check for additional commits tagged with `Fixes:`. This > > script validates that local commits have a `commit upstream.` > > line in their commit message, and whether any additional `Fixes:` shas > > exist in the `master` branch but were not included. It can not know > > about fixes yet to be discovered, or fixes sent to the mailing list but > > not yet in mainline. > > > > To save time, it avoids checking all of `master`, stopping early once > > we've reached the commit time of the earliest backport. It takes 0.5s to > > validate 2 patches to linux-5.4.y when master is v5.12-rc3 and 5s to > > validate 27 patches to linux-4.19.y. It does not recheck dependencies of > > found fixes; the user is expected to run this script to a fixed point. > > It depnds on pygit2 python library for working with git, which can be > > installed via: > > $ pip3 install pygit2 > > > > It's expected to be run from a stable tree with commits applied. For > > example, consider 3cce9d44321e which is a fix for f77ac2e378be. Let's > > say I cherry picked f77ac2e378be into linux-5.4.y but forgot > > 3cce9d44321e (true story). If I ran: > > > > $ ./scripts/stable/check_backports.py > > Checking 1 local commits for additional Fixes: in master > > Please consider backporting 3cce9d44321e as a fix for f77ac2e378be > > While interesting, I don't use a git tree for the stable queue, so this > doesn't really fit into my workflow, sorry. Well, what is your workflow? > And we do have other "stable tree helper" scripts in the > stable-queue.git repo, perhaps that's a better place for this than the > main kernel repo? Sure, here it is moved over to there. Let me know if there's a preferred way to send it. -- Thanks, ~Nick Desaulniers 0001-scripts-add-script-to-validate-backports.patch Description: Binary data
Re: sparc: clang: error: unknown argument: '-mno-fpu'
On Fri, Mar 19, 2021 at 4:56 AM Arnd Bergmann wrote: > > On Fri, Mar 19, 2021 at 12:38 PM John Paul Adrian Glaubitz > wrote: > > On 3/19/21 12:31 PM, Arnd Bergmann wrote: > > > On Fri, Mar 19, 2021 at 8:36 AM Naresh Kamboju > > > wrote: > > >> > > >> Linux mainline master build breaks for sparc defconfig. > > >> There are multiple errors / warnings with clang-12 and clang-11 and 10. > > >> - sparc (defconfig) with clang-12, clang-11 and clang-10 > > >> - sparc (tinyconfig) with clang-12, clang-11 and clang-10 > > >> - sparc (allnoconfig) with clang-12, clang-11 and clang-10 > > >> > > >> make --silent --keep-going --jobs=8 > > >> O=/home/tuxbuild/.cache/tuxmake/builds/1/tmp ARCH=sparc > > >> CROSS_COMPILE=sparc64-linux-gnu- 'HOSTCC=sccache clang' 'CC=sccache > > >> clang' > > > > > > I don't think anyone has successfully built a sparc kernel with clang, > > > and I don't > > > think it's worth trying either, given how little upstream work the > > > sparc port sees overall. > > > > We'll get there. There are some other SPARC-related clang bugs that need > > to be squashed first. We have made quite some improvements and it's actually > > maintained by the community. Of course, we don't have a commercial backer > > but that shouldn't be necessary for open source to work. > > I meant there is no point for Naresh to do it as part of his build > testing with tuxmake. > If someone else gets it working, they can tell Naresh to try again, but until > then, I'd limit clang regression testing to x86, arm, powerpc, s390, mips, > riscv > and arc. We definitely cannot yet build arc. $ cmake ... -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="ARC" $ ARCH=arc CROSS_COMPILE=arc-linux-gnu- make LLVM=1 -j72 defconfig vmlinux ... clang-13: error: unknown argument: '-mmedium-calls' clang-13: error: unknown argument: '-fsection-anchors' clang-13: error: unknown argument: '-mlock' clang-13: error: unknown argument: '-mswape' clang-13: error: unknown argument: '-mno-sdata' clang-13: error: unknown argument: '-fcall-used-gp' -- Thanks, ~Nick Desaulniers
[PATCH] Makefile: fix GDB warning with CONFIG_RELR
GDB produces the following warning when debugging kernels built with CONFIG_RELR: BFD: /android0/linux-next/vmlinux: unknown type [0x13] section `.relr.dyn' when loading a kernel built with CONFIG_RELR into GDB. It can also prevent debugging symbols using such relocations. Peter sugguests: [That flag] means that lld will use dynamic tags and section type numbers in the OS-specific range rather than the generic range. The kernel itself doesn't care about these numbers; it determines the location of the RELR section using symbols defined by a linker script. Link: https://github.com/ClangBuiltLinux/linux/issues/1057 Suggested-by: Peter Collingbourne Signed-off-by: Nick Desaulniers --- Makefile | 2 +- scripts/tools-support-relr.sh | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 5160ff8903c1..47741cb60995 100644 --- a/Makefile +++ b/Makefile @@ -1088,7 +1088,7 @@ LDFLAGS_vmlinux += $(call ld-option, -X,) endif ifeq ($(CONFIG_RELR),y) -LDFLAGS_vmlinux+= --pack-dyn-relocs=relr +LDFLAGS_vmlinux+= --pack-dyn-relocs=relr --use-android-relr-tags endif # We never want expected sections to be placed heuristically by the diff --git a/scripts/tools-support-relr.sh b/scripts/tools-support-relr.sh index 45e8aa360b45..cb55878bd5b8 100755 --- a/scripts/tools-support-relr.sh +++ b/scripts/tools-support-relr.sh @@ -7,7 +7,8 @@ trap "rm -f $tmp_file.o $tmp_file $tmp_file.bin" EXIT cat << "END" | $CC -c -x c - -o $tmp_file.o >/dev/null 2>&1 void *p = END -$LD $tmp_file.o -shared -Bsymbolic --pack-dyn-relocs=relr -o $tmp_file +$LD $tmp_file.o -shared -Bsymbolic --pack-dyn-relocs=relr \ + --use-android-relr-tags -o $tmp_file # Despite printing an error message, GNU nm still exits with exit code 0 if it # sees a relr section. So we need to check that nothing is printed to stderr. -- 2.31.0.rc2.261.g7f71774620-goog
Re: [PATCH v2 04/17] module: ensure __cfi_check alignment
On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen wrote:
>
> CONFIG_CFI_CLANG_SHADOW assumes the __cfi_check() function is page
> aligned and at the beginning of the .text section. While Clang would
> normally align the function correctly, it fails to do so for modules
> with no executable code.
>
> This change ensures the correct __cfi_check() location and
> alignment. It also discards the .eh_frame section, which Clang can
> generate with certain sanitizers, such as CFI.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=46293
> Signed-off-by: Sami Tolvanen
> ---
> scripts/module.lds.S | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/module.lds.S b/scripts/module.lds.S
> index 168cd27e6122..93518579cf5d 100644
> --- a/scripts/module.lds.S
> +++ b/scripts/module.lds.S
> @@ -3,10 +3,19 @@
> * Archs are free to supply their own linker scripts. ld will
> * combine them automatically.
> */
> +#include
> +
> +#ifdef CONFIG_CFI_CLANG
> +# define ALIGN_CFI ALIGN(PAGE_SIZE)
> +#else
> +# define ALIGN_CFI
> +#endif
> +
> SECTIONS {
> /DISCARD/ : {
> *(.discard)
> *(.discard.*)
> + *(.eh_frame)
Do we want to unconditionally discard this section from modules for
all arches/configs? I like how we conditionally do so on
SANITIZER_DISCARDS in include/asm-generic/vmlinux.lds.h for example.
> }
>
> __ksymtab 0 : { *(SORT(___ksymtab+*)) }
> @@ -40,7 +49,14 @@ SECTIONS {
> *(.rodata..L*)
> }
>
> - .text : { *(.text .text.[0-9a-zA-Z_]*) }
> + /*
> +* With CONFIG_CFI_CLANG, we assume __cfi_check is at the beginning
> +* of the .text section, and is aligned to PAGE_SIZE.
> +*/
> + .text : ALIGN_CFI {
> + *(.text.__cfi_check)
> + *(.text .text.[0-9a-zA-Z_]* .text..L.cfi*)
> + }
> }
>
> /* bring in arch-specific sections */
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 07/17] kallsyms: strip ThinLTO hashes from static functions
On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen wrote:
>
> With CONFIG_CFI_CLANG and ThinLTO, Clang appends a hash to the names
> of all static functions not marked __used. This can break userspace
> tools that don't expect the function name to change, so strip out the
> hash from the output.
>
> Suggested-by: Jack Pham
> Signed-off-by: Sami Tolvanen
> Reviewed-by: Kees Cook
> ---
> kernel/kallsyms.c | 54 ++-
> 1 file changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 8043a90aa50e..17d3a704bafa 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -161,6 +161,26 @@ static unsigned long kallsyms_sym_address(int idx)
> return kallsyms_relative_base - 1 - kallsyms_offsets[idx];
> }
>
> +#if defined(CONFIG_CFI_CLANG) && defined(CONFIG_LTO_CLANG_THIN)
> +/*
> + * LLVM appends a hash to static function names when ThinLTO and CFI are
> + * both enabled, which causes confusion and potentially breaks user space
Might be nice to add an example, something along the lines of:
ie. foo() becomes foo$asfdasdfasdfasdf()
> + * tools, so we will strip the postfix from expanded symbol names.
s/postfix/suffix/ ?
> + */
> +static inline char *cleanup_symbol_name(char *s)
> +{
> + char *res = NULL;
> +
> + res = strrchr(s, '$');
> + if (res)
> + *res = '\0';
> +
> + return res;
> +}
> +#else
> +static inline char *cleanup_symbol_name(char *s) { return NULL; }
> +#endif
Might be nicer to return a `bool` and have the larger definition
`return res != NULL`). Not sure what a caller would do with `res` if
it was not `NULL`?
> +
> /* Lookup the address for this symbol. Returns 0 if not found. */
> unsigned long kallsyms_lookup_name(const char *name)
> {
> @@ -173,6 +193,9 @@ unsigned long kallsyms_lookup_name(const char *name)
>
> if (strcmp(namebuf, name) == 0)
> return kallsyms_sym_address(i);
> +
> + if (cleanup_symbol_name(namebuf) && strcmp(namebuf, name) ==
> 0)
> + return kallsyms_sym_address(i);
> }
> return module_kallsyms_lookup_name(name);
> }
> @@ -303,7 +326,9 @@ const char *kallsyms_lookup(unsigned long addr,
>namebuf, KSYM_NAME_LEN);
> if (modname)
> *modname = NULL;
> - return namebuf;
> +
> + ret = namebuf;
> + goto found;
> }
>
> /* See if it's in a module or a BPF JITed image. */
> @@ -316,11 +341,16 @@ const char *kallsyms_lookup(unsigned long addr,
> if (!ret)
> ret = ftrace_mod_address_lookup(addr, symbolsize,
> offset, modname, namebuf);
> +
> +found:
> + cleanup_symbol_name(namebuf);
> return ret;
> }
>
> int lookup_symbol_name(unsigned long addr, char *symname)
> {
> + int res;
> +
> symname[0] = '\0';
> symname[KSYM_NAME_LEN - 1] = '\0';
>
> @@ -331,15 +361,23 @@ int lookup_symbol_name(unsigned long addr, char
> *symname)
> /* Grab name */
> kallsyms_expand_symbol(get_symbol_offset(pos),
>symname, KSYM_NAME_LEN);
> - return 0;
> + goto found;
> }
> /* See if it's in a module. */
> - return lookup_module_symbol_name(addr, symname);
> + res = lookup_module_symbol_name(addr, symname);
> + if (res)
> + return res;
> +
> +found:
> + cleanup_symbol_name(symname);
> + return 0;
> }
>
> int lookup_symbol_attrs(unsigned long addr, unsigned long *size,
> unsigned long *offset, char *modname, char *name)
> {
> + int res;
> +
> name[0] = '\0';
> name[KSYM_NAME_LEN - 1] = '\0';
>
> @@ -351,10 +389,16 @@ int lookup_symbol_attrs(unsigned long addr, unsigned
> long *size,
> kallsyms_expand_symbol(get_symbol_offset(pos),
>name, KSYM_NAME_LEN);
> modname[0] = '\0';
> - return 0;
> + goto found;
> }
> /* See if it's in a module. */
> - return lookup_module_symbol_attrs(addr, size, offset, modname, name);
> + res = lookup_module_symbol_attrs(addr, size, offset, modname, name);
> + if (res)
> + return res;
> +
> +found:
> + cleanup_symbol_name(name);
> + return 0;
> }
>
> /* Look up a kernel symbol and return it in a text buffer. */
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 05/17] workqueue: use WARN_ON_FUNCTION_MISMATCH
On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen wrote: > > With CONFIG_CFI_CLANG, a callback function passed to > __queue_delayed_work from a module points to a jump table entry > defined in the module instead of the one used in the core kernel, > which breaks function address equality in this check: > > WARN_ON_ONCE(timer->function != delayed_work_timer_fn); > > Use WARN_ON_FUNCTION_MISMATCH() instead to disable the warning > when CFI and modules are both enabled. Does __cficanonical help with such comparisons? Or would that be a very invasive change, if the concern was to try to keep these checks in place for CONFIG_CFI_CLANG? > > Signed-off-by: Sami Tolvanen > --- > kernel/workqueue.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/workqueue.c b/kernel/workqueue.c > index 0d150da252e8..03fe07d2f39f 100644 > --- a/kernel/workqueue.c > +++ b/kernel/workqueue.c > @@ -1630,7 +1630,7 @@ static void __queue_delayed_work(int cpu, struct > workqueue_struct *wq, > struct work_struct *work = >work; > > WARN_ON_ONCE(!wq); > - WARN_ON_ONCE(timer->function != delayed_work_timer_fn); > + WARN_ON_FUNCTION_MISMATCH(timer->function, delayed_work_timer_fn); > WARN_ON_ONCE(timer_pending(timer)); > WARN_ON_ONCE(!list_empty(>entry)); > > -- > 2.31.0.291.g576ba9dcdaf-goog > -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 10/17] lkdtm: use __va_function
On Thu, Mar 18, 2021 at 11:43 AM Nick Desaulniers
wrote:
>
> On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen
> wrote:
> >
> > To ensure we take the actual address of a function in kernel text, use
> > __va_function. Otherwise, with CONFIG_CFI_CLANG, the compiler replaces
> > the address with a pointer to the CFI jump table, which is actually in
> > the module when compiled with CONFIG_LKDTM=m.
>
> Should patch 10 and 12 be reordered against one another? Otherwise it
> looks like 12 defines __va_function while 10 uses it?
Ah, nvm patch 3 defines a generic version, I see.
>
>
> >
> > Signed-off-by: Sami Tolvanen
> > Acked-by: Kees Cook
> > ---
> > drivers/misc/lkdtm/usercopy.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c
> > index 109e8d4302c1..d173d6175c87 100644
> > --- a/drivers/misc/lkdtm/usercopy.c
> > +++ b/drivers/misc/lkdtm/usercopy.c
> > @@ -314,7 +314,7 @@ void lkdtm_USERCOPY_KERNEL(void)
> >
> > pr_info("attempting bad copy_to_user from kernel text: %px\n",
> > vm_mmap);
> > - if (copy_to_user((void __user *)user_addr, vm_mmap,
> > + if (copy_to_user((void __user *)user_addr, __va_function(vm_mmap),
> > unconst + PAGE_SIZE)) {
> > pr_warn("copy_to_user failed, but lacked Oops\n");
> > goto free_user;
> > --
> > 2.31.0.291.g576ba9dcdaf-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 10/17] lkdtm: use __va_function
On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen wrote:
>
> To ensure we take the actual address of a function in kernel text, use
> __va_function. Otherwise, with CONFIG_CFI_CLANG, the compiler replaces
> the address with a pointer to the CFI jump table, which is actually in
> the module when compiled with CONFIG_LKDTM=m.
Should patch 10 and 12 be reordered against one another? Otherwise it
looks like 12 defines __va_function while 10 uses it?
>
> Signed-off-by: Sami Tolvanen
> Acked-by: Kees Cook
> ---
> drivers/misc/lkdtm/usercopy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/lkdtm/usercopy.c b/drivers/misc/lkdtm/usercopy.c
> index 109e8d4302c1..d173d6175c87 100644
> --- a/drivers/misc/lkdtm/usercopy.c
> +++ b/drivers/misc/lkdtm/usercopy.c
> @@ -314,7 +314,7 @@ void lkdtm_USERCOPY_KERNEL(void)
>
> pr_info("attempting bad copy_to_user from kernel text: %px\n",
> vm_mmap);
> - if (copy_to_user((void __user *)user_addr, vm_mmap,
> + if (copy_to_user((void __user *)user_addr, __va_function(vm_mmap),
> unconst + PAGE_SIZE)) {
> pr_warn("copy_to_user failed, but lacked Oops\n");
> goto free_user;
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 09/17] lib/list_sort: fix function type mismatches
29e6bab985 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -1155,7 +1155,8 @@ iomap_ioend_try_merge(struct iomap_ioend *ioend,
struct list_head *more_ioends,
EXPORT_SYMBOL_GPL(iomap_ioend_try_merge);
static int
-iomap_ioend_compare(void *priv, struct list_head *a, struct list_head *b)
+iomap_ioend_compare(void *priv, const struct list_head *a,
+ const struct list_head *b)
{
struct iomap_ioend *ia = container_of(a, struct iomap_ioend, io_list);
struct iomap_ioend *ib = container_of(b, struct iomap_ioend, io_list);
diff --git a/include/linux/list_sort.h b/include/linux/list_sort.h
index 20f178c24e9d..4fe9cb94d0d1 100644
--- a/include/linux/list_sort.h
+++ b/include/linux/list_sort.h
@@ -6,8 +6,9 @@
struct list_head;
+typedef int __attribute__((nonnull(2,3))) (*cmp_func)(void *,
+ struct list_head const *, struct list_head const *);
+
__attribute__((nonnull(2,3)))
-void list_sort(void *priv, struct list_head *head,
- int (*cmp)(void *priv, struct list_head *a,
- struct list_head *b));
+void list_sort(void *priv, struct list_head *head, cmp_func cmp);
#endif
diff --git a/lib/list_sort.c b/lib/list_sort.c
index 52f0c258c895..6cfac649c4a6 100644
--- a/lib/list_sort.c
+++ b/lib/list_sort.c
@@ -7,9 +7,6 @@
#include
#include
-typedef int __attribute__((nonnull(2,3))) (*cmp_func)(void *,
- struct list_head const *, struct list_head const *);
-
/*
* Returns a list organized in an intermediate format suited
* to chaining of merge() calls: null-terminated, no reserved or
@@ -185,9 +182,7 @@ static void merge_final(void *priv, cmp_func cmp,
struct list_head *head,
* 2^(k+1) - 1 (second merge of case 5 when x == 2^(k-1) - 1).
*/
__attribute__((nonnull(2,3)))
-void list_sort(void *priv, struct list_head *head,
- int (*cmp)(void *priv, struct list_head *a,
- struct list_head *b))
+void list_sort(void *priv, struct list_head *head, cmp_func cmp)
{
struct list_head *list = head->next, *pending = NULL;
size_t count = 0; /* Count of pending */
```
There are probably more instances in the tree to clean up, but that
compiles with x86_64 defconfig, and I'm sure it doesn't suffer the CFI
issue from the cast.
> if (likely(bits)) {
> struct list_head *a = *tail, *b = a->prev;
>
> - a = merge(priv, (cmp_func)cmp, b, a);
> + a = merge(priv, cmp, b, a);
> /* Install the merged result in place of the inputs */
> a->prev = b->prev;
> *tail = a;
> @@ -249,10 +249,10 @@ void list_sort(void *priv, struct list_head *head,
>
> if (!next)
> break;
> - list = merge(priv, (cmp_func)cmp, pending, list);
> + list = merge(priv, cmp, pending, list);
> pending = next;
> }
> /* The final merge, rebuilding prev links */
> - merge_final(priv, (cmp_func)cmp, head, pending, list);
> + merge_final(priv, cmp, head, pending, list);
> }
> EXPORT_SYMBOL(list_sort);
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 02/17] cfi: add __cficanonical
On Thu, Mar 18, 2021 at 10:11 AM Sami Tolvanen wrote:
>
> With CONFIG_CFI_CLANG, the compiler replaces a function address taken
> in C code with the address of a local jump table entry, which passes
> runtime indirect call checks. However, the compiler won't replace
> addresses taken in assembly code, which will result in a CFI failure
> if we later jump to such an address in instrumented C code. The code
> generated for the non-canonical jump table looks this:
>
> : /* In C, points here */
> jmp noncanonical
> ...
> :/* function body */
> ...
>
> This change adds the __cficanonical attribute, which tells the
> compiler to use a canonical jump table for the function instead. This
> means the compiler will rename the actual function to .cfi
> and points the original symbol to the jump table entry instead:
>
> : /* jump table entry */
> jmp canonical.cfi
> ...
> : /* function body */
> ...
>
> As a result, the address taken in assembly, or other non-instrumented
> code always points to the jump table and therefore, can be used for
> indirect calls in instrumented code without tripping CFI checks.
>
> Signed-off-by: Sami Tolvanen
> Reviewed-by: Kees Cook
> Acked-by: Bjorn Helgaas# pci.h
Irrelevant to this series, but I checked when the FN attr was first
available in clang; clang-10. (That's the minimum supported version of
clang for the kernel, and this series depends on LTO which depends on
clang-12, so no additional guards are necessary).
Reviewed-by: Nick Desaulniers
> ---
> include/linux/compiler-clang.h | 1 +
> include/linux/compiler_types.h | 4
> include/linux/init.h | 4 ++--
> include/linux/pci.h| 4 ++--
> 4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 6de9d0c9377e..adbe76b203e2 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -63,3 +63,4 @@
> #endif
>
> #define __nocfi__attribute__((__no_sanitize__("cfi")))
> +#define __cficanonical __attribute__((__cfi_canonical_jump_table__))
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 796935a37e37..d29bda7f6ebd 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -246,6 +246,10 @@ struct ftrace_likely_data {
> # define __nocfi
> #endif
>
> +#ifndef __cficanonical
> +# define __cficanonical
> +#endif
> +
> #ifndef asm_volatile_goto
> #define asm_volatile_goto(x...) asm goto(x)
> #endif
> diff --git a/include/linux/init.h b/include/linux/init.h
> index b3ea15348fbd..045ad1650ed1 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -220,8 +220,8 @@ extern bool initcall_debug;
> __initcall_name(initstub, __iid, id)
>
> #define __define_initcall_stub(__stub, fn) \
> - int __init __stub(void);\
> - int __init __stub(void) \
> + int __init __cficanonical __stub(void); \
> + int __init __cficanonical __stub(void) \
> { \
> return fn();\
> } \
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 86c799c97b77..39684b72db91 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1944,8 +1944,8 @@ enum pci_fixup_pass {
> #ifdef CONFIG_LTO_CLANG
> #define __DECLARE_PCI_FIXUP_SECTION(sec, name, vendor, device, class, \
> class_shift, hook, stub) \
> - void stub(struct pci_dev *dev); \
> - void stub(struct pci_dev *dev) \
> + void __cficanonical stub(struct pci_dev *dev); \
> + void __cficanonical stub(struct pci_dev *dev) \
> { \
> hook(dev); \
> } \
> --
> 2.31.0.291.g576ba9dcdaf-goog
>
--
Thanks,
~Nick Desaulniers
Re: s390: kernel/entry.o: in function `sys_call_table_emu': (.rodata+0x1bc0): undefined reference to `__s390_'
(Replying to https://lore.kernel.org/linux-s390/ca+g9fytbw0hav5ooayck2rz_m2sj73krxpj0idzt+o8qtc1...@mail.gmail.com/) Yeah, our CI is failing today, too with the same error on linux-next: https://github.com/ClangBuiltLinux/continuous-integration2/runs/2138006304?check_suite_focus=true
[PATCH] scripts: stable: add script to validate backports
A common recurring mistake made when backporting patches to stable is
forgetting to check for additional commits tagged with `Fixes:`. This
script validates that local commits have a `commit upstream.`
line in their commit message, and whether any additional `Fixes:` shas
exist in the `master` branch but were not included. It can not know
about fixes yet to be discovered, or fixes sent to the mailing list but
not yet in mainline.
To save time, it avoids checking all of `master`, stopping early once
we've reached the commit time of the earliest backport. It takes 0.5s to
validate 2 patches to linux-5.4.y when master is v5.12-rc3 and 5s to
validate 27 patches to linux-4.19.y. It does not recheck dependencies of
found fixes; the user is expected to run this script to a fixed point.
It depnds on pygit2 python library for working with git, which can be
installed via:
$ pip3 install pygit2
It's expected to be run from a stable tree with commits applied. For
example, consider 3cce9d44321e which is a fix for f77ac2e378be. Let's
say I cherry picked f77ac2e378be into linux-5.4.y but forgot
3cce9d44321e (true story). If I ran:
$ ./scripts/stable/check_backports.py
Checking 1 local commits for additional Fixes: in master
Please consider backporting 3cce9d44321e as a fix for f77ac2e378be
So then I could cherry pick 3cce9d44321e as well:
$ git cherry-pick -sx 3cce9d44321e
$ ./scripts/stable/check_backports.py
...
Exception: Missing 'commit upstream.' line
Oops, let me fixup the commit message and retry.
$ git commit --amend
$ ./scripts/stable/check_backports.py
Checking 2 local commits for additional Fixes: in master
$ echo $?
0
This allows for client side validation by the backports author, and
server side validation by the stable kernel maintainers.
Signed-off-by: Nick Desaulniers
---
MAINTAINERS | 1 +
scripts/stable/check_backports.py | 92 +++
2 files changed, 93 insertions(+)
create mode 100755 scripts/stable/check_backports.py
diff --git a/MAINTAINERS b/MAINTAINERS
index aa84121c5611..a8639e9277c4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16960,6 +16960,7 @@ M: Sasha Levin
L: sta...@vger.kernel.org
S: Supported
F: Documentation/process/stable-kernel-rules.rst
+F: scripts/stable/
STAGING - ATOMISP DRIVER
M: Mauro Carvalho Chehab
diff --git a/scripts/stable/check_backports.py
b/scripts/stable/check_backports.py
new file mode 100755
index ..529294e247ca
--- /dev/null
+++ b/scripts/stable/check_backports.py
@@ -0,0 +1,92 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2021 Google, Inc.
+
+import os
+import re
+import sys
+
+import pygit2 as pg
+
+
+def get_head_branch(repo):
+# Walk the branches to find which is HEAD.
+for branch_name in repo.branches:
+branch = repo.branches[branch_name]
+if branch.is_head():
+return branch
+
+
+def get_local_commits(repo):
+head_branch = get_head_branch(repo)
+# Walk the HEAD ref until we hit the first commit from the upstream.
+walker = repo.walk(repo.head.target)
+upstream_branch = head_branch.upstream
+upstream_commit, _ = repo.resolve_refish(upstream_branch.name)
+walker.hide(upstream_commit.id)
+commits = [commit for commit in walker]
+if not len(commits):
+raise Exception("No local commits")
+return commits
+
+
+def get_upstream_shas(commits):
+upstream_shas = []
+prog = re.compile('commit ([0-9a-f]{40}) upstream.')
+# For each line of each commit message, record the
+# "commit upstream." line.
+for commit in commits:
+found_upstream_line = False
+for line in commit.message.splitlines():
+result = prog.search(line)
+if result:
+upstream_shas.append(result.group(1)[:12])
+found_upstream_line = True
+break
+if not found_upstream_line:
+raise Exception("Missing 'commit upstream.' line")
+return upstream_shas
+
+
+def get_oldest_commit_time(repo, shas):
+commit_times = [repo.resolve_refish(sha)[0].commit_time for sha in shas]
+return sorted(commit_times)[0]
+
+
+def get_fixes_for(shas):
+shas = set(shas)
+prog = re.compile("Fixes: ([0-9a-f]{12,40})")
+# Walk commits in the master branch.
+master_commit, master_ref = repo.resolve_refish("master")
+walker = repo.walk(master_ref.target)
+oldest_commit_time = get_oldest_commit_time(repo, shas)
+fixes = []
+for commit in walker:
+# It's not possible for a Fixes: to be committed before a fixed tag, so
+# don't iterate all of git history.
+if commit.commit_time < oldest_commit_time:
+break
+for line in reversed(commit.message.splitlines()):
+result = prog.search(line)
+if not result:
+
Re: [PATCH] memblock: fix section mismatch warning again
On Tue, Mar 16, 2021 at 10:13 AM Mike Rapoport wrote:
>
> From: Mike Rapoport
>
> Commit 34dc2efb39a2 ("memblock: fix section mismatch warning") marked
> memblock_bottom_up() and memblock_set_bottom_up() as __init, but they could
> be referenced from non-init functions like memblock_find_in_range_node() on
> architectures that enable CONFIG_ARCH_KEEP_MEMBLOCK.
>
> For such builds kernel test robot reports:
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
> >> WARNING: modpost: vmlinux.o(.text+0x74fea4): Section mismatch in reference
> >> from the function memblock_find_in_range_node() to the function
> >> .init.text:memblock_bottom_up()
> The function memblock_find_in_range_node() references
> the function __init memblock_bottom_up().
> This is often because memblock_find_in_range_node lacks a __init
> annotation or the annotation of memblock_bottom_up is wrong.
>
> Replace __init annotations with __init_memblock annotations so that the
> appropriate section will be selected depending on
> CONFIG_ARCH_KEEP_MEMBLOCK.
>
> Link: https://lore.kernel.org/lkml/202103160133.uzhgy0wt-...@intel.com
> Fixes: 34dc2efb39a2 ("memblock: fix section mismatch warning")
> Signed-off-by: Mike Rapoport
> Reported-by: kernel test robot
> Reviewed-by: Arnd Bergmann
Thank you Mike.
Acked-by: Nick Desaulniers
> ---
>
> @Andrew, please let me know if you'd prefer this merged via memblock tree.
>
> include/linux/memblock.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index d13e3cd938b4..5984fff3f175 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -460,7 +460,7 @@ static inline void memblock_free_late(phys_addr_t base,
> phys_addr_t size)
> /*
> * Set the allocation direction to bottom-up or top-down.
> */
> -static inline __init void memblock_set_bottom_up(bool enable)
> +static inline __init_memblock void memblock_set_bottom_up(bool enable)
> {
> memblock.bottom_up = enable;
> }
> @@ -470,7 +470,7 @@ static inline __init void memblock_set_bottom_up(bool
> enable)
> * if this is true, that said, memblock will allocate memory
> * in bottom-up direction.
> */
> -static inline __init bool memblock_bottom_up(void)
> +static inline __init_memblock bool memblock_bottom_up(void)
> {
> return memblock.bottom_up;
> }
> --
> 2.28.0
>
--
Thanks,
~Nick Desaulniers
Re: WARNING: modpost: vmlinux.o(.text+0x74fea4): Section mismatch in reference from the function memblock_find_in_range_node() to the function .init.text:memblock_bottom_up()
On Tue, Mar 16, 2021 at 12:04 AM Mike Rapoport wrote:
>
> On Tue, Mar 16, 2021 at 01:23:08AM +0800, kernel test robot wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > master
> > head: 1e28eed17697bcf343c6743f0028cc3b5dd88bf0
> > commit: 34dc2efb39a231280fd6696a59bbe712bf3c5c4a memblock: fix section
> > mismatch warning
> > date: 2 days ago
> > config: arm64-randconfig-r013-20210315 (attached as .config)
> > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project
> > a28facba1ccdc957f386b7753f4958307f1bfde8)
> > reproduce (this is a W=1 build):
> > wget
> > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> > ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # install arm64 cross compiling tool for clang build
> > # apt-get install binutils-aarch64-linux-gnu
> > #
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=34dc2efb39a231280fd6696a59bbe712bf3c5c4a
> > git remote add linus
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > git fetch --no-tags linus master
> > git checkout 34dc2efb39a231280fd6696a59bbe712bf3c5c4a
> > # save the attached .config to linux build tree
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross
> > ARCH=arm64
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot
> >
> > All warnings (new ones prefixed by >>, old ones prefixed by <<):
> >
> > >> WARNING: modpost: vmlinux.o(.text+0x74fea4): Section mismatch in
> > >> reference from the function memblock_find_in_range_node() to the
> > >> function .init.text:memblock_bottom_up()
> > The function memblock_find_in_range_node() references
> > the function __init memblock_bottom_up().
> > This is often because memblock_find_in_range_node lacks a __init
> > annotation or the annotation of memblock_bottom_up is wrong.
>
> I don't have clang-13 setup handy so I could not check, but I think this
> should be the fix:
Thanks for taking another look:
https://lore.kernel.org/lkml/20210225205908.gm1447...@kernel.org/
Do we want to switch the above to the below?
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index d13e3cd938b4..5984fff3f175 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -460,7 +460,7 @@ static inline void memblock_free_late(phys_addr_t base,
> phys_addr_t size)
> /*
> * Set the allocation direction to bottom-up or top-down.
> */
> -static inline __init void memblock_set_bottom_up(bool enable)
> +static inline __init_memblock void memblock_set_bottom_up(bool enable)
> {
> memblock.bottom_up = enable;
> }
> @@ -470,7 +470,7 @@ static inline __init void memblock_set_bottom_up(bool
> enable)
> * if this is true, that said, memblock will allocate memory
> * in bottom-up direction.
> */
> -static inline __init bool memblock_bottom_up(void)
> +static inline __init_memblock bool memblock_bottom_up(void)
> {
> return memblock.bottom_up;
> }
>
>
> --
> Sincerely yours,
> Mike.
>
> --
> You received this message because you are subscribed to the Google Groups
> "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to clang-built-linux+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/clang-built-linux/YFBYWjtWJrnGyiVp%40linux.ibm.com.
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 3/3] kbuild: dwarf: use AS_VERSION instead of test_dwarf5_support.sh
On Mon, Mar 15, 2021 at 9:13 AM Masahiro Yamada wrote: > > The test code in scripts/test_dwarf5_support.sh is somewhat difficult > to understand, but after all, we want to check binutils >= 2.35.2 > > From the former discussion, the requirement for generating DWARF v5 from > C code is as follows: > > - gcc + gnu as -> requires gcc 5.0+ (but 7.0+ for full support) > - clang + gnu as-> requires binutils 2.35.2+ > - clang + integrated as -> OK > > Signed-off-by: Masahiro Yamada > Reviewed-by: Nathan Chancellor Reviewed-by: Nick Desaulniers > --- > > Changes in v2: > - fix typos > - simplify the dependency expression > > lib/Kconfig.debug | 3 +-- > scripts/test_dwarf5_support.sh | 8 > 2 files changed, 1 insertion(+), 10 deletions(-) > delete mode 100755 scripts/test_dwarf5_support.sh > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index b479ae609a31..c85d5f7a1aeb 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -284,8 +284,7 @@ config DEBUG_INFO_DWARF4 > > config DEBUG_INFO_DWARF5 > bool "Generate DWARF Version 5 debuginfo" > - depends on GCC_VERSION >= 5 || CC_IS_CLANG > - depends on CC_IS_GCC || > $(success,$(srctree)/scripts/test_dwarf5_support.sh $(CC) $(CLANG_FLAGS)) > + depends on GCC_VERSION >= 5 || (CC_IS_CLANG && (AS_IS_LLVM || > (AS_IS_GNU && AS_VERSION >= 23502))) > depends on !DEBUG_INFO_BTF > help > Generate DWARF v5 debug info. Requires binutils 2.35.2, gcc 5.0+ > (gcc > diff --git a/scripts/test_dwarf5_support.sh b/scripts/test_dwarf5_support.sh > deleted file mode 100755 > index c46e2456b47a.. > --- a/scripts/test_dwarf5_support.sh > +++ /dev/null > @@ -1,8 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > - > -# Test that the assembler doesn't need -Wa,-gdwarf-5 when presented with > DWARF > -# v5 input, such as `.file 0` and `md5 0x00`. Should be fixed in GNU binutils > -# 2.35.2. https://sourceware.org/bugzilla/show_bug.cgi?id=25611 > -echo '.file 0 "filename" md5 0x7a0b65214090b6693bd1dc24dd248245' | \ > - $* -gdwarf-5 -Wno-unused-command-line-argument -c -x assembler -o > /dev/null - > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers On Mon, Mar 15, 2021 at 9:13 AM Masahiro Yamada wrote: > > The test code in scripts/test_dwarf5_support.sh is somewhat difficult > to understand, but after all, we want to check binutils >= 2.35.2 > > From the former discussion, the requirement for generating DWARF v5 from > C code is as follows: > > - gcc + gnu as -> requires gcc 5.0+ (but 7.0+ for full support) > - clang + gnu as-> requires binutils 2.35.2+ > - clang + integrated as -> OK > > Signed-off-by: Masahiro Yamada > Reviewed-by: Nathan Chancellor > --- > > Changes in v2: > - fix typos > - simplify the dependency expression > > lib/Kconfig.debug | 3 +-- > scripts/test_dwarf5_support.sh | 8 > 2 files changed, 1 insertion(+), 10 deletions(-) > delete mode 100755 scripts/test_dwarf5_support.sh > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index b479ae609a31..c85d5f7a1aeb 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -284,8 +284,7 @@ config DEBUG_INFO_DWARF4 > > config DEBUG_INFO_DWARF5 > bool "Generate DWARF Version 5 debuginfo" > - depends on GCC_VERSION >= 5 || CC_IS_CLANG > - depends on CC_IS_GCC || > $(success,$(srctree)/scripts/test_dwarf5_support.sh $(CC) $(CLANG_FLAGS)) > + depends on GCC_VERSION >= 5 || (CC_IS_CLANG && (AS_IS_LLVM || > (AS_IS_GNU && AS_VERSION >= 23502))) > depends on !DEBUG_INFO_BTF > help > Generate DWARF v5 debug info. Requires binutils 2.35.2, gcc 5.0+ > (gcc > diff --git a/scripts/test_dwarf5_support.sh b/scripts/test_dwarf5_support.sh > deleted file mode 100755 > index c46e2456b47a.. > --- a/scripts/test_dwarf5_support.sh > +++ /dev/null > @@ -1,8 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > - > -# Test that the assembler doesn't need -Wa,-gdwarf-5 when presented with > DWARF > -# v5 input, such as `.file 0` and `md5 0x00`. Should be fixed in GNU binutils > -# 2.35.2. https://sourceware.org/bugzilla/show_bug.cgi?id=25611 > -echo '.file 0 "filename" md5 0x7a0b65214090b6693bd1dc24dd248245' | \ > - $* -gdwarf-5 -Wno-unused-command-line-argument -c -x assembler -o > /dev/null - > -- > 2.27.0 > -- Thanks, ~Nick Desaulniers
[PATCH v3] ASoC: Intel: Skylake: skl-topology: fix -frame-larger-than
Fixes:
sound/soc/intel/skylake/skl-topology.c:3613:13: warning: stack frame
size of 1304 bytes in function 'skl_tplg_complete'
[-Wframe-larger-than=]
struct snd_ctl_elem_value is 1224 bytes in my configuration.
Heap allocate it, then free it within the current frame.
Suggested-by: Andy Shevchenko
Signed-off-by: Nick Desaulniers
---
Changes V2 -> V3:
* change to kmalloc+memset to fix logic error, as per Andy.
Changes V1 -> V2:
* rebased on mainline.
sound/soc/intel/skylake/skl-topology.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-topology.c
b/sound/soc/intel/skylake/skl-topology.c
index b824086203b9..c0fdab39e7c2 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3613,10 +3613,15 @@ static int skl_manifest_load(struct snd_soc_component
*cmpnt, int index,
static void skl_tplg_complete(struct snd_soc_component *component)
{
struct snd_soc_dobj *dobj;
- struct snd_soc_acpi_mach *mach =
- dev_get_platdata(component->card->dev);
+ struct snd_soc_acpi_mach *mach;
+ struct snd_ctl_elem_value *val;
int i;
+ val = kmalloc(sizeof(*val), GFP_KERNEL);
+ if (!val)
+ return;
+
+ mach = dev_get_platdata(component->card->dev);
list_for_each_entry(dobj, >dobj_list, list) {
struct snd_kcontrol *kcontrol = dobj->control.kcontrol;
struct soc_enum *se;
@@ -3632,14 +3637,14 @@ static void skl_tplg_complete(struct snd_soc_component
*component)
sprintf(chan_text, "c%d", mach->mach_params.dmic_num);
for (i = 0; i < se->items; i++) {
- struct snd_ctl_elem_value val = {};
-
if (strstr(texts[i], chan_text)) {
- val.value.enumerated.item[0] = i;
- kcontrol->put(kcontrol, );
+ memset(val, 0, sizeof(*val));
+ val->value.enumerated.item[0] = i;
+ kcontrol->put(kcontrol, val);
}
}
}
+ kfree(val);
}
static struct snd_soc_tplg_ops skl_tplg_ops = {
base-commit: 88fe49249c99de14e543c632a46248d85411ab9e
--
2.25.1
[PATCH v2] ASoC: Intel: Skylake: skl-topology: fix -frame-larger-than
Fixes:
sound/soc/intel/skylake/skl-topology.c:3613:13: warning: stack frame
size of 1304 bytes in function 'skl_tplg_complete'
[-Wframe-larger-than=]
struct snd_ctl_elem_value is 1224 bytes in my configuration.
Heap allocate it, then free it within the current frame.
Signed-off-by: Nick Desaulniers
---
Changes V1 -> V2: rebased on mainline.
sound/soc/intel/skylake/skl-topology.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-topology.c
b/sound/soc/intel/skylake/skl-topology.c
index b824086203b9..566d07b4b523 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3613,10 +3613,15 @@ static int skl_manifest_load(struct snd_soc_component
*cmpnt, int index,
static void skl_tplg_complete(struct snd_soc_component *component)
{
struct snd_soc_dobj *dobj;
- struct snd_soc_acpi_mach *mach =
- dev_get_platdata(component->card->dev);
+ struct snd_soc_acpi_mach *mach;
+ struct snd_ctl_elem_value *val;
int i;
+ val = kzalloc(sizeof(*val), GFP_KERNEL);
+ if (!val)
+ return;
+
+ mach = dev_get_platdata(component->card->dev);
list_for_each_entry(dobj, >dobj_list, list) {
struct snd_kcontrol *kcontrol = dobj->control.kcontrol;
struct soc_enum *se;
@@ -3632,14 +3637,13 @@ static void skl_tplg_complete(struct snd_soc_component
*component)
sprintf(chan_text, "c%d", mach->mach_params.dmic_num);
for (i = 0; i < se->items; i++) {
- struct snd_ctl_elem_value val = {};
-
if (strstr(texts[i], chan_text)) {
- val.value.enumerated.item[0] = i;
- kcontrol->put(kcontrol, );
+ val->value.enumerated.item[0] = i;
+ kcontrol->put(kcontrol, val);
}
}
}
+ kfree(val);
}
static struct snd_soc_tplg_ops skl_tplg_ops = {
base-commit: 88fe49249c99de14e543c632a46248d85411ab9e
--
2.25.1
[PATCH] ASoC: Intel: Skylake: skl-topology: fix -frame-larger-than
Fixes:
sound/soc/intel/skylake/skl-topology.c:3613:13: warning: stack frame
size of 1304 bytes in function 'skl_tplg_complete'
[-Wframe-larger-than=]
struct snd_ctl_elem_value is 1224 bytes in my configuration.
Heap allocate it, then free it within the current frame.
Signed-off-by: Nick Desaulniers
---
sound/soc/intel/skylake/skl-topology.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/sound/soc/intel/skylake/skl-topology.c
b/sound/soc/intel/skylake/skl-topology.c
index ae466cd59292..cdc916c91301 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -3613,10 +3613,15 @@ static int skl_manifest_load(struct snd_soc_component
*cmpnt, int index,
static void skl_tplg_complete(struct snd_soc_component *component)
{
struct snd_soc_dobj *dobj;
- struct snd_soc_acpi_mach *mach =
- dev_get_platdata(component->card->dev);
+ struct snd_soc_acpi_mach *mach;
+ struct snd_ctl_elem_value *val;
int i;
+ val = kzalloc(sizeof(*val), GFP_KERNEL);
+ if (!val)
+ return;
+
+ mach = dev_get_platdata(component->card->dev);
list_for_each_entry(dobj, >dobj_list, list) {
struct snd_kcontrol *kcontrol = dobj->control.kcontrol;
struct soc_enum *se =
@@ -3631,14 +3636,13 @@ static void skl_tplg_complete(struct snd_soc_component
*component)
sprintf(chan_text, "c%d", mach->mach_params.dmic_num);
for (i = 0; i < se->items; i++) {
- struct snd_ctl_elem_value val;
-
if (strstr(texts[i], chan_text)) {
- val.value.enumerated.item[0] = i;
- kcontrol->put(kcontrol, );
+ val->value.enumerated.item[0] = i;
+ kcontrol->put(kcontrol, val);
}
}
}
+ kfree(val);
}
static struct snd_soc_tplg_ops skl_tplg_ops = {
base-commit: 65f0d2414b7079556fbbcc070b3d1c9f9587606d
prerequisite-patch-id: 4d05aad8c2b50c0c3b4447dd842abe8b1b840927
--
2.25.1
[PATCH v2 2/2] gcov: clang: drop support for clang-10 and older
LLVM changed the expected function signatures for llvm_gcda_start_file()
and llvm_gcda_emit_function() in the clang-11 release. Drop the older
implementations and require folks to upgrade their compiler if they're
interested in GCOV support.
Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
Suggested-by: Nathan Chancellor
Signed-off-by: Nick Desaulniers
---
For an easier time reviewing this series, reviewers may want to apply
these patches, then check the overall diff with `git diff origin/HEAD`.
kernel/gcov/Kconfig | 1 +
kernel/gcov/clang.c | 85 -
2 files changed, 1 insertion(+), 85 deletions(-)
diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index f62de2dea8a3..58f87a3092f3 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -4,6 +4,7 @@ menu "GCOV-based kernel profiling"
config GCOV_KERNEL
bool "Enable gcov-based kernel profiling"
depends on DEBUG_FS
+ depends on !CC_IS_CLANG || CLANG_VERSION >= 11
select CONSTRUCTORS
default n
help
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index 8743150db2ac..14de5644b5cc 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -75,9 +75,6 @@ struct gcov_fn_info {
u32 num_counters;
u64 *counters;
-#if CONFIG_CLANG_VERSION < 11
- const char *function_name;
-#endif
};
static struct gcov_info *current_info;
@@ -107,16 +104,6 @@ void llvm_gcov_init(llvm_gcov_callback writeout,
llvm_gcov_callback flush)
}
EXPORT_SYMBOL(llvm_gcov_init);
-#if CONFIG_CLANG_VERSION < 11
-void llvm_gcda_start_file(const char *orig_filename, const char version[4],
- u32 checksum)
-{
- current_info->filename = orig_filename;
- memcpy(_info->version, version, sizeof(current_info->version));
- current_info->checksum = checksum;
-}
-EXPORT_SYMBOL(llvm_gcda_start_file);
-#else
void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
{
current_info->filename = orig_filename;
@@ -124,29 +111,7 @@ void llvm_gcda_start_file(const char *orig_filename, u32
version, u32 checksum)
current_info->checksum = checksum;
}
EXPORT_SYMBOL(llvm_gcda_start_file);
-#endif
-
-#if CONFIG_CLANG_VERSION < 11
-void llvm_gcda_emit_function(u32 ident, const char *function_name,
- u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
-{
- struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
-
- if (!info)
- return;
- INIT_LIST_HEAD(>head);
- info->ident = ident;
- info->checksum = func_checksum;
- info->use_extra_checksum = use_extra_checksum;
- info->cfg_checksum = cfg_checksum;
- if (function_name)
- info->function_name = kstrdup(function_name, GFP_KERNEL);
-
- list_add_tail(>head, _info->functions);
-}
-EXPORT_SYMBOL(llvm_gcda_emit_function);
-#else
void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
u8 use_extra_checksum, u32 cfg_checksum)
{
@@ -163,7 +128,6 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
list_add_tail(>head, _info->functions);
}
EXPORT_SYMBOL(llvm_gcda_emit_function);
-#endif
void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
{
@@ -326,7 +290,6 @@ void gcov_info_add(struct gcov_info *dst, struct gcov_info
*src)
}
}
-#if CONFIG_CLANG_VERSION < 11
static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
{
size_t cv_size; /* counter values size */
@@ -335,47 +298,15 @@ static struct gcov_fn_info *gcov_fn_info_dup(struct
gcov_fn_info *fn)
if (!fn_dup)
return NULL;
INIT_LIST_HEAD(_dup->head);
-
- fn_dup->function_name = kstrdup(fn->function_name, GFP_KERNEL);
- if (!fn_dup->function_name)
- goto err_name;
-
- cv_size = fn->num_counters * sizeof(fn->counters[0]);
- fn_dup->counters = vmalloc(cv_size);
- if (!fn_dup->counters)
- goto err_counters;
- memcpy(fn_dup->counters, fn->counters, cv_size);
-
- return fn_dup;
-
-err_counters:
- kfree(fn_dup->function_name);
-err_name:
- kfree(fn_dup);
- return NULL;
-}
-#else
-static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
-{
- size_t cv_size; /* counter values size */
- struct gcov_fn_info *fn_dup = kmemdup(fn, sizeof(*fn),
- GFP_KERNEL);
- if (!fn_dup)
- return NULL;
- INIT_LIST_HEAD(_dup->head);
-
cv_size = fn->num_counters * sizeof(fn->counters[0]);
fn_dup->counters = vmalloc(cv_size);
if (!fn_dup->counters) {
kfree(fn_dup);
[PATCH v2 1/2] gcov: fix clang-11+ support
LLVM changed the expected function signatures for llvm_gcda_start_file()
and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
or newer may have noticed their kernels failing to boot due to a panic
when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
the function signatures so calling these functions doesn't panic the
kernel.
Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
Cc: sta...@vger.kernel.org # 5.4
Reported-by: Prasad Sodagudi
Suggested-by: Nathan Chancellor
Reviewed-by: Fangrui Song
Signed-off-by: Nick Desaulniers
Tested-by: Nathan Chancellor
---
Changes V1 -> V2:
* Use CONFIG_CLANG_VERSION instead of __clang_major__.
* Pick up and retain Suggested-by, Tested-by, and Reviewed-by tags.
* Drop note from commit message about `git blame`; I did what was
sugguested in V1, but it still looks to git like I wrote those
functions. Oh well.
kernel/gcov/clang.c | 69 +
1 file changed, 69 insertions(+)
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index c94b820a1b62..8743150db2ac 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -75,7 +75,9 @@ struct gcov_fn_info {
u32 num_counters;
u64 *counters;
+#if CONFIG_CLANG_VERSION < 11
const char *function_name;
+#endif
};
static struct gcov_info *current_info;
@@ -105,6 +107,7 @@ void llvm_gcov_init(llvm_gcov_callback writeout,
llvm_gcov_callback flush)
}
EXPORT_SYMBOL(llvm_gcov_init);
+#if CONFIG_CLANG_VERSION < 11
void llvm_gcda_start_file(const char *orig_filename, const char version[4],
u32 checksum)
{
@@ -113,7 +116,17 @@ void llvm_gcda_start_file(const char *orig_filename, const
char version[4],
current_info->checksum = checksum;
}
EXPORT_SYMBOL(llvm_gcda_start_file);
+#else
+void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
+{
+ current_info->filename = orig_filename;
+ current_info->version = version;
+ current_info->checksum = checksum;
+}
+EXPORT_SYMBOL(llvm_gcda_start_file);
+#endif
+#if CONFIG_CLANG_VERSION < 11
void llvm_gcda_emit_function(u32 ident, const char *function_name,
u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
{
@@ -133,6 +146,24 @@ void llvm_gcda_emit_function(u32 ident, const char
*function_name,
list_add_tail(>head, _info->functions);
}
EXPORT_SYMBOL(llvm_gcda_emit_function);
+#else
+void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
+ u8 use_extra_checksum, u32 cfg_checksum)
+{
+ struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
+
+ if (!info)
+ return;
+
+ INIT_LIST_HEAD(>head);
+ info->ident = ident;
+ info->checksum = func_checksum;
+ info->use_extra_checksum = use_extra_checksum;
+ info->cfg_checksum = cfg_checksum;
+ list_add_tail(>head, _info->functions);
+}
+EXPORT_SYMBOL(llvm_gcda_emit_function);
+#endif
void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
{
@@ -295,6 +326,7 @@ void gcov_info_add(struct gcov_info *dst, struct gcov_info
*src)
}
}
+#if CONFIG_CLANG_VERSION < 11
static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
{
size_t cv_size; /* counter values size */
@@ -322,6 +354,28 @@ static struct gcov_fn_info *gcov_fn_info_dup(struct
gcov_fn_info *fn)
kfree(fn_dup);
return NULL;
}
+#else
+static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
+{
+ size_t cv_size; /* counter values size */
+ struct gcov_fn_info *fn_dup = kmemdup(fn, sizeof(*fn),
+ GFP_KERNEL);
+ if (!fn_dup)
+ return NULL;
+ INIT_LIST_HEAD(_dup->head);
+
+ cv_size = fn->num_counters * sizeof(fn->counters[0]);
+ fn_dup->counters = vmalloc(cv_size);
+ if (!fn_dup->counters) {
+ kfree(fn_dup);
+ return NULL;
+ }
+
+ memcpy(fn_dup->counters, fn->counters, cv_size);
+
+ return fn_dup;
+}
+#endif
/**
* gcov_info_dup - duplicate profiling data set
@@ -362,6 +416,7 @@ struct gcov_info *gcov_info_dup(struct gcov_info *info)
* gcov_info_free - release memory for profiling data set duplicate
* @info: profiling data set duplicate to free
*/
+#if CONFIG_CLANG_VERSION < 11
void gcov_info_free(struct gcov_info *info)
{
struct gcov_fn_info *fn, *tmp;
@@ -375,6 +430,20 @@ void gcov_info_free(struct gcov_info *info)
kfree(info->filename);
kfree(info);
}
+#else
+void gcov_info_free(struct gcov_info *info)
+{
+ struct gcov_fn_info *fn, *tmp;
+
+ list_for_each_entry_safe(fn, tmp, >functions, head) {
+ vfree(fn->counters);
+
[PATCH v2 0/2] gcov fixes for clang-11
LLVM changed the expected function signatures for llvm_gcda_start_file() and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11 or newer may have noticed their kernels failing to boot due to a panic when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up the function signatures so calling these functions doesn't panic the kernel. The first patch should allow us to backport it to stable; the second drops support for older toolchains. Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041 Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44 Nick Desaulniers (2): gcov: fix clang-11+ support gcov: clang: drop support for clang-10 and older kernel/gcov/Kconfig | 1 + kernel/gcov/clang.c | 32 2 files changed, 9 insertions(+), 24 deletions(-) base-commit: f78d76e72a4671ea52d12752d92077788b4f5d50 -- 2.31.0.rc2.261.g7f71774620-goog
Re: [PATCH] gcov: fix clang-11+ support
On Fri, Mar 12, 2021 at 12:25 PM 'Fangrui Song' via Clang Built Linux
wrote:
>
> function_name can be unconditionally deleted. It is not used by llvm-cov
> gcov. You'll need to delete a few assignments to gcov_info_free but you
> can then unify the gcov_fn_info_dup and gcov_info_free implementations.
>
> LG. On big-endian systems, clang < 11 emitted .gcno/.gcda files do not
> work with llvm-cov gcov < 11. To fix it and make .gcno/.gcda work with
> gcc gcov I chose to break compatibility (and make all the breaking
> changes like deleting some CC1 options) in a short window. At that time
> I was not aware that there is the kernel implementation. Later on I was
> CCed on a few https://github.com/ClangBuiltLinux/linux/ gcov issues but
> I forgot to mention the interface change.
These are all good suggestions. Since in v2 I'll drop support for
clang < 11, I will skip additional patches to disable GCOV when using
older clang for BE, and the function_name cleanup.
> Now in clang 11 onward, clang --coverage defaults to the gcov 4.8
> compatible format. You can specify the CC1 option (internal option,
> subject to change) -coverage-version to make it compatible with other
> versions' gcov.
>
> -Xclang -coverage-version='407*' => 4.7
> -Xclang -coverage-version='704*' => 7.4
> -Xclang -coverage-version='B02*' => 10.2 (('B'-'A')*10 = 10)
How come LLVM doesn't default to 10.2 format, if it can optionally
produce it? We might be able to reuse more code in the kernel between
the two impelementations, though I expect the symbols the runtime is
expected to provide will still differ. Seeing the `B` in `B02*` is
also curious.
Thanks for the review, will include your tag in v2.
--
Thanks,
~Nick Desaulniers
Re: [PATCH] gcov: fix clang-11+ support
On Fri, Mar 12, 2021 at 12:51 PM Nathan Chancellor wrote:
>
> On Fri, Mar 12, 2021 at 12:14:42PM -0800, Nick Desaulniers wrote:
> > On Fri, Mar 12, 2021 at 11:58 AM Nathan Chancellor
> > wrote:
> > >
> > > On Fri, Mar 12, 2021 at 11:21:39AM -0800, Nick Desaulniers wrote:
> > > > LLVM changed the expected function signatures for llvm_gcda_start_file()
> > > > and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
> > > > or newer may have noticed their kernels failing to boot due to a panic
> > > > when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
> > > > the function signatures so calling these functions doesn't panic the
> > > > kernel.
> > > >
> > > > When we drop clang-10 support from the kernel, we should carefully
> > > > update the original implementations to try to preserve git blame,
> > > > deleting these implementations.
> > > >
> > > > Link:
> > > > https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> > > > Link:
> > > > https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> > > > Cc: Fangrui Song
> > > > Reported-by: Prasad Sodagudi
> > > > Signed-off-by: Nick Desaulniers
> > >
> > > I can reproduce the panic (as a boot hang) in QEMU before this patch and
> > > it is resolved after it so:
> > >
> > > Tested-by: Nathan Chancellor
> > >
> > > However, the duplication hurts :( would it potentially be better to just
> > > do the full update to clang-11+ and require it for CONFIG_GCOV_KERNEL?
> > >
> > > depends on CC_IS_GCC || CLANG_VERSION >= 11?
> >
> > I'm not opposed, and value your input on the matter. Either way, this
> > will need to be back ported to stable. Should we be concerned with
> > users of stable's branches before we mandated clang-10 as the minimum
> > supported version?
> >
> > commit 1f7a44f63e6c ("compiler-clang: add build check for clang 10.0.1")
> >
> > first landed in v5.10-rc1. Does not exist in v5.4.y. The diff you
>
> Hmmm fair point, I did not realize that this support had landed in 5.2
> meaning that 5.4 needs it as well at 5.10.
>
> > suggest is certainly easier to review to observe the differences, and
> > I we don't have users of the latest Android or CrOS kernels using
> > older clang, but I suspect there may be older kernel versions where if
> > they try to upgrade their version of clang, GCOV support will regress
> > for them. Though, I guess that's fine since either approach will fix
> > this for them. I guess if they don't want to upgrade from clang-10 say
> > for example, then this approach can be backported to stable.
>
> If people are happy with this approach, it is the more "stable friendly"
> change because it fixes it for all versions of clang that should have
> been supported at their respective times. Maybe it is worthwhile to do
> both? This change gets picked up with a Cc: sta...@vger.kernel.org then
> in a follow up patch, we remove the #ifdef's and gate GCOV on clang-11?
> The CLANG_VERSION string is usually what we will search for when
> removing old workarounds.
Sounds like we're on the same page; will send a v2 with your
recommendation on top.
> Additionally, your patch could just use
>
> #if CLANG_VERSION <= 11
>
> to more easily see this. I have no strong opinion one way or the other
> though. If people are happy with this approach, let's do it.
Err that would be nicer, but:
kernel/gcov/clang.c:78:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 11
^
kernel/gcov/clang.c:110:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 11
^
kernel/gcov/clang.c:130:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 11
^
kernel/gcov/clang.c:330:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 11
^
kernel/gcov/clang.c:420:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 11
^
Did we just break this in commit aec6c60a01d3 ("kbuild: check the
minimum compiler version in Kconfig") in v5.12-rc1? So I'll keep it
as is for v2, but we should discuss with Masahiro and Miguel if we
should be removing CLANG_VERSION even if there are no in tree users at
the moment. (I guess I could re-introduce it in my series for v2, but
that will unnecessarily complicate the backports, so I won't). My
fa
Re: [PATCH] Makefile: LTO: have linker check -Wframe-larger-than
On Fri, Mar 12, 2021 at 9:55 AM Nick Desaulniers wrote: > > On Thu, Mar 11, 2021 at 5:09 PM Nick Desaulniers > wrote: > > > > -Wframe-larger-than= requires stack frame information, which the > > frontend cannot provide. This diagnostic is emitted late during > > compilation once stack frame size is available. > > > > When building with LTO, the frontend simply lowers C to LLVM IR and does > > not have stack frame information, so it cannot emit this diagnostic. > > When the linker drives LTO, it restarts optimizations and lowers LLVM IR > > to object code. At that point, it has stack frame information but > > doesn't know to check for a specific max stack frame size. > > > > I consider this a bug in LLVM that we need to fix. There are some > > details we're working out related to LTO such as which value to use when > > there are multiple different values specified per TU, or how to > > propagate these to compiler synthesized routines properly, if at all. > > > > Until it's fixed, ensure we don't miss these. At that point we can wrap > > this in a compiler version guard or revert this based on the minimum > > support version of Clang. > > > > The error message is not generated during link: > > LTO vmlinux.o > > ld.lld: warning: stack size limit exceeded (8224) in foobarbaz > > > > Cc: Sami Tolvanen > > Reported-by: Candle Sun > > Suggested-by: Fangrui Song > > Signed-off-by: Nick Desaulniers > > --- > > LTO users might want to `make clean` or `rm -rf .thinlto-cache` to test > > this. > > > > Makefile | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/Makefile b/Makefile > > index f9b54da2fca0..74566b1417b8 100644 > > --- a/Makefile > > +++ b/Makefile > > Candle sent me a private message that we probably also want coverage > for kernel modules. Let me revise this and test/send a v2. False alarm, seems specific to Android's LTO support pre-5.11. I will fix that in Android trees. This patch is still relevant going forward. > > > @@ -910,6 +910,11 @@ CC_FLAGS_LTO += -fvisibility=hidden > > > > # Limit inlining across translation units to reduce binary size > > KBUILD_LDFLAGS += -mllvm -import-instr-limit=5 > > + > > +# Check for frame size exceeding threshold during prolog/epilog insertion. > > +ifneq ($(CONFIG_FRAME_WARN),0) > > +KBUILD_LDFLAGS += -plugin-opt=-warn-stack-size=$(CONFIG_FRAME_WARN) > > +endif > > endif > > > > ifdef CONFIG_LTO > > -- > > 2.31.0.rc2.261.g7f71774620-goog > > > > > -- > Thanks, > ~Nick Desaulniers -- Thanks, ~Nick Desaulniers
Re: [PATCH] gcov: fix clang-11+ support
On Fri, Mar 12, 2021 at 12:14 PM Nick Desaulniers
wrote:
>
> On Fri, Mar 12, 2021 at 11:58 AM Nathan Chancellor wrote:
> >
> > On Fri, Mar 12, 2021 at 11:21:39AM -0800, Nick Desaulniers wrote:
> > > LLVM changed the expected function signatures for llvm_gcda_start_file()
> > > and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
> > > or newer may have noticed their kernels failing to boot due to a panic
> > > when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
> > > the function signatures so calling these functions doesn't panic the
> > > kernel.
> > >
> > > When we drop clang-10 support from the kernel, we should carefully
> > > update the original implementations to try to preserve git blame,
> > > deleting these implementations.
> > >
> > > Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> > > Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> > > Cc: Fangrui Song
> > > Reported-by: Prasad Sodagudi
> > > Signed-off-by: Nick Desaulniers
> >
> > I can reproduce the panic (as a boot hang) in QEMU before this patch and
> > it is resolved after it so:
> >
> > Tested-by: Nathan Chancellor
> >
> > However, the duplication hurts :( would it potentially be better to just
> > do the full update to clang-11+ and require it for CONFIG_GCOV_KERNEL?
> >
> > depends on CC_IS_GCC || CLANG_VERSION >= 11?
>
> I'm not opposed, and value your input on the matter. Either way, this
> will need to be back ported to stable. Should we be concerned with
> users of stable's branches before we mandated clang-10 as the minimum
> supported version?
>
> commit 1f7a44f63e6c ("compiler-clang: add build check for clang 10.0.1")
>
> first landed in v5.10-rc1. Does not exist in v5.4.y. The diff you
> suggest is certainly easier to review to observe the differences, and
> I we don't have users of the latest Android or CrOS kernels using
> older clang, but I suspect there may be older kernel versions where if
> they try to upgrade their version of clang, GCOV support will regress
> for them. Though, I guess that's fine since either approach will fix
> this for them. I guess if they don't want to upgrade from clang-10 say
> for example, then this approach can be backported to stable.
Thinking more about this over lunch; what are your thoughts on a V2
that does this first, then what you suggest as a second patch on top,
with the first tagged for inclusion into stable, but the second one
not? Hopefully maintainers don't consider that too much churn?
>
> >
> > > ---
> > > kernel/gcov/clang.c | 69 +
> > > 1 file changed, 69 insertions(+)
> > >
> > > diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
> > > index c94b820a1b62..20e6760ec05d 100644
> > > --- a/kernel/gcov/clang.c
> > > +++ b/kernel/gcov/clang.c
> > > @@ -75,7 +75,9 @@ struct gcov_fn_info {
> > >
> > > u32 num_counters;
> > > u64 *counters;
> > > +#if __clang_major__ < 11
> > > const char *function_name;
> > > +#endif
> > > };
> > >
> > > static struct gcov_info *current_info;
> > > @@ -105,6 +107,7 @@ void llvm_gcov_init(llvm_gcov_callback writeout,
> > > llvm_gcov_callback flush)
> > > }
> > > EXPORT_SYMBOL(llvm_gcov_init);
> > >
> > > +#if __clang_major__ < 11
> > > void llvm_gcda_start_file(const char *orig_filename, const char
> > > version[4],
> > > u32 checksum)
> > > {
> > > @@ -113,7 +116,17 @@ void llvm_gcda_start_file(const char *orig_filename,
> > > const char version[4],
> > > current_info->checksum = checksum;
> > > }
> > > EXPORT_SYMBOL(llvm_gcda_start_file);
> > > +#else
> > > +void llvm_gcda_start_file(const char *orig_filename, u32 version, u32
> > > checksum)
> > > +{
> > > + current_info->filename = orig_filename;
> > > + current_info->version = version;
> > > + current_info->checksum = checksum;
> > > +}
> > > +EXPORT_SYMBOL(llvm_gcda_start_file);
> > > +#endif
> > >
> > > +#if __clang_major__ < 11
> > > void llvm_gcda_emit_function(u32 ident, const char *function_name,
> > > u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
> > > {
> > > @@ -133,
Re: [PATCH] gcov: fix clang-11+ support
On Fri, Mar 12, 2021 at 11:58 AM Nathan Chancellor wrote:
>
> On Fri, Mar 12, 2021 at 11:21:39AM -0800, Nick Desaulniers wrote:
> > LLVM changed the expected function signatures for llvm_gcda_start_file()
> > and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
> > or newer may have noticed their kernels failing to boot due to a panic
> > when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
> > the function signatures so calling these functions doesn't panic the
> > kernel.
> >
> > When we drop clang-10 support from the kernel, we should carefully
> > update the original implementations to try to preserve git blame,
> > deleting these implementations.
> >
> > Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> > Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> > Cc: Fangrui Song
> > Reported-by: Prasad Sodagudi
> > Signed-off-by: Nick Desaulniers
>
> I can reproduce the panic (as a boot hang) in QEMU before this patch and
> it is resolved after it so:
>
> Tested-by: Nathan Chancellor
>
> However, the duplication hurts :( would it potentially be better to just
> do the full update to clang-11+ and require it for CONFIG_GCOV_KERNEL?
>
> depends on CC_IS_GCC || CLANG_VERSION >= 11?
I'm not opposed, and value your input on the matter. Either way, this
will need to be back ported to stable. Should we be concerned with
users of stable's branches before we mandated clang-10 as the minimum
supported version?
commit 1f7a44f63e6c ("compiler-clang: add build check for clang 10.0.1")
first landed in v5.10-rc1. Does not exist in v5.4.y. The diff you
suggest is certainly easier to review to observe the differences, and
I we don't have users of the latest Android or CrOS kernels using
older clang, but I suspect there may be older kernel versions where if
they try to upgrade their version of clang, GCOV support will regress
for them. Though, I guess that's fine since either approach will fix
this for them. I guess if they don't want to upgrade from clang-10 say
for example, then this approach can be backported to stable.
>
> > ---
> > kernel/gcov/clang.c | 69 +
> > 1 file changed, 69 insertions(+)
> >
> > diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
> > index c94b820a1b62..20e6760ec05d 100644
> > --- a/kernel/gcov/clang.c
> > +++ b/kernel/gcov/clang.c
> > @@ -75,7 +75,9 @@ struct gcov_fn_info {
> >
> > u32 num_counters;
> > u64 *counters;
> > +#if __clang_major__ < 11
> > const char *function_name;
> > +#endif
> > };
> >
> > static struct gcov_info *current_info;
> > @@ -105,6 +107,7 @@ void llvm_gcov_init(llvm_gcov_callback writeout,
> > llvm_gcov_callback flush)
> > }
> > EXPORT_SYMBOL(llvm_gcov_init);
> >
> > +#if __clang_major__ < 11
> > void llvm_gcda_start_file(const char *orig_filename, const char version[4],
> > u32 checksum)
> > {
> > @@ -113,7 +116,17 @@ void llvm_gcda_start_file(const char *orig_filename,
> > const char version[4],
> > current_info->checksum = checksum;
> > }
> > EXPORT_SYMBOL(llvm_gcda_start_file);
> > +#else
> > +void llvm_gcda_start_file(const char *orig_filename, u32 version, u32
> > checksum)
> > +{
> > + current_info->filename = orig_filename;
> > + current_info->version = version;
> > + current_info->checksum = checksum;
> > +}
> > +EXPORT_SYMBOL(llvm_gcda_start_file);
> > +#endif
> >
> > +#if __clang_major__ < 11
> > void llvm_gcda_emit_function(u32 ident, const char *function_name,
> > u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
> > {
> > @@ -133,6 +146,24 @@ void llvm_gcda_emit_function(u32 ident, const char
> > *function_name,
> > list_add_tail(>head, _info->functions);
> > }
> > EXPORT_SYMBOL(llvm_gcda_emit_function);
> > +#else
> > +void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
> > + u8 use_extra_checksum, u32 cfg_checksum)
> > +{
> > + struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
> > +
> > + if (!info)
> > + return;
> > +
> > + INIT_LIST_HEAD(>head);
> > + info->ident = ident;
> > + info->checksum = func_checksum;
> > + info->use_extra_checksum = use_extra_checksum;
> > + info->cfg_checksum = cfg_checksum;
> > + list_add_tail(>head, _info-&g
Re: [PATCH] kbuild: fix ld-version.sh to not be affected by locale
On Fri, Mar 12, 2021 at 11:38 AM Masahiro Yamada wrote:
>
> ld-version.sh checks the output from $(LD) --version, but it has a
> problem on some locales.
>
> For example, in Italian:
>
> $ LC_MESSAGES=it_IT.UTF-8 ld --version | head -n 1
> ld di GNU (GNU Binutils for Debian) 2.35.2
>
> This makes ld-version.sh fail because it expects "GNU ld" for the
> BFD linker case.
>
> Add LC_ALL=C to override the user's locale.
>
> BTW, setting LC_MESSAGES=C (or LANG=C) is not enough because it is
> ineffective if LC_ALL is set on the user's environment.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=212105
> Reported-by: Marco Scardovi
> Signed-off-by: Masahiro Yamada
Recensito-Da: Nick Desaulniers
In realtà non parlo italiano, ma so come usare Google Translate.
> ---
>
> scripts/ld-version.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh
> index 30debf78aa09..1bf3aadde9de 100755
> --- a/scripts/ld-version.sh
> +++ b/scripts/ld-version.sh
> @@ -29,7 +29,7 @@ orig_args="$@"
> # Get the first line of the --version output.
> IFS='
> '
> -set -- $("$@" --version)
> +set -- $(LC_ALL=C "$@" --version)
>
> # Split the line on spaces.
> IFS=' '
> --
> 2.27.0
>
--
Thanks,
~Nick Desaulniers
[PATCH] gcov: fix clang-11+ support
LLVM changed the expected function signatures for llvm_gcda_start_file()
and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
or newer may have noticed their kernels failing to boot due to a panic
when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
the function signatures so calling these functions doesn't panic the
kernel.
When we drop clang-10 support from the kernel, we should carefully
update the original implementations to try to preserve git blame,
deleting these implementations.
Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
Cc: Fangrui Song
Reported-by: Prasad Sodagudi
Signed-off-by: Nick Desaulniers
---
kernel/gcov/clang.c | 69 +
1 file changed, 69 insertions(+)
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index c94b820a1b62..20e6760ec05d 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -75,7 +75,9 @@ struct gcov_fn_info {
u32 num_counters;
u64 *counters;
+#if __clang_major__ < 11
const char *function_name;
+#endif
};
static struct gcov_info *current_info;
@@ -105,6 +107,7 @@ void llvm_gcov_init(llvm_gcov_callback writeout,
llvm_gcov_callback flush)
}
EXPORT_SYMBOL(llvm_gcov_init);
+#if __clang_major__ < 11
void llvm_gcda_start_file(const char *orig_filename, const char version[4],
u32 checksum)
{
@@ -113,7 +116,17 @@ void llvm_gcda_start_file(const char *orig_filename, const
char version[4],
current_info->checksum = checksum;
}
EXPORT_SYMBOL(llvm_gcda_start_file);
+#else
+void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
+{
+ current_info->filename = orig_filename;
+ current_info->version = version;
+ current_info->checksum = checksum;
+}
+EXPORT_SYMBOL(llvm_gcda_start_file);
+#endif
+#if __clang_major__ < 11
void llvm_gcda_emit_function(u32 ident, const char *function_name,
u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
{
@@ -133,6 +146,24 @@ void llvm_gcda_emit_function(u32 ident, const char
*function_name,
list_add_tail(>head, _info->functions);
}
EXPORT_SYMBOL(llvm_gcda_emit_function);
+#else
+void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
+ u8 use_extra_checksum, u32 cfg_checksum)
+{
+ struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
+
+ if (!info)
+ return;
+
+ INIT_LIST_HEAD(>head);
+ info->ident = ident;
+ info->checksum = func_checksum;
+ info->use_extra_checksum = use_extra_checksum;
+ info->cfg_checksum = cfg_checksum;
+ list_add_tail(>head, _info->functions);
+}
+EXPORT_SYMBOL(llvm_gcda_emit_function);
+#endif
void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
{
@@ -295,6 +326,7 @@ void gcov_info_add(struct gcov_info *dst, struct gcov_info
*src)
}
}
+#if __clang_major__ < 11
static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
{
size_t cv_size; /* counter values size */
@@ -322,6 +354,28 @@ static struct gcov_fn_info *gcov_fn_info_dup(struct
gcov_fn_info *fn)
kfree(fn_dup);
return NULL;
}
+#else
+static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
+{
+ size_t cv_size; /* counter values size */
+ struct gcov_fn_info *fn_dup = kmemdup(fn, sizeof(*fn),
+ GFP_KERNEL);
+ if (!fn_dup)
+ return NULL;
+ INIT_LIST_HEAD(_dup->head);
+
+ cv_size = fn->num_counters * sizeof(fn->counters[0]);
+ fn_dup->counters = vmalloc(cv_size);
+ if (!fn_dup->counters) {
+ kfree(fn_dup);
+ return NULL;
+ }
+
+ memcpy(fn_dup->counters, fn->counters, cv_size);
+
+ return fn_dup;
+}
+#endif
/**
* gcov_info_dup - duplicate profiling data set
@@ -362,6 +416,7 @@ struct gcov_info *gcov_info_dup(struct gcov_info *info)
* gcov_info_free - release memory for profiling data set duplicate
* @info: profiling data set duplicate to free
*/
+#if __clang_major__ < 11
void gcov_info_free(struct gcov_info *info)
{
struct gcov_fn_info *fn, *tmp;
@@ -375,6 +430,20 @@ void gcov_info_free(struct gcov_info *info)
kfree(info->filename);
kfree(info);
}
+#else
+void gcov_info_free(struct gcov_info *info)
+{
+ struct gcov_fn_info *fn, *tmp;
+
+ list_for_each_entry_safe(fn, tmp, >functions, head) {
+ vfree(fn->counters);
+ list_del(>head);
+ kfree(fn);
+ }
+ kfree(info->filename);
+ kfree(info);
+}
+#endif
#define ITER_STRIDEPAGE_SIZE
base-commit: f78d76e72a4671ea52d12752d92077788b4f5d50
--
2.31.0.rc2.261.g7f71774620-goog
Re: [PATCH] Makefile: LTO: have linker check -Wframe-larger-than
On Thu, Mar 11, 2021 at 5:09 PM Nick Desaulniers wrote: > > -Wframe-larger-than= requires stack frame information, which the > frontend cannot provide. This diagnostic is emitted late during > compilation once stack frame size is available. > > When building with LTO, the frontend simply lowers C to LLVM IR and does > not have stack frame information, so it cannot emit this diagnostic. > When the linker drives LTO, it restarts optimizations and lowers LLVM IR > to object code. At that point, it has stack frame information but > doesn't know to check for a specific max stack frame size. > > I consider this a bug in LLVM that we need to fix. There are some > details we're working out related to LTO such as which value to use when > there are multiple different values specified per TU, or how to > propagate these to compiler synthesized routines properly, if at all. > > Until it's fixed, ensure we don't miss these. At that point we can wrap > this in a compiler version guard or revert this based on the minimum > support version of Clang. > > The error message is not generated during link: > LTO vmlinux.o > ld.lld: warning: stack size limit exceeded (8224) in foobarbaz > > Cc: Sami Tolvanen > Reported-by: Candle Sun > Suggested-by: Fangrui Song > Signed-off-by: Nick Desaulniers > --- > LTO users might want to `make clean` or `rm -rf .thinlto-cache` to test > this. > > Makefile | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/Makefile b/Makefile > index f9b54da2fca0..74566b1417b8 100644 > --- a/Makefile > +++ b/Makefile Candle sent me a private message that we probably also want coverage for kernel modules. Let me revise this and test/send a v2. > @@ -910,6 +910,11 @@ CC_FLAGS_LTO += -fvisibility=hidden > > # Limit inlining across translation units to reduce binary size > KBUILD_LDFLAGS += -mllvm -import-instr-limit=5 > + > +# Check for frame size exceeding threshold during prolog/epilog insertion. > +ifneq ($(CONFIG_FRAME_WARN),0) > +KBUILD_LDFLAGS += -plugin-opt=-warn-stack-size=$(CONFIG_FRAME_WARN) > +endif > endif > > ifdef CONFIG_LTO > -- > 2.31.0.rc2.261.g7f71774620-goog > -- Thanks, ~Nick Desaulniers
[PATCH] Makefile: LTO: have linker check -Wframe-larger-than
-Wframe-larger-than= requires stack frame information, which the frontend cannot provide. This diagnostic is emitted late during compilation once stack frame size is available. When building with LTO, the frontend simply lowers C to LLVM IR and does not have stack frame information, so it cannot emit this diagnostic. When the linker drives LTO, it restarts optimizations and lowers LLVM IR to object code. At that point, it has stack frame information but doesn't know to check for a specific max stack frame size. I consider this a bug in LLVM that we need to fix. There are some details we're working out related to LTO such as which value to use when there are multiple different values specified per TU, or how to propagate these to compiler synthesized routines properly, if at all. Until it's fixed, ensure we don't miss these. At that point we can wrap this in a compiler version guard or revert this based on the minimum support version of Clang. The error message is not generated during link: LTO vmlinux.o ld.lld: warning: stack size limit exceeded (8224) in foobarbaz Cc: Sami Tolvanen Reported-by: Candle Sun Suggested-by: Fangrui Song Signed-off-by: Nick Desaulniers --- LTO users might want to `make clean` or `rm -rf .thinlto-cache` to test this. Makefile | 5 + 1 file changed, 5 insertions(+) diff --git a/Makefile b/Makefile index f9b54da2fca0..74566b1417b8 100644 --- a/Makefile +++ b/Makefile @@ -910,6 +910,11 @@ CC_FLAGS_LTO += -fvisibility=hidden # Limit inlining across translation units to reduce binary size KBUILD_LDFLAGS += -mllvm -import-instr-limit=5 + +# Check for frame size exceeding threshold during prolog/epilog insertion. +ifneq ($(CONFIG_FRAME_WARN),0) +KBUILD_LDFLAGS += -plugin-opt=-warn-stack-size=$(CONFIG_FRAME_WARN) +endif endif ifdef CONFIG_LTO -- 2.31.0.rc2.261.g7f71774620-goog
Re: [PATCH] ARM: Make UNWINDER_ARM depend on ld.bfd or ld.lld 11.0.0+
On Wed, Mar 10, 2021 at 4:54 PM Nathan Chancellor wrote: > > When linking aspeed_g5_defconfig with ld.lld 10.0.1, the following error > occurs: > > ld.lld: error: .tmp_vmlinux.kallsyms1:(.ARM.exidx+0x34D98): relocation > R_ARM_PREL31 out of range: 2135538592 is not in [-1073741824, > 1073741823] > > This was resolved in ld.lld 11.0.0 but the minimum supported version of > ld.lld for the kernel is 10.0.1. Prevent CONFIG_UNWINDER_ARM from being > selected in this case so that the problematic sections cannot be > created. > > Link: https://github.com/ClangBuiltLinux/linux/issues/732 > Link: > https://github.com/llvm/llvm-project/commit/48aebfc908ba7b9372aaa478a9c200789491096e > Suggested-by: Nick Desaulniers > Signed-off-by: Nathan Chancellor Thanks for the patch. We discussed at the kernelCI meeting yesterday and the clangbuiltlinux meeting today continuing coverage for kernel builds with clang-10, so this is still worthwhile IMO at least for randconfig testing not to select known broken configs when using older tools. We can rip it out once we bump the minimum supported version of clang. Reviewed-by: Nick Desaulniers > --- > arch/arm/Kconfig.debug | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug > index 9e0b5e7f12af..64c1f8a46ab5 100644 > --- a/arch/arm/Kconfig.debug > +++ b/arch/arm/Kconfig.debug > @@ -66,6 +66,8 @@ config UNWINDER_FRAME_POINTER > config UNWINDER_ARM > bool "ARM EABI stack unwinder" > depends on AEABI && !FUNCTION_GRAPH_TRACER > + # https://github.com/ClangBuiltLinux/linux/issues/732 > + depends on !LD_IS_LLD || LLD_VERSION >= 11 > select ARM_UNWIND > help > This option enables stack unwinding support in the kernel > > base-commit: a38fd8748464831584a19438cbb3082b5a2dab15 > -- -- Thanks, ~Nick Desaulniers
Re: [PATCH] [RFC] arm64: enable HAVE_LD_DEAD_CODE_DATA_ELIMINATION
On Wed, Mar 10, 2021 at 1:08 PM Arnd Bergmann wrote:
>
> On Wed, Mar 10, 2021 at 9:50 PM Masahiro Yamada wrote:
> > On Mon, Mar 1, 2021 at 10:11 AM Nicholas Piggin wrote:
> > > Excerpts from Arnd Bergmann's message of February 27, 2021 7:49 pm:
>
> >
> > masahiro@oscar:~/ref/linux$ echo 'void this_func_is_unused(void) {}'
> > >> kernel/cpu.c
> > masahiro@oscar:~/ref/linux$ export
> > CROSS_COMPILE=/home/masahiro/tools/powerpc-10.1.0/bin/powerpc-linux-
> > masahiro@oscar:~/ref/linux$ make ARCH=powerpc defconfig
> > masahiro@oscar:~/ref/linux$ ./scripts/config -e EXPERT
> > masahiro@oscar:~/ref/linux$ ./scripts/config -e
> > LD_DEAD_CODE_DATA_ELIMINATION
> > masahiro@oscar:~/ref/linux$
> > ~/tools/powerpc-10.1.0/bin/powerpc-linux-nm -n vmlinux | grep
> > this_func
> > c0170560 T .this_func_is_unused
> > c1d8d560 D this_func_is_unused
> > masahiro@oscar:~/ref/linux$ grep DEAD_CODE_ .config
> > CONFIG_HAVE_LD_DEAD_CODE_DATA_ELIMINATION=y
> > CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y
> >
> >
> > If I remember correctly,
> > LD_DEAD_CODE_DATA_ELIMINATION dropped unused functions
> > when I tried it last time.
> >
> >
> > I also tried arm64 with a HAVE_LD_DEAD_CODE_DATA_ELIMINATION hack.
> > The result was the same.
> >
> >
> >
> > Am I missing something?
>
> It's possible that it only works in combination with CLANG_LTO now
> because something broke. I definitely saw a reduction in kernel
> size when both options are enabled, but did not try a simple test
> case like you did.
>
> Maybe some other reference gets created that prevents the function
> from being garbage-collected unless that other option is removed
> as well?
I wish the linker had a debug flag that could let developers discover
the decisions it made during --gc-sections as to why certain symbols
were retained/kept or not.
--
Thanks,
~Nick Desaulniers
Re: [RFC PATCH v2 00/13] objtool: add base support for arm64
On Fri, Mar 5, 2021 at 3:51 PM Nick Desaulniers wrote: > > (in response to > https://lore.kernel.org/linux-arm-kernel/20210303170932.1838634-1-jthie...@redhat.com/ > from the command line) > > > Changes since v1[2]: > > - Drop gcc plugin in favor of -fno-jump-tables > > Thank you for this! I built+booted(under emulation) arm64 defconfig and built > arm64 allmodconfig with LLVM=1 with this series applied. > > Tested-by: Nick Desaulniers > > One thing I noticed was a spew of warnings for allmodconfig, like: > init/main.o: warning: objtool: asan.module_ctor()+0xc: call without frame > pointer save/setup > init/main.o: warning: objtool: asan.module_dtor()+0xc: call without frame > pointer save/setup > > I assume those are from the KASAN constructors. See also: > https://github.com/ClangBuiltLinux/linux/issues/1238 > > Can we disable HAVE_STACK_PROTECTOR if CC_IS_CLANG and CONFIG_KASAN is set, > until we can resolve the above issue? Ah, filtering the logs more, it looks like GCOV is has the same issue KASAN does (known issue). Here's a filtered log: https://gist.github.com/nickdesaulniers/01358015b33bd16ccd7d951c4a8c44e7 I'm curious about the failure to decode certain instructions? The stack state mismatches are what are valuable to me; we'll need some help digging into those at some point. The logs from defconfig are clean. -- Thanks, ~Nick Desaulniers
Re: [RFC PATCH v2 00/13] objtool: add base support for arm64
(in response to https://lore.kernel.org/linux-arm-kernel/20210303170932.1838634-1-jthie...@redhat.com/ from the command line) > Changes since v1[2]: > - Drop gcc plugin in favor of -fno-jump-tables Thank you for this! I built+booted(under emulation) arm64 defconfig and built arm64 allmodconfig with LLVM=1 with this series applied. Tested-by: Nick Desaulniers One thing I noticed was a spew of warnings for allmodconfig, like: init/main.o: warning: objtool: asan.module_ctor()+0xc: call without frame pointer save/setup init/main.o: warning: objtool: asan.module_dtor()+0xc: call without frame pointer save/setup I assume those are from the KASAN constructors. See also: https://github.com/ClangBuiltLinux/linux/issues/1238 Can we disable HAVE_STACK_PROTECTOR if CC_IS_CLANG and CONFIG_KASAN is set, until we can resolve the above issue?
Re: [PATCH 3/4] kbuild: check the minimum assembler version in Kconfig
error out if it is not supported.
> ld-info := $(shell,$(srctree)/scripts/ld-version.sh $(LD))
> $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not
> supported.)
> diff --git a/scripts/as-version.sh b/scripts/as-version.sh
> new file mode 100755
> index ..205d8b9fc4d4
> --- /dev/null
> +++ b/scripts/as-version.sh
> @@ -0,0 +1,77 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Print the assembler name and its version in a 5 or 6-digit form.
> +# Also, perform the minimum version check.
> +# (If it is the integrated assembler, return 0 as the version, and
> +# the version check is skipped.)
> +
> +set -e
> +
> +# Convert the version string x.y.z to a canonical 5 or 6-digit form.
> +get_canonical_version()
> +{
> + IFS=.
> + set -- $1
> +
> + # If the 2nd or 3rd field is missing, fill it with a zero.
> + #
> + # The 4th field, if present, is ignored.
> + # This occurs in development snapshots as in 2.35.1.20201116
> + echo $((1 * $1 + 100 * ${2:-0} + ${3:-0}))
> +}
> +
> +orig_args="$@"
> +
> +# Get the first line of the --version output.
> +IFS='
> +'
> +# Add 2>&1 to check both stdout and stderr.
> +# If the backing assembler is binutils, we get the version string in stdout.
> +# If it is clang's integrated assembler, we get the following error in
> stderr:
> +# clang: error: unsupported argument '--version' to option 'Wa,'
> +# To avoid the error message affected by locale, set LC_MESSAGES=C just in
> case.
> +set -- $(LC_MESSAGES=C "$@" -Wno-unused-command-line-argument -Wa,--version
> -c -x assembler /dev/null -o /dev/null 2>&1)
> +line="$1"
> +
> +if [ "$line" = "clang: error: unsupported argument '--version' to option
> 'Wa,'" ]; then
Checking the precise error message is too brittle; what if it changes?
Why not check the return code a la cc-option and friends? Is checking
the return code of a subshell an issue here?
> + # For the intergrated assembler, we do not check the version here.
s/intergrated/integrated/
> + # It is the same as the clang version, and it has been already checked
> + # by scripts/cc-version.sh.
> + echo LLVM 0
> + exit 0
> +fi
> +
> +# Split the line on spaces.
> +IFS=' '
> +set -- $line
> +
> +tool_version=$(dirname $0)/tool-version.sh
> +
> +if [ "$1" = GNU -a "$2" = assembler ]; then
> + shift $(($# - 1))
> + version=$1
> + min_version=$($tool_version binutils)
> + name=GNU
> +else
> + echo "$orig_args: unknown assembler invoked" >&2
> + exit 1
> +fi
> +
> +# Some distributions append a package release number, as in 2.34-4.fc32
> +# Trim the hyphen and any characters that follow.
> +version=${version%-*}
> +
> +cversion=$(get_canonical_version $version)
> +min_cversion=$(get_canonical_version $min_version)
> +
> +if [ "$cversion" -lt "$min_cversion" ]; then
> + echo >&2 "***"
> + echo >&2 "*** Assembler is too old."
> + echo >&2 "*** Your $name assembler version:$version"
> + echo >&2 "*** Minimum $name assembler version: $min_version"
> + echo >&2 "***"
> + exit 1
> +fi
> +
> +echo $name $cversion
> --
> 2.27.0
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH v1] powerpc: Include running function as first entry in save_stack_trace() and friends
On Thu, Mar 4, 2021 at 9:42 AM Marco Elver wrote:
>
> On Thu, Mar 04, 2021 at 04:59PM +, Mark Rutland wrote:
> > On Thu, Mar 04, 2021 at 04:30:34PM +0100, Marco Elver wrote:
> > > On Thu, 4 Mar 2021 at 15:57, Mark Rutland wrote:
> > > > [adding Mark Brown]
> > > >
> > > > The bigger problem here is that skipping is dodgy to begin with, and
> > > > this is still liable to break in some cases. One big concern is that
> > > > (especially with LTO) we cannot guarantee the compiler will not inline
> > > > or outline functions, causing the skipp value to be too large or too
> > > > small. That's liable to happen to callers, and in theory (though
> > > > unlikely in practice), portions of arch_stack_walk() or
> > > > stack_trace_save() could get outlined too.
> > > >
> > > > Unless we can get some strong guarantees from compiler folk such that we
> > > > can guarantee a specific function acts boundary for unwinding (and
> > > > doesn't itself get split, etc), the only reliable way I can think to
> > > > solve this requires an assembly trampoline. Whatever we do is liable to
> > > > need some invasive rework.
> > >
> > > Will LTO and friends respect 'noinline'?
> >
> > I hope so (and suspect we'd have more problems otherwise), but I don't
> > know whether they actually so.
> >
> > I suspect even with 'noinline' the compiler is permitted to outline
> > portions of a function if it wanted to (and IIUC it could still make
> > specialized copies in the absence of 'noclone').
> >
> > > One thing I also noticed is that tail calls would also cause the stack
> > > trace to appear somewhat incomplete (for some of my tests I've
> > > disabled tail call optimizations).
> >
> > I assume you mean for a chain A->B->C where B tail-calls C, you get a
> > trace A->C? ... or is A going missing too?
>
> Correct, it's just the A->C outcome.
>
> > > Is there a way to also mark a function non-tail-callable?
> >
> > I think this can be bodged using __attribute__((optimize("$OPTIONS")))
> > on a caller to inhibit TCO (though IIRC GCC doesn't reliably support
> > function-local optimization options), but I don't expect there's any way
> > to mark a callee as not being tail-callable.
>
> I don't think this is reliable. It'd be
> __attribute__((optimize("-fno-optimize-sibling-calls"))), but doesn't
> work if applied to the function we do not want to tail-call-optimize,
> but would have to be applied to the function that does the tail-calling.
> So it's a bit backwards, even if it worked.
>
> > Accoding to the GCC documentation, GCC won't TCO noreturn functions, but
> > obviously that's not something we can use generally.
> >
> > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
include/linux/compiler.h:246:
prevent_tail_call_optimization
commit a9a3ed1eff36 ("x86: Fix early boot crash on gcc-10, third try")
>
> Perhaps we can ask the toolchain folks to help add such an attribute. Or
> maybe the feature already exists somewhere, but hidden.
>
> +Cc linux-toolcha...@vger.kernel.org
>
> > > But I'm also not sure if with all that we'd be guaranteed the code we
> > > want, even though in practice it might.
> >
> > True! I'd just like to be on the least dodgy ground we can be.
>
> It's been dodgy for a while, and I'd welcome any low-cost fixes to make
> it less dodgy in the short-term at least. :-)
>
> Thanks,
> -- Marco
--
Thanks,
~Nick Desaulniers
Re: [PATCH v3] MIPS: Make MIPS32_O32 depends on !CC_IS_CLANG
On Thu, Mar 4, 2021 at 12:04 AM Tiezhu Yang wrote: > > When building with Clang [1]: > > $ make CC=clang loongson3_defconfig > $ make CC=clang > > there exists the following error: > > Checking missing-syscalls for O32 > CALLscripts/checksyscalls.sh > error: ABI 'o32' is not supported on CPU 'mips64r2' > make[1]: *** [Kbuild:48: missing-syscalls] Error 1 > make: *** [arch/mips/Makefile:419: archprepare] Error 2 > > This is a known bug [2] with Clang, as Simon Atanasyan said, > "There is no plan on support O32 for MIPS64 due to lack of > resources". It's my hope we will fix the resourcing issue. I'm working on that; it's a non-technical challenge though. Acked-by: Nick Desaulniers > > It is not a good idea to remove CONFIG_MIPS32_O32=y directly > in defconfig because GCC works, as Nathan said, the config > should not even be selectable when building with Clang, so > just make MIPS32_O32 depends on !CC_IS_CLANG. > > [1] https://www.kernel.org/doc/html/latest/kbuild/llvm.html > [2] https://bugs.llvm.org/show_bug.cgi?id=38063 > > Signed-off-by: Tiezhu Yang > Acked-by: Nathan Chancellor > --- > > v3: Update the commit message suggested by Nathan, thank you! > > arch/mips/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index 3a38d27..f6ba59f 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -3318,6 +3318,8 @@ config SYSVIPC_COMPAT > config MIPS32_O32 > bool "Kernel support for o32 binaries" > depends on 64BIT > + # https://bugs.llvm.org/show_bug.cgi?id=38063 > + depends on !CC_IS_CLANG > select ARCH_WANT_OLD_COMPAT_IPC > select COMPAT > select MIPS32_COMPAT > -- > 2.1.0 > > -- > You received this message because you are subscribed to the Google Groups > "Clang Built Linux" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to clang-built-linux+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/clang-built-linux/1614845040-12995-1-git-send-email-yangtiezhu%40loongson.cn. -- Thanks, ~Nick Desaulniers
Re: [PATCH v2] sched: Optimize __calc_delta.
On Wed, Mar 3, 2021 at 2:48 PM Josh Don wrote:
>
> From: Clement Courbet
>
> A significant portion of __calc_delta time is spent in the loop
> shifting a u64 by 32 bits. Use `fls` instead of iterating.
>
> This is ~7x faster on benchmarks.
>
> The generic `fls` implementation (`generic_fls`) is still ~4x faster
> than the loop.
> Architectures that have a better implementation will make use of it. For
> example, on X86 we get an additional factor 2 in speed without dedicated
> implementation.
>
> On gcc, the asm versions of `fls` are about the same speed as the
> builtin. On clang, the versions that use fls are more than twice as
> slow as the builtin. This is because the way the `fls` function is
> written, clang puts the value in memory:
> https://godbolt.org/z/EfMbYe. This bug is filed at
> https://bugs.llvm.org/show_bug.cgi?id=49406.
Hi Josh, Thanks for helping get this patch across the finish line.
Would you mind updating the commit message to point to
https://bugs.llvm.org/show_bug.cgi?id=20197?
>
> ```
> name cpu/op
> BM_Calc<__calc_delta_loop> 9.57ms ±12%
> BM_Calc<__calc_delta_generic_fls> 2.36ms ±13%
> BM_Calc<__calc_delta_asm_fls> 2.45ms ±13%
> BM_Calc<__calc_delta_asm_fls_nomem>1.66ms ±12%
> BM_Calc<__calc_delta_asm_fls64>2.46ms ±13%
> BM_Calc<__calc_delta_asm_fls64_nomem> 1.34ms ±15%
> BM_Calc<__calc_delta_builtin> 1.32ms ±11%
> ```
>
> Signed-off-by: Clement Courbet
> Signed-off-by: Josh Don
> ---
> kernel/sched/fair.c | 19 +++
> kernel/sched/sched.h | 1 +
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 8a8bd7b13634..a691371960ae 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -229,22 +229,25 @@ static void __update_inv_weight(struct load_weight *lw)
> static u64 __calc_delta(u64 delta_exec, unsigned long weight, struct
> load_weight *lw)
> {
> u64 fact = scale_load_down(weight);
> + u32 fact_hi = (u32)(fact >> 32);
> int shift = WMULT_SHIFT;
> + int fs;
>
> __update_inv_weight(lw);
>
> - if (unlikely(fact >> 32)) {
> - while (fact >> 32) {
> - fact >>= 1;
> - shift--;
> - }
> + if (unlikely(fact_hi)) {
> + fs = fls(fact_hi);
> + shift -= fs;
> + fact >>= fs;
> }
>
> fact = mul_u32_u32(fact, lw->inv_weight);
>
> - while (fact >> 32) {
> - fact >>= 1;
> - shift--;
> + fact_hi = (u32)(fact >> 32);
> + if (fact_hi) {
> + fs = fls(fact_hi);
> + shift -= fs;
> + fact >>= fs;
> }
>
> return mul_u64_u32_shr(delta_exec, fact, shift);
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 10a1522b1e30..714af71cf983 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -36,6 +36,7 @@
> #include
>
> #include
> +#include
This hunk of the patch is curious. I assume that bitops.h is needed
for fls(); if so, why not #include it in kernel/sched/fair.c?
Otherwise this potentially hurts compile time for all TUs that include
kernel/sched/sched.h.
> #include
> #include
> #include
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH 1/4] kbuild: remove LLVM=1 test from HAS_LTO_CLANG
+ Sami
On Wed, Mar 3, 2021 at 10:34 AM Masahiro Yamada wrote:
>
> This guarding is wrong. As Documentation/kbuild/llvm.rst notes, LLVM=1
> switches the default of tools, but you can still override CC, LD, etc.
> individually.
>
> BTW, LLVM is not 1/0 flag. If LLVM is not passed in, it is empty.
Do we have the same problem with LLVM_IAS? LGTM otherwise, but wanted
to check that before signing off.
(Also, the rest of the patches in this series seem more related to
DWARFv5 cleanups; this patch seems orthogonal while those are a
visible progression).
>
> Non-zero return code is all treated as failure anyway.
>
> So, $(success,test $(LLVM) -eq 1) and $(success,test "$(LLVM)" = 1)
> works equivalently in the sense that both are expanded to 'n' if LLVM
> is not given. The difference is that the former internally fails due
> to syntax error.
>
> $ test ${LLVM} -eq 1
> bash: test: -eq: unary operator expected
> $ echo $?
> 2
>
> $ test "${LLVM}" -eq 1
> bash: test: : integer expression expected
> $ echo $?
> 2
>
> $ test "${LLVM}" = 1
> echo $?
> 1
>
> $ test -n "${LLVM}"
> $ echo $?
> 1
>
> Signed-off-by: Masahiro Yamada
> ---
>
> arch/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2bb30673d8e6..2af10ebe5ed0 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -632,7 +632,6 @@ config HAS_LTO_CLANG
> def_bool y
> # Clang >= 11: https://github.com/ClangBuiltLinux/linux/issues/510
> depends on CC_IS_CLANG && CLANG_VERSION >= 11 && LD_IS_LLD
> - depends on $(success,test $(LLVM) -eq 1)
IIRC, we needed some other LLVM utilities like llvm-nm and llvm-ar,
which are checked below. So I guess we can still support CC=clang
AR=llvm-ar NM=llvm-nm, and this check is redundant.
> depends on $(success,test $(LLVM_IAS) -eq 1)
> depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm)
> depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm)
> --
> 2.27.0
>
--
Thanks,
~Nick Desaulniers
