Re: [PATCH v2] trace: tracing_event_filter: fast path when no subsystem filters
On Wed, Oct 4, 2023 at 10:52 AM Steven Rostedt wrote: > > On Wed, 4 Oct 2023 10:39:33 -0400 > Nick Lowell wrote: > > > > [ cut here ] > > > WARNING: CPU: 5 PID: 944 at kernel/trace/trace_events_filter.c:2423 > > > apply_subsystem_event_filter+0x18c/0x5e0 > > > Modules linked in: > > > CPU: 5 PID: 944 Comm: trace-cmd Not tainted > > > 6.6.0-rc4-test-9-gff7cd7446fe5 #102 > > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > > > 1.16.2-debian-1.16.2-1 04/01/2014 > > > RIP: 0010:apply_subsystem_event_filter+0x18c/0x5e0 > > > Code: 44 24 08 00 00 00 00 48 8b 6d 00 4c 39 f5 75 bc 48 8b 44 24 18 4c > > > 8b 60 18 4c 89 e5 45 84 ff 75 14 48 85 ed 0f 84 37 ff ff ff <0f> 0b eb 10 > > > e8 4b be fd ff eb b0 4d 85 e4 0f 84 a3 02 00 00 48 8b > > > RSP: 0018:9b4941607db8 EFLAGS: 00010286 > > > RAX: 8b2780a77280 RBX: 8b2780a77400 RCX: > > > RDX: RSI: 8b2781c11c38 RDI: 8b2781c11c38 > > > RBP: 8b28df449030 R08: 8b2781c11c38 R09: > > > R10: 8b2781c11c38 R11: R12: 8b28df449030 > > > R13: aaf64de0 R14: aaf66bb8 R15: > > > FS: 7fd221def3c0() GS:8b28f7d4() > > > knlGS: > > > CS: 0010 DS: ES: CR0: 80050033 > > > CR2: 56117c93e160 CR3: 00010173a003 CR4: 00170ee0 > > > Call Trace: > > > > > > ? apply_subsystem_event_filter+0x18c/0x5e0 > > > ? __warn+0x81/0x130 > > > ? apply_subsystem_event_filter+0x18c/0x5e0 > > > ? report_bug+0x191/0x1c0 > > > ? handle_bug+0x3c/0x80 > > > ? exc_invalid_op+0x17/0x70 > > > ? asm_exc_invalid_op+0x1a/0x20 > > > ? apply_subsystem_event_filter+0x18c/0x5e0 > > > ? apply_subsystem_event_filter+0x5b/0x5e0 > > > ? __check_object_size+0x25b/0x2c0 > > > subsystem_filter_write+0x41/0x70 > > > vfs_write+0xf2/0x440 > > > ? kmem_cache_free+0x22/0x350 > > > ksys_write+0x6f/0xf0 > > > do_syscall_64+0x3f/0xc0 > > > entry_SYSCALL_64_after_hwframe+0x6e/0xd8 > > > RIP: 0033:0x7fd221ee7ae0 > > > > > > -- Steve > > > > Is this just informative indicating that there are issues with how > > filters are being used or are you saying there is something else I > > need to do before this patch is approved? > > What version of trace-cmd is that using? > > Not sure if it matters, but the above was with trace-cmd v3.2. > > So, I guess we need to look a bit deeper at the change. > > > @@ -2411,7 +2418,12 @@ int apply_subsystem_event_filter(struct > > trace_subsystem_dir *dir, > > } > > > > if (!strcmp(strstrip(filter_string), "0")) { > > - filter_free_subsystem_preds(dir, tr); > > + /* If nothing was freed, we do not need to sync */ > > + if (!filter_free_subsystem_preds(dir, tr)) { > > + if(!(WARN_ON_ONCE(system->filter))) > > + goto out_unlock; > > When do we want to skip the below? > > The original version just did the "goto out_unlock" before the > "system->filter" check, and that would have caused a memory leak, or just > left the "system->filter" around when unneeded. > > In other words, the patch is incorrect in general then. > > > + } > > + > > remove_filter_string(system->filter); > > filter = system->filter; > > system->filter = NULL; > > I believe, what you really want here is simply: > > bool sync; > > [..] > > if (!strcmp(strstrip(filter_string), "0")) { > + sync = filter_free_subsystem_preds(dir, tr); > remove_filter_string(system->filter); > filter = system->filter; > system->filter = NULL; > - /* Ensure all filters are no longer used */ > - tracepoint_synchronize_unregister(); > + if (sync) { > + /* Ensure all filters are no longer used */ > + tracepoint_synchronize_unregister(); > + } > filter_free_subsystem_filters(dir, tr); > I really appreciate the continued feedback. I was able to reproduce. I think I'm understanding better but still need some help. I am actually
Re: [PATCH v2] trace: tracing_event_filter: fast path when no subsystem filters
On Tue, Oct 3, 2023 at 10:29 PM Steven Rostedt wrote: > > On Mon, 2 Oct 2023 10:41:48 -0400 > Nicholas Lowell wrote: > > > @@ -2411,7 +2418,12 @@ int apply_subsystem_event_filter(struct > > trace_subsystem_dir *dir, > > } > > > > if (!strcmp(strstrip(filter_string), "0")) { > > - filter_free_subsystem_preds(dir, tr); > > + /* If nothing was freed, we do not need to sync */ > > + if (!filter_free_subsystem_preds(dir, tr)) { > > + if(!(WARN_ON_ONCE(system->filter))) > > + goto out_unlock; > > + } > > + > > remove_filter_string(system->filter); > > filter = system->filter; > > system->filter = NULL; > > -- > > This is why I asked for the warning: > > trace-cmd record -o /tmp/trace.dat -e sched -f "(common_pid == $$) || > ((common_pid > 10) && common_pid < 100) || (common_pid >= 1000 && common_pid > <= 1050) || (common_pid > 1 && common_pid < 2)" sleep 5 > > > Causes: > > [ cut here ] > WARNING: CPU: 5 PID: 944 at kernel/trace/trace_events_filter.c:2423 > apply_subsystem_event_filter+0x18c/0x5e0 > Modules linked in: > CPU: 5 PID: 944 Comm: trace-cmd Not tainted > 6.6.0-rc4-test-9-gff7cd7446fe5 #102 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > 1.16.2-debian-1.16.2-1 04/01/2014 > RIP: 0010:apply_subsystem_event_filter+0x18c/0x5e0 > Code: 44 24 08 00 00 00 00 48 8b 6d 00 4c 39 f5 75 bc 48 8b 44 24 18 4c 8b > 60 18 4c 89 e5 45 84 ff 75 14 48 85 ed 0f 84 37 ff ff ff <0f> 0b eb 10 e8 4b > be fd ff eb b0 4d 85 e4 0f 84 a3 02 00 00 48 8b > RSP: 0018:9b4941607db8 EFLAGS: 00010286 > RAX: 8b2780a77280 RBX: 8b2780a77400 RCX: > RDX: RSI: 8b2781c11c38 RDI: 8b2781c11c38 > RBP: 8b28df449030 R08: 8b2781c11c38 R09: > R10: 8b2781c11c38 R11: R12: 8b28df449030 > R13: aaf64de0 R14: aaf66bb8 R15: > FS: 7fd221def3c0() GS:8b28f7d4() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 56117c93e160 CR3: 00010173a003 CR4: 00170ee0 > Call Trace: > > ? apply_subsystem_event_filter+0x18c/0x5e0 > ? __warn+0x81/0x130 > ? apply_subsystem_event_filter+0x18c/0x5e0 > ? report_bug+0x191/0x1c0 > ? handle_bug+0x3c/0x80 > ? exc_invalid_op+0x17/0x70 > ? asm_exc_invalid_op+0x1a/0x20 > ? apply_subsystem_event_filter+0x18c/0x5e0 > ? apply_subsystem_event_filter+0x5b/0x5e0 > ? __check_object_size+0x25b/0x2c0 > subsystem_filter_write+0x41/0x70 > vfs_write+0xf2/0x440 > ? kmem_cache_free+0x22/0x350 > ksys_write+0x6f/0xf0 > do_syscall_64+0x3f/0xc0 > entry_SYSCALL_64_after_hwframe+0x6e/0xd8 > RIP: 0033:0x7fd221ee7ae0 > > -- Steve Is this just informative indicating that there are issues with how filters are being used or are you saying there is something else I need to do before this patch is approved? What version of trace-cmd is that using?
Re: [PATCH] trace: tracing_event_filter: fast path when no subsystem filters
Sending again in plain text mode. Thanks for the great feedback! Hopefully my inline comments/questions aren't garbled. On Sat, Sep 30, 2023 at 4:04 AM Steven Rostedt wrote: > > On Tue, 26 Sep 2023 10:20:58 -0400 > Nicholas Lowell wrote: > > > From: Nicholas Lowell > > > > If there are no filters in the event subsystem, then there's no > > reason to continue and hit the potentially time consuming > > tracepoint_synchronize_unregister function. This should give > > a speed up for initial disabling/configuring > > > > Signed-off-by: Nicholas Lowell > > --- > > kernel/trace/trace_events_filter.c | 19 ++- > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/kernel/trace/trace_events_filter.c > > b/kernel/trace/trace_events_filter.c > > index 33264e510d16..93653d37a132 100644 > > --- a/kernel/trace/trace_events_filter.c > > +++ b/kernel/trace/trace_events_filter.c > > @@ -1317,22 +1317,29 @@ void free_event_filter(struct event_filter *filter) > > __free_filter(filter); > > } > > > > -static inline void __remove_filter(struct trace_event_file *file) > > +static inline int __remove_filter(struct trace_event_file *file) > > { > > filter_disable(file); > > - remove_filter_string(file->filter); > > + if (file->filter) > > + remove_filter_string(file->filter); > > + else > > + return 0; > > + > > + return 1; > > The above looks awkward. What about: > > if (!file->filter) > return 0; > > remove_filter_string(file->filter); > return 1; > > ? > > Or better yet: > > if (!file->filter) > return false; > > remove_filter_string(file->filter); > return true; > Is it safe to assume you would like the function's return type to change from int to bool if I go with option 2? > and ... > > > } > > > > -static void filter_free_subsystem_preds(struct trace_subsystem_dir *dir, > > +static int filter_free_subsystem_preds(struct trace_subsystem_dir *dir, > > struct trace_array *tr) > > { > > struct trace_event_file *file; > > + int i = 0; > > We don't really need a counter. It's either do the synchronization or > we don't. > > bool do_sync = false; > > > > > list_for_each_entry(file, &tr->events, list) { > > if (file->system != dir) > > continue; > > - __remove_filter(file); > > + i += __remove_filter(file); > > if (remove_filter(file)) > do_sync = true; > > > } > > return do_sync; > Going to assume the same here--that return type should change from int to bool. > > + return i; > > } > > > > static inline void __free_subsystem_filter(struct trace_event_file *file) > > @@ -2411,7 +2418,9 @@ int apply_subsystem_event_filter(struct > > trace_subsystem_dir *dir, > > } > > > > if (!strcmp(strstrip(filter_string), "0")) { > > - filter_free_subsystem_preds(dir, tr); > > + if (filter_free_subsystem_preds(dir, tr) == 0) > > + goto out_unlock; > > + > > /* If nothing was freed, we do not need to sync */ > if (!filter_free_subsystem_preds(dir, tr)) > goto out_unlock; > > And yes, add the comment. > > And actually, in that block with the goto out_unlock, we should have: > > if (!filter_free_subsystem_preds(dir, tr)) { > if (!(WARN_ON_ONCE(system->filter)) > goto out_unlock; > } > Can you explain why the WARN_ON_ONCE should be in a conditional? Don't we still want the original conditional to cause the goto regardless? if (!filter_free_subsystem_preds(dir, tr)) { WARN_ON_ONCE(system->filter); goto out_unlock; } > If there were no preds, ideally there would be no subsystem filter. But > if that's not the case, we need to warn about that and then continue. > > -- Steve > > > remove_filter_string(system->filter); > > filter = system->filter; > > system->filter = NULL; >
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?)
[PATCH v4 4/5] RISC-V: Add kdump support
This patch adds support for kdump, the kernel will reserve a region for the crash kernel and jump there on panic. In order for userspace tools (kexec-tools) to prepare the crash kernel kexec image, we also need to expose some information on /proc/iomem for the memory regions used by the kernel and for the region reserved for crash kernel. Note that on userspace the device tree is used to determine the system's memory layout so the "System RAM" on /proc/iomem is ignored. I tested this on riscv64 qemu and works as expected, you may test it by triggering a crash through /proc/sysrq_trigger: echo c > /proc/sysrq_trigger v4: * Re-base on top of "fixes" branch * Use CSR_* macros and PMD_SIZE where possible v3: * Move ELF_CORE_COPY_REGS to asm/elf.h instead of uapi/asm/elf.h * Set stvec when disabling MMU * Minor cleanups and re-base v2: * Properly populate the ioresources tree, so that it can be used later on for implementing strict /dev/mem. * Minor cleanups and re-base Signed-off-by: Nick Kossifidis --- arch/riscv/include/asm/elf.h| 6 +++ arch/riscv/include/asm/kexec.h | 19 +--- arch/riscv/kernel/Makefile | 2 +- arch/riscv/kernel/crash_save_regs.S | 56 +++ arch/riscv/kernel/kexec_relocate.S | 68 ++- arch/riscv/kernel/machine_kexec.c | 43 + arch/riscv/kernel/setup.c | 11 - arch/riscv/mm/init.c| 71 + 8 files changed, 249 insertions(+), 27 deletions(-) create mode 100644 arch/riscv/kernel/crash_save_regs.S diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h index 5c725e1df..f4b490cd0 100644 --- a/arch/riscv/include/asm/elf.h +++ b/arch/riscv/include/asm/elf.h @@ -81,4 +81,10 @@ extern int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp); #endif /* CONFIG_MMU */ +#define ELF_CORE_COPY_REGS(dest, regs) \ +do { \ + *(struct user_regs_struct *)&(dest) = \ + *(struct user_regs_struct *)regs; \ +} while (0); + #endif /* _ASM_RISCV_ELF_H */ diff --git a/arch/riscv/include/asm/kexec.h b/arch/riscv/include/asm/kexec.h index 86e6e4922..1e9541019 100644 --- a/arch/riscv/include/asm/kexec.h +++ b/arch/riscv/include/asm/kexec.h @@ -23,11 +23,16 @@ #define KEXEC_ARCH KEXEC_ARCH_RISCV +extern void riscv_crash_save_regs(struct pt_regs *newregs); + static inline void crash_setup_regs(struct pt_regs *newregs, struct pt_regs *oldregs) { - /* Dummy implementation for now */ + if (oldregs) + memcpy(newregs, oldregs, sizeof(struct pt_regs)); + else + riscv_crash_save_regs(newregs); } @@ -40,10 +45,12 @@ struct kimage_arch { const extern unsigned char riscv_kexec_relocate[]; const extern unsigned int riscv_kexec_relocate_size; -typedef void (*riscv_kexec_do_relocate)(unsigned long first_ind_entry, - unsigned long jump_addr, - unsigned long fdt_addr, - unsigned long hartid, - unsigned long va_pa_off); +typedef void (*riscv_kexec_method)(unsigned long first_ind_entry, + unsigned long jump_addr, + unsigned long fdt_addr, + unsigned long hartid, + unsigned long va_pa_off); + +extern riscv_kexec_method riscv_kexec_norelocate; #endif diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index 211ef87de..41a1469b2 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -59,7 +59,7 @@ obj-$(CONFIG_SMP) += cpu_ops_sbi.o endif obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o obj-$(CONFIG_KGDB) += kgdb.o -obj-$(CONFIG_KEXEC)+= kexec_relocate.o machine_kexec.o +obj-$(CONFIG_KEXEC)+= kexec_relocate.o crash_save_regs.o machine_kexec.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o diff --git a/arch/riscv/kernel/crash_save_regs.S b/arch/riscv/kernel/crash_save_regs.S new file mode 100644 index 0..7832fb763 --- /dev/null +++ b/arch/riscv/kernel/crash_save_regs.S @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2020 FORTH-ICS/CARV + * Nick Kossifidis + */ + +#include/* For RISCV_* and REG_* macros */ +#include/* For CSR_* macros */ +#include/* For offsets on pt_regs */ +#include /* For SYM_* macros */ + +.section ".text" +SYM_CODE_START(riscv_crash_save_regs) + REG_S ra, PT_RA(a0)/* x1 */ + REG_S sp, PT_SP(a0)/* x2 */ + REG_S gp, PT_GP(a0)/* x3 */ + REG_S tp, PT_TP(a0)/* x4 */ + REG_S t0, PT_T0(a0)/* x
[PATCH v4 3/5] RISC-V: Improve init_resources
* Kernel region is always present and we know where it is, no need to look for it inside the loop, just ignore it like the rest of the reserved regions within system's memory. * Don't call memblock_free inside the loop, if called it'll split the region of pre-allocated resources in two parts, messing things up, just re-use the previous pre-allocated resource and free any unused resources after both loops finish. Signed-off-by: Nick Kossifidis --- arch/riscv/kernel/setup.c | 91 --- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index f8f15332c..030554bab 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -60,6 +60,7 @@ static DEFINE_PER_CPU(struct cpu, cpu_devices); * also add "System RAM" regions for compatibility with other * archs, and the rest of the known regions for completeness. */ +static struct resource kimage_res = { .name = "Kernel image", }; static struct resource code_res = { .name = "Kernel code", }; static struct resource data_res = { .name = "Kernel data", }; static struct resource rodata_res = { .name = "Kernel rodata", }; @@ -80,45 +81,54 @@ static int __init add_resource(struct resource *parent, return 1; } -static int __init add_kernel_resources(struct resource *res) +static int __init add_kernel_resources(void) { int ret = 0; /* * The memory region of the kernel image is continuous and -* was reserved on setup_bootmem, find it here and register -* it as a resource, then register the various segments of -* the image as child nodes +* was reserved on setup_bootmem, register it here as a +* resource, with the various segments of the image as +* child nodes. */ - if (!(res->start <= code_res.start && res->end >= data_res.end)) - return 0; - res->name = "Kernel image"; - res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + code_res.start = __pa_symbol(_text); + code_res.end = __pa_symbol(_etext) - 1; + code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - /* -* We removed a part of this region on setup_bootmem so -* we need to expand the resource for the bss to fit in. -*/ - res->end = bss_res.end; + rodata_res.start = __pa_symbol(__start_rodata); + rodata_res.end = __pa_symbol(__end_rodata) - 1; + rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + data_res.start = __pa_symbol(_data); + data_res.end = __pa_symbol(_edata) - 1; + data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + bss_res.start = __pa_symbol(__bss_start); + bss_res.end = __pa_symbol(__bss_stop) - 1; + bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + kimage_res.start = code_res.start; + kimage_res.end = bss_res.end; + kimage_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - ret = add_resource(&iomem_resource, res); + ret = add_resource(&iomem_resource, &kimage_res); if (ret < 0) return ret; - ret = add_resource(res, &code_res); + ret = add_resource(&kimage_res, &code_res); if (ret < 0) return ret; - ret = add_resource(res, &rodata_res); + ret = add_resource(&kimage_res, &rodata_res); if (ret < 0) return ret; - ret = add_resource(res, &data_res); + ret = add_resource(&kimage_res, &data_res); if (ret < 0) return ret; - ret = add_resource(res, &bss_res); + ret = add_resource(&kimage_res, &bss_res); return ret; } @@ -129,54 +139,42 @@ static void __init init_resources(void) struct resource *res = NULL; struct resource *mem_res = NULL; size_t mem_res_sz = 0; - int ret = 0, i = 0; - - code_res.start = __pa_symbol(_text); - code_res.end = __pa_symbol(_etext) - 1; - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - rodata_res.start = __pa_symbol(__start_rodata); - rodata_res.end = __pa_symbol(__end_rodata) - 1; - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - data_res.start = __pa_symbol(_data); - data_res.end = __pa_symbol(_edata) - 1; - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - bss_res.start = __pa_symbol(__bss_start); - bss_res.end = __pa_symbol(__bss_stop) - 1; - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + int num_resources = 0, res_idx = 0; + int ret = 0; /* + 1 as memblock_alloc() might increase memblock.reserved.cnt */ - mem_res_sz = (me
[PATCH v4 2/5] RISC-V: Add kexec support
This patch adds support for kexec on RISC-V. On SMP systems it depends on HOTPLUG_CPU in order to be able to bring up all harts after kexec. It also needs a recent OpenSBI version that supports the HSM extension. I tested it on riscv64 QEMU on both an smp and a non-smp system. v6: * Re-based on top of "fixes" branch * Use PAGE_SIZE and CSR_* macros where possible * Small cleanups v5: * For now depend on MMU, further changes needed for NOMMU support * Make sure stvec is aligned * Cleanup some unneeded fences * Verify control code's buffer size * Compile kexec_relocate.S with medany and norelax v4: * No functional changes, just re-based v3: * Use the new smp_shutdown_nonboot_cpus() call. * Move riscv_kexec_relocate to .rodata v2: * Pass needed parameters as arguments to riscv_kexec_relocate instead of using global variables. * Use kimage_arch to hold the fdt address of the included fdt. * Use SYM_* macros on kexec_relocate.S. * Compatibility with STRICT_KERNEL_RWX. * Compatibility with HOTPLUG_CPU for SMP * Small cleanups Signed-off-by: Nick Kossifidis --- arch/riscv/Kconfig | 15 +++ arch/riscv/include/asm/kexec.h | 49 arch/riscv/kernel/Makefile | 5 + arch/riscv/kernel/kexec_relocate.S | 157 arch/riscv/kernel/machine_kexec.c | 186 + 5 files changed, 412 insertions(+) create mode 100644 arch/riscv/include/asm/kexec.h create mode 100644 arch/riscv/kernel/kexec_relocate.S create mode 100644 arch/riscv/kernel/machine_kexec.c diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 4515a10c5..10cc19be0 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -386,6 +386,21 @@ config RISCV_SBI_V01 help This config allows kernel to use SBI v0.1 APIs. This will be deprecated in future once legacy M-mode software are no longer in use. + +config KEXEC + bool "Kexec system call" + select KEXEC_CORE + select HOTPLUG_CPU if SMP + depends on MMU + help + kexec is a system call that implements the ability to shutdown your + current kernel, and to start another kernel. It is like a reboot + but it is independent of the system firmware. And like a reboot + you can start any kernel with it, not just Linux. + + The name comes from the similarity to the exec system call. + + endmenu menu "Boot options" diff --git a/arch/riscv/include/asm/kexec.h b/arch/riscv/include/asm/kexec.h new file mode 100644 index 0..86e6e4922 --- /dev/null +++ b/arch/riscv/include/asm/kexec.h @@ -0,0 +1,49 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2019 FORTH-ICS/CARV + * Nick Kossifidis + */ + +#ifndef _RISCV_KEXEC_H +#define _RISCV_KEXEC_H + +#include /* For PAGE_SIZE */ + +/* Maximum physical address we can use pages from */ +#define KEXEC_SOURCE_MEMORY_LIMIT (-1UL) + +/* Maximum address we can reach in physical address mode */ +#define KEXEC_DESTINATION_MEMORY_LIMIT (-1UL) + +/* Maximum address we can use for the control code buffer */ +#define KEXEC_CONTROL_MEMORY_LIMIT (-1UL) + +/* Reserve a page for the control code buffer */ +#define KEXEC_CONTROL_PAGE_SIZE PAGE_SIZE + +#define KEXEC_ARCH KEXEC_ARCH_RISCV + +static inline void +crash_setup_regs(struct pt_regs *newregs, +struct pt_regs *oldregs) +{ + /* Dummy implementation for now */ +} + + +#define ARCH_HAS_KIMAGE_ARCH + +struct kimage_arch { + unsigned long fdt_addr; +}; + +const extern unsigned char riscv_kexec_relocate[]; +const extern unsigned int riscv_kexec_relocate_size; + +typedef void (*riscv_kexec_do_relocate)(unsigned long first_ind_entry, + unsigned long jump_addr, + unsigned long fdt_addr, + unsigned long hartid, + unsigned long va_pa_off); + +#endif diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index 647a47f54..211ef87de 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -10,6 +10,10 @@ CFLAGS_REMOVE_sbi.o = $(CC_FLAGS_FTRACE) endif CFLAGS_syscall_table.o += $(call cc-option,-Wno-override-init,) +ifdef CONFIG_KEXEC +AFLAGS_kexec_relocate.o := -mcmodel=medany -mno-relax +endif + extra-y += head.o extra-y += vmlinux.lds @@ -55,6 +59,7 @@ obj-$(CONFIG_SMP) += cpu_ops_sbi.o endif obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o obj-$(CONFIG_KGDB) += kgdb.o +obj-$(CONFIG_KEXEC)+= kexec_relocate.o machine_kexec.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o diff --git a/arch/riscv/kernel/kexec_relocate.S b/arch/riscv/kernel/kexec_relocate.S new file mode 100644 index 0..84101d0a2 --- /dev/null +++ b/arch/riscv/kernel/kexec_relocate.S @@ -0,0 +1,157 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyrigh
[PATCH v4 1/5] RISC-V: Add EM_RISCV to kexec UAPI header
Add RISC-V to the list of supported kexec architectures, we need to add the definition early-on so that later patches can use it. EM_RISCV is 243 as per ELF psABI specification here: https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md Signed-off-by: Nick Kossifidis --- include/uapi/linux/kexec.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h index 05669c87a..778dc191c 100644 --- a/include/uapi/linux/kexec.h +++ b/include/uapi/linux/kexec.h @@ -42,6 +42,7 @@ #define KEXEC_ARCH_MIPS_LE (10 << 16) #define KEXEC_ARCH_MIPS( 8 << 16) #define KEXEC_ARCH_AARCH64 (183 << 16) +#define KEXEC_ARCH_RISCV (243 << 16) /* The artificial cap on the number of segments passed to kexec_load. */ #define KEXEC_SEGMENT_MAX 16 -- 2.26.2
[PATCH v4 0/5] RISC-V: Add kexec/kdump support
This patch series adds kexec/kdump and crash kernel support on RISC-V. For testing the patches a patched version of kexec-tools is needed (still a work in progress) which can be found at: https://riscv.ics.forth.gr/kexec-tools-patched.tar.xz v4: * Rebase on top of "fixes" branch * Resolve Alex's comments v3: * Rebase on newer kernel tree * Minor cleanups * Split UAPI changes to a separate patch * Improve / cleanup init_resources * Resolve Palmer's comments v2: * Rebase on newer kernel tree * Minor cleanups * Properly populate the ioresources tre, so that it can be used later on for implementing strict /dev/mem * Use linux,usable-memory on /memory instead of a new binding * USe a reserved-memory node for ELF core header Nick Kossifidis (5): RISC-V: Add EM_RISCV to kexec UAPI header RISC-V: Add kexec support RISC-V: Improve init_resources RISC-V: Add kdump support RISC-V: Add crash kernel support arch/riscv/Kconfig | 25 arch/riscv/include/asm/elf.h| 6 + arch/riscv/include/asm/kexec.h | 56 +++ arch/riscv/kernel/Makefile | 6 + arch/riscv/kernel/crash_dump.c | 46 ++ arch/riscv/kernel/crash_save_regs.S | 56 +++ arch/riscv/kernel/kexec_relocate.S | 223 arch/riscv/kernel/machine_kexec.c | 193 arch/riscv/kernel/setup.c | 114 -- arch/riscv/mm/init.c| 104 + include/uapi/linux/kexec.h | 1 + 11 files changed, 784 insertions(+), 46 deletions(-) create mode 100644 arch/riscv/include/asm/kexec.h create mode 100644 arch/riscv/kernel/crash_dump.c create mode 100644 arch/riscv/kernel/crash_save_regs.S create mode 100644 arch/riscv/kernel/kexec_relocate.S create mode 100644 arch/riscv/kernel/machine_kexec.c -- 2.26.2
[PATCH v4 5/5] RISC-V: Add crash kernel support
This patch allows Linux to act as a crash kernel for use with kdump. Userspace will let the crash kernel know about the memory region it can use through linux,usable-memory property on the /memory node (overriding its reg property), and about the memory region where the elf core header of the previous kernel is saved, through a reserved-memory node with a compatible string of "linux,elfcorehdr". This approach is the least invasive and re-uses functionality already present. I tested this on riscv64 qemu and it works as expected, you may test it by retrieving the dmesg of the previous kernel through /proc/vmcore, using the vmcore-dmesg utility from kexec-tools. v4: * Rebase on top of "fixes" branch v3: * Rebase v2: * Use linux,usable-memory on /memory instead of a new binding * Use a reserved-memory node for ELF core header Signed-off-by: Nick Kossifidis --- arch/riscv/Kconfig | 10 arch/riscv/kernel/Makefile | 1 + arch/riscv/kernel/crash_dump.c | 46 ++ arch/riscv/kernel/setup.c | 12 + arch/riscv/mm/init.c | 33 5 files changed, 102 insertions(+) create mode 100644 arch/riscv/kernel/crash_dump.c diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 10cc19be0..39aa18ef4 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -400,6 +400,16 @@ config KEXEC The name comes from the similarity to the exec system call. +config CRASH_DUMP + bool "Build kdump crash kernel" + help + Generate crash dump after being started by kexec. This should + be normally only set in special crash dump kernels which are + loaded in the main kernel with kexec-tools into a specially + reserved region and then later executed after a crash by + kdump/kexec. + + For more details see Documentation/admin-guide/kdump/kdump.rst endmenu diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index 41a1469b2..d3081e4d9 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -60,6 +60,7 @@ endif obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o obj-$(CONFIG_KGDB) += kgdb.o obj-$(CONFIG_KEXEC)+= kexec_relocate.o crash_save_regs.o machine_kexec.o +obj-$(CONFIG_CRASH_DUMP) += crash_dump.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o diff --git a/arch/riscv/kernel/crash_dump.c b/arch/riscv/kernel/crash_dump.c new file mode 100644 index 0..86cc0ada5 --- /dev/null +++ b/arch/riscv/kernel/crash_dump.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * This code comes from arch/arm64/kernel/crash_dump.c + * Created by: AKASHI Takahiro + * Copyright (C) 2017 Linaro Limited + */ + +#include +#include + +/** + * copy_oldmem_page() - copy one page from old kernel memory + * @pfn: page frame number to be copied + * @buf: buffer where the copied page is placed + * @csize: number of bytes to copy + * @offset: offset in bytes into the page + * @userbuf: if set, @buf is in a user address space + * + * This function copies one page from old kernel memory into buffer pointed by + * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes + * copied or negative error in case of failure. + */ +ssize_t copy_oldmem_page(unsigned long pfn, char *buf, +size_t csize, unsigned long offset, +int userbuf) +{ + void *vaddr; + + if (!csize) + return 0; + + vaddr = memremap(__pfn_to_phys(pfn), PAGE_SIZE, MEMREMAP_WB); + if (!vaddr) + return -ENOMEM; + + if (userbuf) { + if (copy_to_user((char __user *)buf, vaddr + offset, csize)) { + memunmap(vaddr); + return -EFAULT; + } + } else + memcpy(buf, vaddr + offset, csize); + + memunmap(vaddr); + return csize; +} diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 31866dce9..ff398a3d8 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -66,6 +66,9 @@ static struct resource code_res = { .name = "Kernel code", }; static struct resource data_res = { .name = "Kernel data", }; static struct resource rodata_res = { .name = "Kernel rodata", }; static struct resource bss_res = { .name = "Kernel bss", }; +#ifdef CONFIG_CRASH_DUMP +static struct resource elfcorehdr_res = { .name = "ELF Core hdr", }; +#endif static int __init add_resource(struct resource *parent, struct resource *res) @@ -169,6 +172,15 @@ static void __init init_resources(void) } #endif +#ifdef CONFIG_CRASH_DUMP + if (elfcorehdr_size > 0) { + elfcorehdr_res.start = elfcorehdr_addr; + elfcorehdr_res.end = elfcorehdr_addr + elfcorehdr_size
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
pr_err(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
PY_QUIET) $(quiet_modtag) $@ > + cmd_rustc_o_rs = \ > + RUST_MODFILE=$(modfile) \ > + $(RUSTC_OR_CLIPPY) $(rustc_flags) $(rustc_cross_flags) \ > + --extern alloc --extern kernel \ > + --crate-type rlib --out-dir $(obj) -L $(objtree)/rust/ \ > + --crate-name $(patsubst %.o,%,$(notdir $@)) $<; \ > + 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, &kconfig_printer_cb, NULL); > conf_write_heading(out_h, &header_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, &kconfig_printer_cb, (void *)1); > conf_write_symbol(out_h, sym, &header_printer_cb, NULL); > + conf_write_symbol(out_rustc_cfg, sym, &rustc_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
nother profile, you 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: [GIT PULL][PATCH v9 0/3] Update to zstd-1.4.10
On Wed, Apr 14, 2021 at 12:04 PM Eric Biggers wrote: > > On Wed, Apr 14, 2021 at 11:53:51AM -0700, Nick Terrell wrote: > > On Wed, Apr 14, 2021 at 11:35 AM Eric Biggers wrote: > > > > > > On Wed, Apr 14, 2021 at 11:01:29AM -0700, Nick Terrell wrote: > > > > Hi all, > > > > > > > > I would really like to make some progress on this and get it merged. > > > > This patchset offsers: > > > > * 15-30% better decompression speed > > > > * 3 years of zstd bug fixes and code improvements > > > > * Allows us to import zstd directly from upstream so we don't fall 3 > > > > years out of date again > > > > > > > > Thanks, > > > > Nick > > > > > > > > > > I think it would help get it merged if someone actually volunteered to > > > maintain > > > it. As-is there is no entry in MAINTAINERS for this code. > > > > I was discussing with Chris Mason about volunteering to maintain the > > code myself. > > We wanted to wait until this series got merged before going that > > route, because there > > was already a lot of comments about it, and I didn't want to appear to > > be trying to bypass > > any review or criticisms. But, please let me know what you think. > > > > I expect that most people would like to see a commitment to maintain this code > before merging. The usual way to do that is to add a MAINTAINERS entry. > > Otherwise it is 27000 lines of code dumped on other people to maintain. I will add a 4th patch in the series to update the MAINTAINERS. > - Eric
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
Re: [GIT PULL][PATCH v9 0/3] Update to zstd-1.4.10
On Wed, Apr 14, 2021 at 11:35 AM Eric Biggers wrote: > > On Wed, Apr 14, 2021 at 11:01:29AM -0700, Nick Terrell wrote: > > Hi all, > > > > I would really like to make some progress on this and get it merged. > > This patchset offsers: > > * 15-30% better decompression speed > > * 3 years of zstd bug fixes and code improvements > > * Allows us to import zstd directly from upstream so we don't fall 3 > > years out of date again > > > > Thanks, > > Nick > > > > I think it would help get it merged if someone actually volunteered to > maintain > it. As-is there is no entry in MAINTAINERS for this code. I was discussing with Chris Mason about volunteering to maintain the code myself. We wanted to wait until this series got merged before going that route, because there was already a lot of comments about it, and I didn't want to appear to be trying to bypass any review or criticisms. But, please let me know what you think. Best, Nick > - Eric
Re: [GIT PULL][PATCH v9 0/3] Update to zstd-1.4.10
Hi all, I would really like to make some progress on this and get it merged. This patchset offsers: * 15-30% better decompression speed * 3 years of zstd bug fixes and code improvements * Allows us to import zstd directly from upstream so we don't fall 3 years out of date again Thanks, Nick On Fri, Apr 9, 2021 at 2:39 PM Nick Terrell wrote: > > What can I do to help get this merged? > > Cristoph, is this new patch series with the kernel wrapper API satisfactory? > > Best, > Nick > > On Tue, Mar 30, 2021 at 3:45 PM Nick Terrell wrote: > > > > From: Nick Terrell > > > > Please pull from > > > > g...@github.com:terrelln/linux.git tags/v9-zstd-1.4.10 > > > > to get these changes. Alternatively the patchset is included. > > > > This patchset upgrades the zstd library to the latest upstream release. The > > current zstd version in the kernel is a modified version of upstream > > zstd-1.3.1. > > At the time it was integrated, zstd wasn't ready to be used in the kernel > > as-is. > > But, it is now possible to use upstream zstd directly in the kernel. > > > > I have not yet released zstd-1.4.10 upstream. I want the zstd version in the > > kernel to match up with a known upstream release, so we know exactly what > > code > > is running. Whenever this patchset is ready for merge, I will cut a release > > at > > the upstream commit that gets merged. This should not be necessary for > > future > > releases. > > > > The kernel zstd library is automatically generated from upstream zstd. A > > script > > makes the necessary changes and imports it into the kernel. The changes are: > > > > 1. Replace all libc dependencies with kernel replacements and rewrite > > includes. > > 2. Remove unncessary portability macros like: #if defined(_MSC_VER). > > 3. Use the kernel xxhash instead of bundling it. > > > > This automation gets tested every commit by upstream's continuous > > integration. > > When we cut a new zstd release, we will submit a patch to the kernel to > > update > > the zstd version in the kernel. > > > > I've updated zstd to upstream with one big patch because every commit must > > build, > > so that precludes partial updates. Since the commit is 100% generated, I > > hope the > > review burden is lightened. I considered replaying upstream commits, but > > that is > > not possible because there have been ~3500 upstream commits since the last > > zstd > > import, and the commits don't all build individually. The bulk update > > preserves > > bisectablity because bugs can be bisected to the zstd version update. At > > that > > point the update can be reverted, and we can work with upstream to find and > > fix > > the bug. After this big switch in how the kernel consumes zstd, future > > patches > > will be smaller, because they will only have one upstream release worth of > > changes each. > > > > This patchset adds a new kernel-style wrapper around zstd. This wrapper API > > is > > functionally equivalent to the subset of the current zstd API that is > > currently > > used. The wrapper API changes to be kernel style so that the symbols don't > > collide with zstd's symbols. The update to zstd-1.4.6 maintains the same API > > and preserves the semantics, so that none of the callers need to be updated. > > > > This patchset comes in 2 parts: > > 1. The first 2 patches prepare for the zstd upgrade. The first patch adds > > the > >new kernel style API so zstd can be upgraded without modifying any > > callers. > >The second patch adds an indirection for the lib/decompress_unzstd.c > >including of all decompression source files. > > 2. Import zstd-1.4.10. This patch is completely generated from upstream > > using > >automated tooling. > > > > I tested every caller of zstd on x86_64. I tested both after the 1.4.10 > > upgrade > > using the compatibility wrapper, and after the final patch in this series. > > > > I tested kernel and initramfs decompression in i386 and arm. > > > > I ran benchmarks to compare the current zstd in the kernel with zstd-1.4.6. > > I benchmarked on x86_64 using QEMU with KVM enabled on an Intel i9-9900k. > > I found: > > * BtrFS zstd compression at levels 1 and 3 is 5% faster > > * BtrFS zstd decompression+read is 15% faster > > * SquashFS zstd decompression+read is 15% faster > > * F2FS zstd compression+write at level 3 is 8% faster > > *
[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(¤t_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(&info->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(&info->head, ¤t_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(&info->head, ¤t_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(&fn_ptr2->head, &info2->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)
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(&fn_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: [PATCH -next] lib: zstd: Make symbol 'HUF_compressWeights_wksp' static
> On Apr 8, 2021, at 8:09 PM, Miguel Ojeda > wrote: > > On Fri, Apr 9, 2021 at 2:20 AM Nick Desaulniers > wrote: >> >> 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? > > Yeah, the equivalent cleanup was done upstream by Nick in 2018 [1], > but there has been no major update to lib/zstd since 2017. > > Thus a cleanup would actually make it closer to upstream, which is the > best case scenario :) > >Reviewed-by: Miguel Ojeda > > [1] > https://github.com/facebook/zstd/commit/f2d6db45cd28457fa08467416e8535985f062859 This looks good to me as well. I have a patchset up to use upstream zstd directly in the kernel [0]. That will allow us to keep zstd up to date. And after that lands, I hope to set up a zstd linux tree to make merging patches into lib/zstd easier, since over the years quite a few have been ignored. [0] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2532407.html Best, Nick Terrell > Cheers, > Miguel
Re: [GIT PULL][PATCH v9 0/3] Update to zstd-1.4.10
What can I do to help get this merged? Cristoph, is this new patch series with the kernel wrapper API satisfactory? Best, Nick On Tue, Mar 30, 2021 at 3:45 PM Nick Terrell wrote: > > From: Nick Terrell > > Please pull from > > g...@github.com:terrelln/linux.git tags/v9-zstd-1.4.10 > > to get these changes. Alternatively the patchset is included. > > This patchset upgrades the zstd library to the latest upstream release. The > current zstd version in the kernel is a modified version of upstream > zstd-1.3.1. > At the time it was integrated, zstd wasn't ready to be used in the kernel > as-is. > But, it is now possible to use upstream zstd directly in the kernel. > > I have not yet released zstd-1.4.10 upstream. I want the zstd version in the > kernel to match up with a known upstream release, so we know exactly what code > is running. Whenever this patchset is ready for merge, I will cut a release at > the upstream commit that gets merged. This should not be necessary for future > releases. > > The kernel zstd library is automatically generated from upstream zstd. A > script > makes the necessary changes and imports it into the kernel. The changes are: > > 1. Replace all libc dependencies with kernel replacements and rewrite > includes. > 2. Remove unncessary portability macros like: #if defined(_MSC_VER). > 3. Use the kernel xxhash instead of bundling it. > > This automation gets tested every commit by upstream's continuous integration. > When we cut a new zstd release, we will submit a patch to the kernel to update > the zstd version in the kernel. > > I've updated zstd to upstream with one big patch because every commit must > build, > so that precludes partial updates. Since the commit is 100% generated, I hope > the > review burden is lightened. I considered replaying upstream commits, but that > is > not possible because there have been ~3500 upstream commits since the last > zstd > import, and the commits don't all build individually. The bulk update > preserves > bisectablity because bugs can be bisected to the zstd version update. At that > point the update can be reverted, and we can work with upstream to find and > fix > the bug. After this big switch in how the kernel consumes zstd, future patches > will be smaller, because they will only have one upstream release worth of > changes each. > > This patchset adds a new kernel-style wrapper around zstd. This wrapper API is > functionally equivalent to the subset of the current zstd API that is > currently > used. The wrapper API changes to be kernel style so that the symbols don't > collide with zstd's symbols. The update to zstd-1.4.6 maintains the same API > and preserves the semantics, so that none of the callers need to be updated. > > This patchset comes in 2 parts: > 1. The first 2 patches prepare for the zstd upgrade. The first patch adds the >new kernel style API so zstd can be upgraded without modifying any callers. >The second patch adds an indirection for the lib/decompress_unzstd.c >including of all decompression source files. > 2. Import zstd-1.4.10. This patch is completely generated from upstream using >automated tooling. > > I tested every caller of zstd on x86_64. I tested both after the 1.4.10 > upgrade > using the compatibility wrapper, and after the final patch in this series. > > I tested kernel and initramfs decompression in i386 and arm. > > I ran benchmarks to compare the current zstd in the kernel with zstd-1.4.6. > I benchmarked on x86_64 using QEMU with KVM enabled on an Intel i9-9900k. > I found: > * BtrFS zstd compression at levels 1 and 3 is 5% faster > * BtrFS zstd decompression+read is 15% faster > * SquashFS zstd decompression+read is 15% faster > * F2FS zstd compression+write at level 3 is 8% faster > * F2FS zstd decompression+read is 20% faster > * ZRAM decompression+read is 30% faster > * Kernel zstd decompression is 35% faster > * Initramfs zstd decompression+build is 5% faster > > The latest zstd also offers bug fixes. For example the problem with large > kernel > decompression has been fixed upstream for over 2 years > https://lkml.org/lkml/2020/9/29/27. > > Please let me know if there is anything that I can do to ease the way for > these > patches. I think it is important because it gets large performance > improvements, > contains bug fixes, and is switching to a more maintainable model of consuming > upstream zstd directly, making it easy to keep up to date. > > Best, > Nick Terrell > > v1 -> v2: > * Successfully tested F2FS with help from Chao Yu to fix my test. > * (1/9) Fix ZSTD_initCStream() wrapper to handle pledged_src_si
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 v3 4/5] RISC-V: Add kdump support
Στις 2021-04-06 21:36, Alex Ghiti έγραψε: + /* Switch to physical addressing */ + la s4, 1f + sub s4, s4, s3 + csrwstvec, s4 + csrwsptbr, zero satp is used everywhere instead of sptbr. And maybe you could CSR_ naming, like you did in riscv_crash_save_regs and like it's done in head.S too. ACK + crash_base = memblock_find_in_range(search_start, search_end, +#ifdef CONFIG_64BIT + crash_size, SZ_2M); +#else + crash_size, SZ_4M); +#endif You can use PMD_SIZE here and get rid of #ifdef. + +#ifdef CONFIG_64BIT + if (!IS_ALIGNED(crash_base, SZ_2M)) { +#else + if (!IS_ALIGNED(crash_base, SZ_4M)) { +#endif Ditto here. Will do. Thanks a lot for your review !
Re: [PATCH v3 2/5] RISC-V: Add kexec support
Στις 2021-04-06 21:38, Alex Ghiti έγραψε: Le 4/5/21 à 4:57 AM, Nick Kossifidis a écrit : + +/* Reserve a page for the control code buffer */ +#define KEXEC_CONTROL_PAGE_SIZE 4096 PAGE_SIZE instead ? Yup, I'll change it. +obj-${CONFIG_KEXEC}+= kexec_relocate.o machine_kexec.o Other obj-$() use parenthesis. ACK + li s5, ((1 << PAGE_SHIFT) / RISCV_SZPTR) 1 << PAGE_SHIFT = PAGE_SIZE ACK +#if defined(CONFIG_HOTPLUG_CPU) && (CONFIG_SMP) Shouldn't it be defined(CONFIG_SMP) ? It depends on SMP anyway, I'll remove the second part.
Re: [PATCH v3 3/5] RISC-V: Improve init_resources
Στις 2021-04-06 11:22, Geert Uytterhoeven έγραψε: Hi Nick, On Tue, Apr 6, 2021 at 10:11 AM Nick Kossifidis wrote: Hello Geert, Στις 2021-04-06 10:19, Geert Uytterhoeven έγραψε: > On Mon, Apr 5, 2021 at 10:57 AM Nick Kossifidis > wrote: >> * Kernel region is always present and we know where it is, no >> need to look for it inside the loop, just ignore it like the >> rest of the reserved regions within system's memory. >> >> * Don't call memblock_free inside the loop, if called it'll split >> the region of pre-allocated resources in two parts, messing things >> up, just re-use the previous pre-allocated resource and free any >> unused resources after both loops finish. >> >> * memblock_alloc may add a region when called, so increase the >> number of pre-allocated regions by one to be on the safe side >> (reported and patched by Geert Uytterhoeven) >> >> Signed-off-by: Geert Uytterhoeven > > Where does this SoB come from? > >> Signed-off-by: Nick Kossifidis > >> --- a/arch/riscv/kernel/setup.c >> +++ b/arch/riscv/kernel/setup.c > >> @@ -129,53 +139,42 @@ static void __init init_resources(void) >> struct resource *res = NULL; >> struct resource *mem_res = NULL; >> size_t mem_res_sz = 0; >> - int ret = 0, i = 0; >> - >> - code_res.start = __pa_symbol(_text); >> - code_res.end = __pa_symbol(_etext) - 1; >> - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> - >> - rodata_res.start = __pa_symbol(__start_rodata); >> - rodata_res.end = __pa_symbol(__end_rodata) - 1; >> - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> - >> - data_res.start = __pa_symbol(_data); >> - data_res.end = __pa_symbol(_edata) - 1; >> - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> + int num_resources = 0, res_idx = 0; >> + int ret = 0; >> >> - bss_res.start = __pa_symbol(__bss_start); >> - bss_res.end = __pa_symbol(__bss_stop) - 1; >> - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; >> + /* + 1 as memblock_alloc() might increase >> memblock.reserved.cnt */ >> + num_resources = memblock.memory.cnt + memblock.reserved.cnt + >> 1; >> + res_idx = num_resources - 1; >> >> - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * >> sizeof(*mem_res); > > Oh, you incorporated my commit ce989f1472ae350e ("RISC-V: Fix > out-of-bounds > accesses in init_resources()") (from v5.12-rc4) into your patch. > Why? This means your patch does not apply against upstream. > Sorry if this looks awkward, I'm under the impression that new features go on for-next instead of fixes and your patch hasn't been merged on for-next yet. I thought it would be cleaner to have one patch to merge for init_resources instead of two, and simpler for people to test the series. I can rebase this on top of fixes if that works better for you or Palmer. Ideally the fixes branch is part of the next branch. That also helps to avoid other people having to fix conflicts when merging both. OK I'll re-base this on top of fixes instead.
Re: [PATCH v3 0/5] RISC-V: Add kexec/kdump support
Στις 2021-04-07 19:29, Rob Herring έγραψε: On Mon, Apr 05, 2021 at 11:57:07AM +0300, Nick Kossifidis wrote: This patch series adds kexec/kdump and crash kernel support on RISC-V. For testing the patches a patched version of kexec-tools is needed (still a work in progress) which can be found at: https://riscv.ics.forth.gr/kexec-tools-patched.tar.xz v3: * Rebase on newer kernel tree * Minor cleanups * Split UAPI changes to a separate patch * Improve / cleanup init_resources * Resolve Palmer's comments v2: * Rebase on newer kernel tree * Minor cleanups * Properly populate the ioresources tre, so that it can be used later on for implementing strict /dev/mem * Use linux,usable-memory on /memory instead of a new binding Where? In any case, that's not going to work well with EFI support assuming like arm64, 'memory' is passed in UEFI structures instead of DT. That's why there's now a /chosen linux,usable-memory-ranges property. Here: https://elixir.bootlin.com/linux/v5.12-rc5/source/drivers/of/fdt.c#L1001 The "linux,usable-memory" binding is already defined and is part of early_init_dt_scan_memory() which we call on mm/init.c to determine system's memory layout. It's simple, clean and I don't see a reason to use another binding on /chosen and add extra code for this, when we already handle it on early_init_dt_scan_memory() anyway. As for EFI, even when enabled, we still use DT to determine system memory layout, not EFI structures, plus I don't see how EFI is relevant here, the bootloader in kexec's case is Linux, not EFI. BTW the /memory node is mandatory in any case, it should exist on DT regardless of EFI, /chosen node on the other hand is -in general- optional, and we can still boot a riscv system without /chosen node present (we only require it for the built-in cmdline to work). Also a simple grep for "linux,usable-memory-ranges" on the latest kernel sources didn't return anything, there is also nothing on chosen.txt, where is that binding documented/implemented ? Isn't the preferred kexec interface the file based interface? I'd expect a new arch to only support that. And there's common kexec DT handling for that pending for 5.13. Both approaches have their pros an cons, that's why both are available, in no way CONFIG_KEXEC is deprecated in favor of CONFIG_KEXEC_FILE, at least not as far as I know. The main point for the file-based syscall is to support secure boot, since the image is loaded by the kernel directly without any processing by the userspace tools, so it can be pre-signed by the kernel's "vendor". On the other hand, the kernel part is more complicated and you can't pass a new device tree, the kernel needs to re-use the existing one (or modify it in-kernel), you can only override the cmdline. This doesn't work for our use cases in FORTH, where we use kexec not only to re-boot our systems, but also to boot to a system with different hw layout (e.g. FPGA prototypes or systems with FPGAs on the side), device tree overlays also don't cover our use cases. To give you an idea we can add/remove/modify devices, move them to another region etc and still use kexec to avoid going through the full boot cycle. We just unload their drivers, perform a full or partial re-programming of the FPGA from within Linux, and kexec to the new system with the new device tree. The file-based syscall can't cover this scenario, in general it's less flexible and it's only there for secure boot, not for using custom-built kernels, nor custom device tree images. Security-wise the file load syscall provides guarantees for integrity and authenticity, but depending on the kernel "vendor"'s infrastructure and signing process this may allow e.g. to load an older/vulnerable kernel through kexec and get away with it, there is no check as far as I know to make sure the loaded kernel is at least as old as the running kernel, the assumption is that the "vendor" will use a different signing key/cert for each kernel and that you'll kexec to a kernel/crash kernel that's the same version as the running one. Until we have clear guidelines on how this is meant to be used and have a discussion on secure boot within RISC-V (we have something on the TEE TG but we'll probably switch to a SIG committee for this), I don't see how this feature is a priority compared to the more generic CONFIG_KEXEC. Regards, Nick
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(&info->head, ¤t_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(&info->head); info->ident = ident; info->checksum = func_checksum; - info->use_extra_checksum = use_extra_checksum; info->cfg_checksum = cfg_checksum; list_add_tail(&info->head, ¤t_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(&fn_ptr2->head, &info2->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, &info->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(&info->head); - info->ident = ident; - info->checksum = func_checksum; - info->use_extra_checksum = use_extra_checksum; - info->cfg_checksum = cfg_checksum; - list_add_tail(&info->head, ¤t_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(&info->head, ¤t_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(&fn_ptr2->head, &info2->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(&info->head, ¤t_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(&info->head); + info->ident = ident; + info->checksum = func_checksum; + info->cfg_checksum = cfg_checksum; + list_add_tail(&info->head, ¤t_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(&fn_ptr2->head, &info2->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, &info->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] init: add support for zstd compressed modules
> On Apr 7, 2021, at 6:53 AM, Masahiro Yamada wrote: > > On Thu, Apr 1, 2021 at 4:21 AM Nick Terrell wrote: >> >> >> >>> On Mar 31, 2021, at 10:48 AM, Oleksandr Natalenko >>> wrote: >>> >>> Hello. >>> >>> On Wed, Mar 31, 2021 at 05:39:25PM +, Nick Terrell wrote: >>>> >>>> >>>>> On Mar 30, 2021, at 4:50 AM, Oleksandr Natalenko >>>>> wrote: >>>>> >>>>> On Tue, Mar 30, 2021 at 01:32:35PM +0200, Piotr Gorski wrote: >>>>>> kmod 28 supports modules compressed in zstd format so let's add this >>>>>> possibility to kernel. >>>>>> >>>>>> Signed-off-by: Piotr Gorski >>>>>> --- >>>>>> Makefile | 7 +-- >>>>>> init/Kconfig | 9 ++--- >>>>>> 2 files changed, 11 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/Makefile b/Makefile >>>>>> index 5160ff8903c1..82f4f4cc2955 100644 >>>>>> --- a/Makefile >>>>>> +++ b/Makefile >>>>>> @@ -1156,8 +1156,8 @@ endif # INSTALL_MOD_STRIP >>>>>> export mod_strip_cmd >>>>>> >>>>>> # CONFIG_MODULE_COMPRESS, if defined, will cause module to be compressed >>>>>> -# after they are installed in agreement with CONFIG_MODULE_COMPRESS_GZIP >>>>>> -# or CONFIG_MODULE_COMPRESS_XZ. >>>>>> +# after they are installed in agreement with >>>>>> CONFIG_MODULE_COMPRESS_GZIP, >>>>>> +# CONFIG_MODULE_COMPRESS_XZ, or CONFIG_MODULE_COMPRESS_ZSTD. >>>>>> >>>>>> mod_compress_cmd = true >>>>>> ifdef CONFIG_MODULE_COMPRESS >>>>>> @@ -1167,6 +1167,9 @@ ifdef CONFIG_MODULE_COMPRESS >>>>>> ifdef CONFIG_MODULE_COMPRESS_XZ >>>>>> mod_compress_cmd = $(XZ) --lzma2=dict=2MiB -f >>>>>> endif # CONFIG_MODULE_COMPRESS_XZ >>>>>> + ifdef CONFIG_MODULE_COMPRESS_ZSTD >>>>>> +mod_compress_cmd = $(ZSTD) -T0 --rm -f -q >>>> >>>> This will use the default zstd level, level 3. I think it would make more >>>> sense to use a high >>>> compression level. Level 19 would probably be a good choice. That will >>>> choose a window >>>> size of up to 8MB, meaning the decompressor needs to allocate that much >>>> memory. If that >>>> is unacceptable, you could use `zstd -T0 --rm -f -q -19 --zstd=wlog=21`, >>>> which will use a >>>> window size of up to 2MB, to match the XZ command. Note that if the file >>>> is smaller than >>>> the window size, it will be shrunk to the smallest power of two at least >>>> as large as the file. >>> >>> Please no. We've already done that with initramfs in Arch, and it >>> increased the time to generate it enormously. >>> >>> I understand that building a kernel is a more rare operation than >>> regenerating initramfs, but still I'd go against hard-coding the level. >>> And if it should be specified anyway, I'd opt in for an explicit >>> configuration option. Remember, not all the kernel are built on >>> build farms... >>> >>> FWIW, Piotr originally used level 9 which worked okay, but I insisted >>> on sending the patch initially without specifying level at all like it is >>> done for other compressors. If this is a wrong approach, then oh meh, >>> mea culpa ;). >>> >>> Whatever default non-standard compression level you choose, I'm fine >>> as long as I can change it without editing Makefile. >> >> That makes sense to me. I have a deep seated need to compress files as >> efficiently as possible for widely distributed packages. But, I understand >> that >> slow compression significantly impacts build times for quick iteration. I’d >> be >> happy with a compression level parameter that defaults to a happy middle. >> >> I’m also fine with taking this patch as-is if it is easier, and I can put up >> another >> patch that adds a compression level parameter, since I don’t want to block >> merging this. > > > I do not want to take such a patch. > Meeking everyone's requirement > results in a bad project for everyone. > > > Does this work for you? > > make modules_install ZSTD="zstd -19"
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 v3 3/5] RISC-V: Improve init_resources
Hello Geert, Στις 2021-04-06 10:19, Geert Uytterhoeven έγραψε: Hi Nick, Thanks for your patch! On Mon, Apr 5, 2021 at 10:57 AM Nick Kossifidis wrote: * Kernel region is always present and we know where it is, no need to look for it inside the loop, just ignore it like the rest of the reserved regions within system's memory. * Don't call memblock_free inside the loop, if called it'll split the region of pre-allocated resources in two parts, messing things up, just re-use the previous pre-allocated resource and free any unused resources after both loops finish. * memblock_alloc may add a region when called, so increase the number of pre-allocated regions by one to be on the safe side (reported and patched by Geert Uytterhoeven) Signed-off-by: Geert Uytterhoeven Where does this SoB come from? Signed-off-by: Nick Kossifidis --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -129,53 +139,42 @@ static void __init init_resources(void) struct resource *res = NULL; struct resource *mem_res = NULL; size_t mem_res_sz = 0; - int ret = 0, i = 0; - - code_res.start = __pa_symbol(_text); - code_res.end = __pa_symbol(_etext) - 1; - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - rodata_res.start = __pa_symbol(__start_rodata); - rodata_res.end = __pa_symbol(__end_rodata) - 1; - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - data_res.start = __pa_symbol(_data); - data_res.end = __pa_symbol(_edata) - 1; - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + int num_resources = 0, res_idx = 0; + int ret = 0; - bss_res.start = __pa_symbol(__bss_start); - bss_res.end = __pa_symbol(__bss_stop) - 1; - bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + /* + 1 as memblock_alloc() might increase memblock.reserved.cnt */ + num_resources = memblock.memory.cnt + memblock.reserved.cnt + 1; + res_idx = num_resources - 1; - mem_res_sz = (memblock.memory.cnt + memblock.reserved.cnt) * sizeof(*mem_res); Oh, you incorporated my commit ce989f1472ae350e ("RISC-V: Fix out-of-bounds accesses in init_resources()") (from v5.12-rc4) into your patch. Why? This means your patch does not apply against upstream. Sorry if this looks awkward, I'm under the impression that new features go on for-next instead of fixes and your patch hasn't been merged on for-next yet. I thought it would be cleaner to have one patch to merge for init_resources instead of two, and simpler for people to test the series. I can rebase this on top of fixes if that works better for you or Palmer. Regards, Nick
[PATCH v3 1/5] RISC-V: Add EM_RISCV to kexec UAPI header
Add RISC-V to the list of supported kexec architecturs, we need to add the definition early-on so that later patches can use it. EM_RISCV is 243 as per ELF psABI specification here: https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md Signed-off-by: Nick Kossifidis --- include/uapi/linux/kexec.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/uapi/linux/kexec.h b/include/uapi/linux/kexec.h index 05669c87a..778dc191c 100644 --- a/include/uapi/linux/kexec.h +++ b/include/uapi/linux/kexec.h @@ -42,6 +42,7 @@ #define KEXEC_ARCH_MIPS_LE (10 << 16) #define KEXEC_ARCH_MIPS( 8 << 16) #define KEXEC_ARCH_AARCH64 (183 << 16) +#define KEXEC_ARCH_RISCV (243 << 16) /* The artificial cap on the number of segments passed to kexec_load. */ #define KEXEC_SEGMENT_MAX 16 -- 2.26.2
[PATCH v3 3/5] RISC-V: Improve init_resources
* Kernel region is always present and we know where it is, no need to look for it inside the loop, just ignore it like the rest of the reserved regions within system's memory. * Don't call memblock_free inside the loop, if called it'll split the region of pre-allocated resources in two parts, messing things up, just re-use the previous pre-allocated resource and free any unused resources after both loops finish. * memblock_alloc may add a region when called, so increase the number of pre-allocated regions by one to be on the safe side (reported and patched by Geert Uytterhoeven) Signed-off-by: Geert Uytterhoeven Signed-off-by: Nick Kossifidis --- arch/riscv/kernel/setup.c | 90 --- 1 file changed, 46 insertions(+), 44 deletions(-) diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index e85bacff1..030554bab 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -60,6 +60,7 @@ static DEFINE_PER_CPU(struct cpu, cpu_devices); * also add "System RAM" regions for compatibility with other * archs, and the rest of the known regions for completeness. */ +static struct resource kimage_res = { .name = "Kernel image", }; static struct resource code_res = { .name = "Kernel code", }; static struct resource data_res = { .name = "Kernel data", }; static struct resource rodata_res = { .name = "Kernel rodata", }; @@ -80,45 +81,54 @@ static int __init add_resource(struct resource *parent, return 1; } -static int __init add_kernel_resources(struct resource *res) +static int __init add_kernel_resources(void) { int ret = 0; /* * The memory region of the kernel image is continuous and -* was reserved on setup_bootmem, find it here and register -* it as a resource, then register the various segments of -* the image as child nodes +* was reserved on setup_bootmem, register it here as a +* resource, with the various segments of the image as +* child nodes. */ - if (!(res->start <= code_res.start && res->end >= data_res.end)) - return 0; - res->name = "Kernel image"; - res->flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + code_res.start = __pa_symbol(_text); + code_res.end = __pa_symbol(_etext) - 1; + code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - /* -* We removed a part of this region on setup_bootmem so -* we need to expand the resource for the bss to fit in. -*/ - res->end = bss_res.end; + rodata_res.start = __pa_symbol(__start_rodata); + rodata_res.end = __pa_symbol(__end_rodata) - 1; + rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - ret = add_resource(&iomem_resource, res); + data_res.start = __pa_symbol(_data); + data_res.end = __pa_symbol(_edata) - 1; + data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + bss_res.start = __pa_symbol(__bss_start); + bss_res.end = __pa_symbol(__bss_stop) - 1; + bss_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + kimage_res.start = code_res.start; + kimage_res.end = bss_res.end; + kimage_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + + ret = add_resource(&iomem_resource, &kimage_res); if (ret < 0) return ret; - ret = add_resource(res, &code_res); + ret = add_resource(&kimage_res, &code_res); if (ret < 0) return ret; - ret = add_resource(res, &rodata_res); + ret = add_resource(&kimage_res, &rodata_res); if (ret < 0) return ret; - ret = add_resource(res, &data_res); + ret = add_resource(&kimage_res, &data_res); if (ret < 0) return ret; - ret = add_resource(res, &bss_res); + ret = add_resource(&kimage_res, &bss_res); return ret; } @@ -129,53 +139,42 @@ static void __init init_resources(void) struct resource *res = NULL; struct resource *mem_res = NULL; size_t mem_res_sz = 0; - int ret = 0, i = 0; - - code_res.start = __pa_symbol(_text); - code_res.end = __pa_symbol(_etext) - 1; - code_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - rodata_res.start = __pa_symbol(__start_rodata); - rodata_res.end = __pa_symbol(__end_rodata) - 1; - rodata_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; - - data_res.start = __pa_symbol(_data); - data_res.end = __pa_symbol(_edata) - 1; - data_res.flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY; + int num_resources = 0, res_idx = 0; + int ret = 0; - bss_res.start = __pa_symbol(__bss_start); - bss
[PATCH v3 5/5] RISC-V: Add crash kernel support
From: Nick Kossifidis This patch allows Linux to act as a crash kernel for use with kdump. Userspace will let the crash kernel know about the memory region it can use through linux,usable-memory property on the /memory node (overriding its reg property), and about the memory region where the elf core header of the previous kernel is saved, through a reserved-memory node with a compatible string of "linux,elfcorehdr". This approach is the least invasive and re-uses functionality already present. I tested this on riscv64 qemu and it works as expected, you may test it by retrieving the dmesg of the previous kernel through /proc/vmcore, using the vmcore-dmesg utility from kexec-tools. v3: * Rebase v2: * Use linux,usable-memory on /memory instead of a new binding * Use a reserved-memory node for ELF core header Signed-off-by: Nick Kossifidis --- arch/riscv/Kconfig | 10 arch/riscv/kernel/Makefile | 1 + arch/riscv/kernel/crash_dump.c | 46 ++ arch/riscv/kernel/setup.c | 12 + arch/riscv/mm/init.c | 33 5 files changed, 102 insertions(+) create mode 100644 arch/riscv/kernel/crash_dump.c diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 3716262ef..553c2dced 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -403,6 +403,16 @@ config KEXEC The name comes from the similarity to the exec system call. +config CRASH_DUMP + bool "Build kdump crash kernel" + help + Generate crash dump after being started by kexec. This should + be normally only set in special crash dump kernels which are + loaded in the main kernel with kexec-tools into a specially + reserved region and then later executed after a crash by + kdump/kexec. + + For more details see Documentation/admin-guide/kdump/kdump.rst endmenu diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index 07f676ad3..bd66d2ce0 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -59,6 +59,7 @@ endif obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o obj-$(CONFIG_KGDB) += kgdb.o obj-${CONFIG_KEXEC}+= kexec_relocate.o crash_save_regs.o machine_kexec.o +obj-$(CONFIG_CRASH_DUMP) += crash_dump.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o diff --git a/arch/riscv/kernel/crash_dump.c b/arch/riscv/kernel/crash_dump.c new file mode 100644 index 0..86cc0ada5 --- /dev/null +++ b/arch/riscv/kernel/crash_dump.c @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * This code comes from arch/arm64/kernel/crash_dump.c + * Created by: AKASHI Takahiro + * Copyright (C) 2017 Linaro Limited + */ + +#include +#include + +/** + * copy_oldmem_page() - copy one page from old kernel memory + * @pfn: page frame number to be copied + * @buf: buffer where the copied page is placed + * @csize: number of bytes to copy + * @offset: offset in bytes into the page + * @userbuf: if set, @buf is in a user address space + * + * This function copies one page from old kernel memory into buffer pointed by + * @buf. If @buf is in userspace, set @userbuf to %1. Returns number of bytes + * copied or negative error in case of failure. + */ +ssize_t copy_oldmem_page(unsigned long pfn, char *buf, +size_t csize, unsigned long offset, +int userbuf) +{ + void *vaddr; + + if (!csize) + return 0; + + vaddr = memremap(__pfn_to_phys(pfn), PAGE_SIZE, MEMREMAP_WB); + if (!vaddr) + return -ENOMEM; + + if (userbuf) { + if (copy_to_user((char __user *)buf, vaddr + offset, csize)) { + memunmap(vaddr); + return -EFAULT; + } + } else + memcpy(buf, vaddr + offset, csize); + + memunmap(vaddr); + return csize; +} diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 31866dce9..ff398a3d8 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -66,6 +66,9 @@ static struct resource code_res = { .name = "Kernel code", }; static struct resource data_res = { .name = "Kernel data", }; static struct resource rodata_res = { .name = "Kernel rodata", }; static struct resource bss_res = { .name = "Kernel bss", }; +#ifdef CONFIG_CRASH_DUMP +static struct resource elfcorehdr_res = { .name = "ELF Core hdr", }; +#endif static int __init add_resource(struct resource *parent, struct resource *res) @@ -169,6 +172,15 @@ static void __init init_resources(void) } #endif +#ifdef CONFIG_CRASH_DUMP + if (elfcorehdr_size > 0) { + elfcorehdr_res.start = elfcorehdr_addr; + elfcorehdr_res.end = elfcorehdr_addr + elfcorehdr_size - 1; +
[PATCH v3 2/5] RISC-V: Add kexec support
This patch adds support for kexec on RISC-V. On SMP systems it depends on HOTPLUG_CPU in order to be able to bring up all harts after kexec. It also needs a recent OpenSBI version that supports the HSM extension. I tested it on riscv64 QEMU on both an smp and a non-smp system. v5: * For now depend on MMU, further changes needed for NOMMU support * Make sure stvec is aligned * Cleanup some unneeded fences * Verify control code's buffer size * Compile kexec_relocate.S with medany and norelax v4: * No functional changes, just re-based v3: * Use the new smp_shutdown_nonboot_cpus() call. * Move riscv_kexec_relocate to .rodata v2: * Pass needed parameters as arguments to riscv_kexec_relocate instead of using global variables. * Use kimage_arch to hold the fdt address of the included fdt. * Use SYM_* macros on kexec_relocate.S. * Compatibility with STRICT_KERNEL_RWX. * Compatibility with HOTPLUG_CPU for SMP * Small cleanups Signed-off-by: Nick Kossifidis --- arch/riscv/Kconfig | 15 +++ arch/riscv/include/asm/kexec.h | 47 arch/riscv/kernel/Makefile | 5 + arch/riscv/kernel/kexec_relocate.S | 156 arch/riscv/kernel/machine_kexec.c | 186 + 5 files changed, 409 insertions(+) create mode 100644 arch/riscv/include/asm/kexec.h create mode 100644 arch/riscv/kernel/kexec_relocate.S create mode 100644 arch/riscv/kernel/machine_kexec.c diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 8ea60a0a1..3716262ef 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -389,6 +389,21 @@ config RISCV_SBI_V01 help This config allows kernel to use SBI v0.1 APIs. This will be deprecated in future once legacy M-mode software are no longer in use. + +config KEXEC + bool "Kexec system call" + select KEXEC_CORE + select HOTPLUG_CPU if SMP + depends on MMU + help + kexec is a system call that implements the ability to shutdown your + current kernel, and to start another kernel. It is like a reboot + but it is independent of the system firmware. And like a reboot + you can start any kernel with it, not just Linux. + + The name comes from the similarity to the exec system call. + + endmenu menu "Boot options" diff --git a/arch/riscv/include/asm/kexec.h b/arch/riscv/include/asm/kexec.h new file mode 100644 index 0..efc69feb4 --- /dev/null +++ b/arch/riscv/include/asm/kexec.h @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2019 FORTH-ICS/CARV + * Nick Kossifidis + */ + +#ifndef _RISCV_KEXEC_H +#define _RISCV_KEXEC_H + +/* Maximum physical address we can use pages from */ +#define KEXEC_SOURCE_MEMORY_LIMIT (-1UL) + +/* Maximum address we can reach in physical address mode */ +#define KEXEC_DESTINATION_MEMORY_LIMIT (-1UL) + +/* Maximum address we can use for the control code buffer */ +#define KEXEC_CONTROL_MEMORY_LIMIT (-1UL) + +/* Reserve a page for the control code buffer */ +#define KEXEC_CONTROL_PAGE_SIZE 4096 + +#define KEXEC_ARCH KEXEC_ARCH_RISCV + +static inline void +crash_setup_regs(struct pt_regs *newregs, +struct pt_regs *oldregs) +{ + /* Dummy implementation for now */ +} + + +#define ARCH_HAS_KIMAGE_ARCH + +struct kimage_arch { + unsigned long fdt_addr; +}; + +const extern unsigned char riscv_kexec_relocate[]; +const extern unsigned int riscv_kexec_relocate_size; + +typedef void (*riscv_kexec_do_relocate)(unsigned long first_ind_entry, + unsigned long jump_addr, + unsigned long fdt_addr, + unsigned long hartid, + unsigned long va_pa_off); + +#endif diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index 3dc0abde9..c2594018c 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -9,6 +9,10 @@ CFLAGS_REMOVE_patch.o = $(CC_FLAGS_FTRACE) CFLAGS_REMOVE_sbi.o= $(CC_FLAGS_FTRACE) endif +ifdef CONFIG_KEXEC +AFLAGS_kexec_relocate.o := -mcmodel=medany -mno-relax +endif + extra-y += head.o extra-y += vmlinux.lds @@ -54,6 +58,7 @@ obj-$(CONFIG_SMP) += cpu_ops_sbi.o endif obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o obj-$(CONFIG_KGDB) += kgdb.o +obj-${CONFIG_KEXEC}+= kexec_relocate.o machine_kexec.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o diff --git a/arch/riscv/kernel/kexec_relocate.S b/arch/riscv/kernel/kexec_relocate.S new file mode 100644 index 0..616c20771 --- /dev/null +++ b/arch/riscv/kernel/kexec_relocate.S @@ -0,0 +1,156 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2019 FORTH-ICS/CARV + * Nick Kossifidis + */ + +#include/* For RISCV_* and REG_* macros */ +#include /* For PAGE_SHIFT */ +#include /* For SYM_* macro
[PATCH v3 4/5] RISC-V: Add kdump support
This patch adds support for kdump, the kernel will reserve a region for the crash kernel and jump there on panic. In order for userspace tools (kexec-tools) to prepare the crash kernel kexec image, we also need to expose some information on /proc/iomem for the memory regions used by the kernel and for the region reserved for crash kernel. Note that on userspace the device tree is used to determine the system's memory layout so the "System RAM" on /proc/iomem is ignored. I tested this on riscv64 qemu and works as expected, you may test it by triggering a crash through /proc/sysrq_trigger: echo c > /proc/sysrq_trigger v3: * Move ELF_CORE_COPY_REGS to asm/elf.h instead of uapi/asm/elf.h * Set stvec when disabling MMU * Minor cleanups and re-base v2: * Properly populate the ioresources tree, so that it can be used later on for implementing strict /dev/mem. * Minor cleanups and re-base Signed-off-by: Nick Kossifidis --- arch/riscv/include/asm/elf.h| 6 +++ arch/riscv/include/asm/kexec.h | 19 --- arch/riscv/kernel/Makefile | 2 +- arch/riscv/kernel/crash_save_regs.S | 56 + arch/riscv/kernel/kexec_relocate.S | 68 - arch/riscv/kernel/machine_kexec.c | 43 +--- arch/riscv/kernel/setup.c | 11 - arch/riscv/mm/init.c| 77 + 8 files changed, 255 insertions(+), 27 deletions(-) create mode 100644 arch/riscv/kernel/crash_save_regs.S diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h index 5c725e1df..f4b490cd0 100644 --- a/arch/riscv/include/asm/elf.h +++ b/arch/riscv/include/asm/elf.h @@ -81,4 +81,10 @@ extern int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp); #endif /* CONFIG_MMU */ +#define ELF_CORE_COPY_REGS(dest, regs) \ +do { \ + *(struct user_regs_struct *)&(dest) = \ + *(struct user_regs_struct *)regs; \ +} while (0); + #endif /* _ASM_RISCV_ELF_H */ diff --git a/arch/riscv/include/asm/kexec.h b/arch/riscv/include/asm/kexec.h index efc69feb4..4fd583acc 100644 --- a/arch/riscv/include/asm/kexec.h +++ b/arch/riscv/include/asm/kexec.h @@ -21,11 +21,16 @@ #define KEXEC_ARCH KEXEC_ARCH_RISCV +extern void riscv_crash_save_regs(struct pt_regs *newregs); + static inline void crash_setup_regs(struct pt_regs *newregs, struct pt_regs *oldregs) { - /* Dummy implementation for now */ + if (oldregs) + memcpy(newregs, oldregs, sizeof(struct pt_regs)); + else + riscv_crash_save_regs(newregs); } @@ -38,10 +43,12 @@ struct kimage_arch { const extern unsigned char riscv_kexec_relocate[]; const extern unsigned int riscv_kexec_relocate_size; -typedef void (*riscv_kexec_do_relocate)(unsigned long first_ind_entry, - unsigned long jump_addr, - unsigned long fdt_addr, - unsigned long hartid, - unsigned long va_pa_off); +typedef void (*riscv_kexec_method)(unsigned long first_ind_entry, + unsigned long jump_addr, + unsigned long fdt_addr, + unsigned long hartid, + unsigned long va_pa_off); + +extern riscv_kexec_method riscv_kexec_norelocate; #endif diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index c2594018c..07f676ad3 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -58,7 +58,7 @@ obj-$(CONFIG_SMP) += cpu_ops_sbi.o endif obj-$(CONFIG_HOTPLUG_CPU) += cpu-hotplug.o obj-$(CONFIG_KGDB) += kgdb.o -obj-${CONFIG_KEXEC}+= kexec_relocate.o machine_kexec.o +obj-${CONFIG_KEXEC}+= kexec_relocate.o crash_save_regs.o machine_kexec.o obj-$(CONFIG_JUMP_LABEL) += jump_label.o diff --git a/arch/riscv/kernel/crash_save_regs.S b/arch/riscv/kernel/crash_save_regs.S new file mode 100644 index 0..7832fb763 --- /dev/null +++ b/arch/riscv/kernel/crash_save_regs.S @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2020 FORTH-ICS/CARV + * Nick Kossifidis + */ + +#include/* For RISCV_* and REG_* macros */ +#include/* For CSR_* macros */ +#include/* For offsets on pt_regs */ +#include /* For SYM_* macros */ + +.section ".text" +SYM_CODE_START(riscv_crash_save_regs) + REG_S ra, PT_RA(a0)/* x1 */ + REG_S sp, PT_SP(a0)/* x2 */ + REG_S gp, PT_GP(a0)/* x3 */ + REG_S tp, PT_TP(a0)/* x4 */ + REG_S t0, PT_T0(a0)/* x5 */ + REG_S t1, PT_T1(a0)/* x6 */ + REG_S t2, PT_T2(a0)/* x7 */ + REG_S s0, PT_S0(a0)
[PATCH v3 0/5] RISC-V: Add kexec/kdump support
This patch series adds kexec/kdump and crash kernel support on RISC-V. For testing the patches a patched version of kexec-tools is needed (still a work in progress) which can be found at: https://riscv.ics.forth.gr/kexec-tools-patched.tar.xz v3: * Rebase on newer kernel tree * Minor cleanups * Split UAPI changes to a separate patch * Improve / cleanup init_resources * Resolve Palmer's comments v2: * Rebase on newer kernel tree * Minor cleanups * Properly populate the ioresources tre, so that it can be used later on for implementing strict /dev/mem * Use linux,usable-memory on /memory instead of a new binding * USe a reserved-memory node for ELF core header Nick Kossifidis (5): RISC-V: Add EM_RISCV to kexec UAPI header RISC-V: Add kexec support RISC-V: Improve init_resources RISC-V: Add kdump support RISC-V: Add crash kernel support arch/riscv/Kconfig | 25 arch/riscv/include/asm/elf.h| 6 + arch/riscv/include/asm/kexec.h | 54 +++ arch/riscv/kernel/Makefile | 6 + arch/riscv/kernel/crash_dump.c | 46 ++ arch/riscv/kernel/crash_save_regs.S | 56 +++ arch/riscv/kernel/kexec_relocate.S | 222 arch/riscv/kernel/machine_kexec.c | 193 arch/riscv/kernel/setup.c | 113 -- arch/riscv/mm/init.c| 110 ++ include/uapi/linux/kexec.h | 1 + 11 files changed, 787 insertions(+), 45 deletions(-) create mode 100644 arch/riscv/include/asm/kexec.h create mode 100644 arch/riscv/kernel/crash_dump.c create mode 100644 arch/riscv/kernel/crash_save_regs.S create mode 100644 arch/riscv/kernel/kexec_relocate.S create mode 100644 arch/riscv/kernel/machine_kexec.c -- 2.26.2
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(&isp1301_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] init: add support for zstd compressed modules
> On Apr 1, 2021, at 12:54 AM, torv...@mailbox.org wrote: > > Thanks Piotr, good work! > Question: Is `-T0` really faster in this particular case than the default > `-T1`? Are modules installed sequentially? The zstd CLI produces deterministic output regardless of the number of threads used. `-T1` (or not specifying `-T`) will produce the same output as `-T0`. `-T0` will be faster for large files (at the default level, multiple jobs will be spawned for files > 8MB), and be just as fast as `-T1` for smaller files. Best, Nick > I also saw that Masahiro did some work on modules_install, moving > MODULE_COMPRESS from the base Makefile to scripts/Makefile.modinst, so > perhaps this should also be moved there at a later point. > > Tor Vic
Re: [PATCH v2 2/3] kbuild: check the minimum assembler version in Kconfig
e as the clang > > version, and > > + # 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] init: add support for zstd compressed modules
> On Mar 31, 2021, at 10:48 AM, Oleksandr Natalenko > wrote: > > Hello. > > On Wed, Mar 31, 2021 at 05:39:25PM +, Nick Terrell wrote: >> >> >>> On Mar 30, 2021, at 4:50 AM, Oleksandr Natalenko >>> wrote: >>> >>> On Tue, Mar 30, 2021 at 01:32:35PM +0200, Piotr Gorski wrote: >>>> kmod 28 supports modules compressed in zstd format so let's add this >>>> possibility to kernel. >>>> >>>> Signed-off-by: Piotr Gorski >>>> --- >>>> Makefile | 7 +-- >>>> init/Kconfig | 9 ++--- >>>> 2 files changed, 11 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/Makefile b/Makefile >>>> index 5160ff8903c1..82f4f4cc2955 100644 >>>> --- a/Makefile >>>> +++ b/Makefile >>>> @@ -1156,8 +1156,8 @@ endif # INSTALL_MOD_STRIP >>>> export mod_strip_cmd >>>> >>>> # CONFIG_MODULE_COMPRESS, if defined, will cause module to be compressed >>>> -# after they are installed in agreement with CONFIG_MODULE_COMPRESS_GZIP >>>> -# or CONFIG_MODULE_COMPRESS_XZ. >>>> +# after they are installed in agreement with CONFIG_MODULE_COMPRESS_GZIP, >>>> +# CONFIG_MODULE_COMPRESS_XZ, or CONFIG_MODULE_COMPRESS_ZSTD. >>>> >>>> mod_compress_cmd = true >>>> ifdef CONFIG_MODULE_COMPRESS >>>> @@ -1167,6 +1167,9 @@ ifdef CONFIG_MODULE_COMPRESS >>>> ifdef CONFIG_MODULE_COMPRESS_XZ >>>>mod_compress_cmd = $(XZ) --lzma2=dict=2MiB -f >>>> endif # CONFIG_MODULE_COMPRESS_XZ >>>> + ifdef CONFIG_MODULE_COMPRESS_ZSTD >>>> +mod_compress_cmd = $(ZSTD) -T0 --rm -f -q >> >> This will use the default zstd level, level 3. I think it would make more >> sense to use a high >> compression level. Level 19 would probably be a good choice. That will >> choose a window >> size of up to 8MB, meaning the decompressor needs to allocate that much >> memory. If that >> is unacceptable, you could use `zstd -T0 --rm -f -q -19 --zstd=wlog=21`, >> which will use a >> window size of up to 2MB, to match the XZ command. Note that if the file is >> smaller than >> the window size, it will be shrunk to the smallest power of two at least as >> large as the file. > > Please no. We've already done that with initramfs in Arch, and it > increased the time to generate it enormously. > > I understand that building a kernel is a more rare operation than > regenerating initramfs, but still I'd go against hard-coding the level. > And if it should be specified anyway, I'd opt in for an explicit > configuration option. Remember, not all the kernel are built on > build farms... > > FWIW, Piotr originally used level 9 which worked okay, but I insisted > on sending the patch initially without specifying level at all like it is > done for other compressors. If this is a wrong approach, then oh meh, > mea culpa ;). > > Whatever default non-standard compression level you choose, I'm fine > as long as I can change it without editing Makefile. That makes sense to me. I have a deep seated need to compress files as efficiently as possible for widely distributed packages. But, I understand that slow compression significantly impacts build times for quick iteration. I’d be happy with a compression level parameter that defaults to a happy middle. I’m also fine with taking this patch as-is if it is easier, and I can put up another patch that adds a compression level parameter, since I don’t want to block merging this. Best, Nick Terrell > Thanks! > >> >> Best, >> Nick Terrell >> >>>> + endif # CONFIG_MODULE_COMPRESS_ZSTD >>>> endif # CONFIG_MODULE_COMPRESS >>>> export mod_compress_cmd >>>> >>>> diff --git a/init/Kconfig b/init/Kconfig >>>> index 8c2cfd88f6ef..86a452bc2747 100644 >>>> --- a/init/Kconfig >>>> +++ b/init/Kconfig >>>> @@ -2250,8 +2250,8 @@ config MODULE_COMPRESS >>>>bool "Compress modules on installation" >>>>help >>>> >>>> -Compresses kernel modules when 'make modules_install' is run; gzip or >>>> -xz depending on "Compression algorithm" below. >>>> +Compresses kernel modules when 'make modules_install' is run; gzip, >>>> +xz, or zstd depending on "Compression algorithm" below. >>>> >>>> module-init-tools MAY support gzip, and kmod MAY support gzip and
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
Re: [PATCH] init: add support for zstd compressed modules
> On Mar 30, 2021, at 4:50 AM, Oleksandr Natalenko > wrote: > > On Tue, Mar 30, 2021 at 01:32:35PM +0200, Piotr Gorski wrote: >> kmod 28 supports modules compressed in zstd format so let's add this >> possibility to kernel. >> >> Signed-off-by: Piotr Gorski >> --- >> Makefile | 7 +-- >> init/Kconfig | 9 ++--- >> 2 files changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/Makefile b/Makefile >> index 5160ff8903c1..82f4f4cc2955 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -1156,8 +1156,8 @@ endif # INSTALL_MOD_STRIP >> export mod_strip_cmd >> >> # CONFIG_MODULE_COMPRESS, if defined, will cause module to be compressed >> -# after they are installed in agreement with CONFIG_MODULE_COMPRESS_GZIP >> -# or CONFIG_MODULE_COMPRESS_XZ. >> +# after they are installed in agreement with CONFIG_MODULE_COMPRESS_GZIP, >> +# CONFIG_MODULE_COMPRESS_XZ, or CONFIG_MODULE_COMPRESS_ZSTD. >> >> mod_compress_cmd = true >> ifdef CONFIG_MODULE_COMPRESS >> @@ -1167,6 +1167,9 @@ ifdef CONFIG_MODULE_COMPRESS >> ifdef CONFIG_MODULE_COMPRESS_XZ >> mod_compress_cmd = $(XZ) --lzma2=dict=2MiB -f >> endif # CONFIG_MODULE_COMPRESS_XZ >> + ifdef CONFIG_MODULE_COMPRESS_ZSTD >> +mod_compress_cmd = $(ZSTD) -T0 --rm -f -q This will use the default zstd level, level 3. I think it would make more sense to use a high compression level. Level 19 would probably be a good choice. That will choose a window size of up to 8MB, meaning the decompressor needs to allocate that much memory. If that is unacceptable, you could use `zstd -T0 --rm -f -q -19 --zstd=wlog=21`, which will use a window size of up to 2MB, to match the XZ command. Note that if the file is smaller than the window size, it will be shrunk to the smallest power of two at least as large as the file. Best, Nick Terrell >> + endif # CONFIG_MODULE_COMPRESS_ZSTD >> endif # CONFIG_MODULE_COMPRESS >> export mod_compress_cmd >> >> diff --git a/init/Kconfig b/init/Kconfig >> index 8c2cfd88f6ef..86a452bc2747 100644 >> --- a/init/Kconfig >> +++ b/init/Kconfig >> @@ -2250,8 +2250,8 @@ config MODULE_COMPRESS >> bool "Compress modules on installation" >> help >> >> - Compresses kernel modules when 'make modules_install' is run; gzip or >> - xz depending on "Compression algorithm" below. >> + Compresses kernel modules when 'make modules_install' is run; gzip, >> + xz, or zstd depending on "Compression algorithm" below. >> >>module-init-tools MAY support gzip, and kmod MAY support gzip and xz. >> >> @@ -2273,7 +2273,7 @@ choice >>This determines which sort of compression will be used during >>'make modules_install'. >> >> - GZIP (default) and XZ are supported. >> + GZIP (default), XZ, and ZSTD are supported. >> >> config MODULE_COMPRESS_GZIP >> bool "GZIP" >> @@ -2281,6 +2281,9 @@ config MODULE_COMPRESS_GZIP >> config MODULE_COMPRESS_XZ >> bool "XZ" >> >> +config MODULE_COMPRESS_ZSTD >> +bool "ZSTD" >> + >> endchoice >> >> config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS >> -- >> 2.31.0.97.g1424303384 >> > > Great! > > Reviewed-by: Oleksandr Natalenko > > This works perfectly fine in Arch Linux if accompanied by the > following mkinitcpio amendment: [1]. > > I'm also Cc'ing other people from get_maintainers output just > to make this submission more visible. > > Thanks. > > [1] https://github.com/archlinux/mkinitcpio/pull/43 > > -- > Oleksandr Natalenko (post-factum)
[PATCH v9 2/3] lib: zstd: Add decompress_sources.h for decompress_unzstd
From: Nick Terrell Adds decompress_sources.h which includes every .c file necessary for zstd decompression. This is used in decompress_unzstd.c so the internal structure of the library isn't exposed. This allows us to upgrade the zstd library version without modifying any callers. Instead we just need to update decompress_sources.h. Signed-off-by: Nick Terrell --- lib/decompress_unzstd.c | 6 +- lib/zstd/decompress_sources.h | 23 +++ 2 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 lib/zstd/decompress_sources.h diff --git a/lib/decompress_unzstd.c b/lib/decompress_unzstd.c index c88aad49e996..6e5ecfba0a8d 100644 --- a/lib/decompress_unzstd.c +++ b/lib/decompress_unzstd.c @@ -68,11 +68,7 @@ #ifdef STATIC # define UNZSTD_PREBOOT # include "xxhash.c" -# include "zstd/entropy_common.c" -# include "zstd/fse_decompress.c" -# include "zstd/huf_decompress.c" -# include "zstd/zstd_common.c" -# include "zstd/decompress.c" +# include "zstd/decompress_sources.h" #endif #include diff --git a/lib/zstd/decompress_sources.h b/lib/zstd/decompress_sources.h new file mode 100644 index ..d82cea4316f5 --- /dev/null +++ b/lib/zstd/decompress_sources.h @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under both the BSD-style license (found in the + * LICENSE file in the root directory of this source tree) and the GPLv2 (found + * in the COPYING file in the root directory of this source tree). + * You may select, at your option, one of the above-listed licenses. + */ + +/* + * This file includes every .c file needed for decompression. + * It is used by lib/decompress_unzstd.c to include the decompression + * source into the translation-unit, so it can be used for kernel + * decompression. + */ + +#include "entropy_common.c" +#include "fse_decompress.c" +#include "huf_decompress.c" +#include "zstd_common.c" +#include "decompress.c" -- 2.31.0
[PATCH v9 1/3] lib: zstd: Add kernel-specific API
From: Nick Terrell This patch: - Moves `include/linux/zstd.h` -> `include/linux/zstd_lib.h` - Updates modified zstd headers to yearless copyright - Adds a new API in `include/linux/zstd.h` that is functionally equivalent to the in-use subset of the current API. Functions are renamed to avoid symbol collisions with zstd, to make it clear it is not the upstream zstd API, and to follow the kernel style guide. - Updates all callers to use the new API. There are no functional changes in this patch. Since there are no functional change, I felt it was okay to update all the callers in a single patch. Once the API is approved, the callers are mechanically changed. This patch is preparing for the 3rd patch in this series, which updates zstd to version 1.4.10. Since the upstream zstd API is no longer exposed to callers, the update can happen transparently. Signed-off-by: Nick Terrell --- crypto/zstd.c | 28 +- fs/btrfs/zstd.c| 68 +- fs/f2fs/compress.c | 56 +- fs/f2fs/super.c|2 +- fs/pstore/platform.c |2 +- fs/squashfs/zstd_wrapper.c | 16 +- include/linux/zstd.h | 1243 include/linux/zstd_lib.h | 1157 + lib/decompress_unzstd.c| 42 +- lib/zstd/compress.c| 123 ++-- lib/zstd/decompress.c | 112 ++-- 11 files changed, 1691 insertions(+), 1158 deletions(-) create mode 100644 include/linux/zstd_lib.h diff --git a/crypto/zstd.c b/crypto/zstd.c index 1a3309f066f7..154a969c83a8 100644 --- a/crypto/zstd.c +++ b/crypto/zstd.c @@ -18,22 +18,22 @@ #define ZSTD_DEF_LEVEL 3 struct zstd_ctx { - ZSTD_CCtx *cctx; - ZSTD_DCtx *dctx; + zstd_cctx *cctx; + zstd_dctx *dctx; void *cwksp; void *dwksp; }; -static ZSTD_parameters zstd_params(void) +static zstd_parameters zstd_params(void) { - return ZSTD_getParams(ZSTD_DEF_LEVEL, 0, 0); + return zstd_get_params(ZSTD_DEF_LEVEL, 0); } static int zstd_comp_init(struct zstd_ctx *ctx) { int ret = 0; - const ZSTD_parameters params = zstd_params(); - const size_t wksp_size = ZSTD_CCtxWorkspaceBound(params.cParams); + const zstd_parameters params = zstd_params(); + const size_t wksp_size = zstd_cctx_workspace_bound(¶ms.cParams); ctx->cwksp = vzalloc(wksp_size); if (!ctx->cwksp) { @@ -41,7 +41,7 @@ static int zstd_comp_init(struct zstd_ctx *ctx) goto out; } - ctx->cctx = ZSTD_initCCtx(ctx->cwksp, wksp_size); + ctx->cctx = zstd_init_cctx(ctx->cwksp, wksp_size); if (!ctx->cctx) { ret = -EINVAL; goto out_free; @@ -56,7 +56,7 @@ static int zstd_comp_init(struct zstd_ctx *ctx) static int zstd_decomp_init(struct zstd_ctx *ctx) { int ret = 0; - const size_t wksp_size = ZSTD_DCtxWorkspaceBound(); + const size_t wksp_size = zstd_dctx_workspace_bound(); ctx->dwksp = vzalloc(wksp_size); if (!ctx->dwksp) { @@ -64,7 +64,7 @@ static int zstd_decomp_init(struct zstd_ctx *ctx) goto out; } - ctx->dctx = ZSTD_initDCtx(ctx->dwksp, wksp_size); + ctx->dctx = zstd_init_dctx(ctx->dwksp, wksp_size); if (!ctx->dctx) { ret = -EINVAL; goto out_free; @@ -152,10 +152,10 @@ static int __zstd_compress(const u8 *src, unsigned int slen, { size_t out_len; struct zstd_ctx *zctx = ctx; - const ZSTD_parameters params = zstd_params(); + const zstd_parameters params = zstd_params(); - out_len = ZSTD_compressCCtx(zctx->cctx, dst, *dlen, src, slen, params); - if (ZSTD_isError(out_len)) + out_len = zstd_compress_cctx(zctx->cctx, dst, *dlen, src, slen, ¶ms); + if (zstd_is_error(out_len)) return -EINVAL; *dlen = out_len; return 0; @@ -182,8 +182,8 @@ static int __zstd_decompress(const u8 *src, unsigned int slen, size_t out_len; struct zstd_ctx *zctx = ctx; - out_len = ZSTD_decompressDCtx(zctx->dctx, dst, *dlen, src, slen); - if (ZSTD_isError(out_len)) + out_len = zstd_decompress_dctx(zctx->dctx, dst, *dlen, src, slen); + if (zstd_is_error(out_len)) return -EINVAL; *dlen = out_len; return 0; diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c index 8e9626d63976..14418b02c189 100644 --- a/fs/btrfs/zstd.c +++ b/fs/btrfs/zstd.c @@ -28,10 +28,10 @@ /* 307s to avoid pathologically clashing with transaction commit */ #define ZSTD_BTRFS_RECLAIM_JIFFIES (307 * HZ) -static ZSTD_parameters zstd_get_btrfs_parameters(unsigned int level, +static zstd_parameters zstd_get_btrfs_parameters(unsigned int level, size_t src_len) { - ZSTD_parameters params = ZSTD_getParams(level, src_len, 0); +
[GIT PULL][PATCH v9 0/3] Update to zstd-1.4.10
From: Nick Terrell Please pull from g...@github.com:terrelln/linux.git tags/v9-zstd-1.4.10 to get these changes. Alternatively the patchset is included. This patchset upgrades the zstd library to the latest upstream release. The current zstd version in the kernel is a modified version of upstream zstd-1.3.1. At the time it was integrated, zstd wasn't ready to be used in the kernel as-is. But, it is now possible to use upstream zstd directly in the kernel. I have not yet released zstd-1.4.10 upstream. I want the zstd version in the kernel to match up with a known upstream release, so we know exactly what code is running. Whenever this patchset is ready for merge, I will cut a release at the upstream commit that gets merged. This should not be necessary for future releases. The kernel zstd library is automatically generated from upstream zstd. A script makes the necessary changes and imports it into the kernel. The changes are: 1. Replace all libc dependencies with kernel replacements and rewrite includes. 2. Remove unncessary portability macros like: #if defined(_MSC_VER). 3. Use the kernel xxhash instead of bundling it. This automation gets tested every commit by upstream's continuous integration. When we cut a new zstd release, we will submit a patch to the kernel to update the zstd version in the kernel. I've updated zstd to upstream with one big patch because every commit must build, so that precludes partial updates. Since the commit is 100% generated, I hope the review burden is lightened. I considered replaying upstream commits, but that is not possible because there have been ~3500 upstream commits since the last zstd import, and the commits don't all build individually. The bulk update preserves bisectablity because bugs can be bisected to the zstd version update. At that point the update can be reverted, and we can work with upstream to find and fix the bug. After this big switch in how the kernel consumes zstd, future patches will be smaller, because they will only have one upstream release worth of changes each. This patchset adds a new kernel-style wrapper around zstd. This wrapper API is functionally equivalent to the subset of the current zstd API that is currently used. The wrapper API changes to be kernel style so that the symbols don't collide with zstd's symbols. The update to zstd-1.4.6 maintains the same API and preserves the semantics, so that none of the callers need to be updated. This patchset comes in 2 parts: 1. The first 2 patches prepare for the zstd upgrade. The first patch adds the new kernel style API so zstd can be upgraded without modifying any callers. The second patch adds an indirection for the lib/decompress_unzstd.c including of all decompression source files. 2. Import zstd-1.4.10. This patch is completely generated from upstream using automated tooling. I tested every caller of zstd on x86_64. I tested both after the 1.4.10 upgrade using the compatibility wrapper, and after the final patch in this series. I tested kernel and initramfs decompression in i386 and arm. I ran benchmarks to compare the current zstd in the kernel with zstd-1.4.6. I benchmarked on x86_64 using QEMU with KVM enabled on an Intel i9-9900k. I found: * BtrFS zstd compression at levels 1 and 3 is 5% faster * BtrFS zstd decompression+read is 15% faster * SquashFS zstd decompression+read is 15% faster * F2FS zstd compression+write at level 3 is 8% faster * F2FS zstd decompression+read is 20% faster * ZRAM decompression+read is 30% faster * Kernel zstd decompression is 35% faster * Initramfs zstd decompression+build is 5% faster The latest zstd also offers bug fixes. For example the problem with large kernel decompression has been fixed upstream for over 2 years https://lkml.org/lkml/2020/9/29/27. Please let me know if there is anything that I can do to ease the way for these patches. I think it is important because it gets large performance improvements, contains bug fixes, and is switching to a more maintainable model of consuming upstream zstd directly, making it easy to keep up to date. Best, Nick Terrell v1 -> v2: * Successfully tested F2FS with help from Chao Yu to fix my test. * (1/9) Fix ZSTD_initCStream() wrapper to handle pledged_src_size=0 means unknown. This fixes F2FS with the zstd-1.4.6 compatibility wrapper, exposed by the test. v2 -> v3: * (3/9) Silence warnings by Kernel Test Robot: https://github.com/facebook/zstd/pull/2324 Stack size warnings remain, but these aren't new, and the functions it warns on are either unused or not in the maximum stack path. This patchset reduces zstd compression stack usage by 1 KB overall. I've gotten the low hanging fruit, and more stack reduction would require significant changes that have the potential to introduce new bugs. However, I do hope to continue to reduce zstd stack usage in future versions. v3 -> v4: * (3/9) Fix errors an
Re: [PATCH v3 11/17] riscv: Convert to GENERIC_CMDLINE
Στις 2021-03-26 17:26, Rob Herring έγραψε: On Fri, Mar 26, 2021 at 8:20 AM Christophe Leroy wrote: Le 26/03/2021 à 15:08, Andreas Schwab a écrit : > On Mär 26 2021, Christophe Leroy wrote: > >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >> index f8f15332caa2..e7c91ee478d1 100644 >> --- a/arch/riscv/kernel/setup.c >> +++ b/arch/riscv/kernel/setup.c >> @@ -20,6 +20,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> #include >> @@ -228,10 +229,8 @@ static void __init parse_dtb(void) >> } >> >> pr_err("No DTB passed to the kernel\n"); >> -#ifdef CONFIG_CMDLINE_FORCE >> -strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE); >> +cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE); >> pr_info("Forcing kernel command line to: %s\n", boot_command_line); > > Shouldn't that message become conditional in some way? > You are right, I did something similar on ARM but looks like I missed it on RISCV. How is this hunk even useful? Under what conditions can you boot without a DTB? Even with a built-in DTB, the DT cmdline handling would be called. Rob cced Paul who introduced this: https://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git/commit/arch/riscv/kernel/setup.c?id=8fd6e05c7463b635e51ec7df0a1858c1b5a6e350
[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: [PATCH v8 1/3] lib: zstd: Add kernel-specific API
On Sat, Mar 27, 2021 at 2:48 PM Oleksandr Natalenko wrote: > > Hello. > > On Sat, Mar 27, 2021 at 05:48:01PM +0800, kernel test robot wrote: > > >> ERROR: modpost: "ZSTD_maxCLevel" [fs/f2fs/f2fs.ko] undefined! > > Since f2fs can be built as a module, the following correction seems to > be needed: Thanks Oleksandr! Looks like f2fs has been updated to use ZSTD_maxCLevel() since the first version of these patches. I'll put up a new version shortly with the fix, and update my test suite to build f2fs and other users as modules, so it can catch this. Best, Nick > ``` > diff --git a/lib/zstd/compress/zstd_compress.c > b/lib/zstd/compress/zstd_compress.c > index 9c998052a0e5..584c92c51169 100644 > --- a/lib/zstd/compress/zstd_compress.c > +++ b/lib/zstd/compress/zstd_compress.c > @@ -4860,6 +4860,7 @@ size_t ZSTD_endStream(ZSTD_CStream* zcs, > ZSTD_outBuffer* output) > > #define ZSTD_MAX_CLEVEL 22 > int ZSTD_maxCLevel(void) { return ZSTD_MAX_CLEVEL; } > +EXPORT_SYMBOL(ZSTD_maxCLevel); > int ZSTD_minCLevel(void) { return (int)-ZSTD_TARGETLENGTH_MAX; } > > static const ZSTD_compressionParameters > ZSTD_defaultCParameters[4][ZSTD_MAX_CLEVEL+1] = { > ``` > > Not sure if the same should be done for `ZSTD_minCLevel()` since I don't > see it being used anywhere else. > > -- > Oleksandr Natalenko (post-factum)
Re: [PATCH v8 3/3] lib: zstd: Upgrade to latest upstream zstd version 1.4.10
On Fri, Mar 26, 2021 at 3:02 PM kernel test robot wrote: > > Hi Nick, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on cryptodev/master] > [also build test WARNING on kdave/for-next f2fs/dev-test linus/master > v5.12-rc4 next-20210326] > [cannot apply to crypto/master kees/for-next/pstore squashfs/master] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: > https://github.com/0day-ci/linux/commits/Nick-Terrell/Update-to-zstd-1-4-10/20210327-031827 > base: > https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git > master > config: um-allmodconfig (attached as .config) > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 > reproduce (this is a W=1 build): > # > https://github.com/0day-ci/linux/commit/ebbff13fa6a537fb8b3dc6b42c3093f9ce4358f8 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review > Nick-Terrell/Update-to-zstd-1-4-10/20210327-031827 > git checkout ebbff13fa6a537fb8b3dc6b42c3093f9ce4358f8 > # save the attached .config to linux build tree > make W=1 ARCH=um > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All warnings (new ones prefixed by >>): > >lib/zstd/compress/zstd_compress_sequences.c:17: warning: Cannot understand > * -log2(x / 256) lookup table for x in [0, 256). > on line 17 - I thought it was a doc line >lib/zstd/compress/zstd_compress_sequences.c:58: warning: Function > parameter or member 'nbSeq' not described in 'ZSTD_useLowProbCount' > >> lib/zstd/compress/zstd_compress_sequences.c:58: warning: expecting > >> prototype for 1 else we should(). Prototype was for ZSTD_useLowProbCount() > >> instead > >> lib/zstd/compress/zstd_compress_sequences.c:67: warning: wrong kernel-doc > >> identifier on line: > * Returns the cost in bytes of encoding the normalized count header. >lib/zstd/compress/zstd_compress_sequences.c:85: warning: Function > parameter or member 'count' not described in 'ZSTD_entropyCost' >lib/zstd/compress/zstd_compress_sequences.c:85: warning: Function > parameter or member 'max' not described in 'ZSTD_entropyCost' >lib/zstd/compress/zstd_compress_sequences.c:85: warning: Function > parameter or member 'total' not described in 'ZSTD_entropyCost' > >> lib/zstd/compress/zstd_compress_sequences.c:85: warning: expecting > >> prototype for Returns the cost in bits of encoding the distribution > >> described by count(). Prototype was for ZSTD_entropyCost() instead >lib/zstd/compress/zstd_compress_sequences.c:99: warning: wrong kernel-doc > identifier on line: > * Returns the cost in bits of encoding the distribution in count using > ctable. >lib/zstd/compress/zstd_compress_sequences.c:139: warning: Function > parameter or member 'norm' not described in 'ZSTD_crossEntropyCost' >lib/zstd/compress/zstd_compress_sequences.c:139: warning: Function > parameter or member 'accuracyLog' not described in 'ZSTD_crossEntropyCost' >lib/zstd/compress/zstd_compress_sequences.c:139: warning: Function > parameter or member 'count' not described in 'ZSTD_crossEntropyCost' >lib/zstd/compress/zstd_compress_sequences.c:139: warning: Function > parameter or member 'max' not described in 'ZSTD_crossEntropyCost' > >> lib/zstd/compress/zstd_compress_sequences.c:139: warning: expecting > >> prototype for Returns the cost in bits of encoding the distribution in > >> count using the(). Prototype was for ZSTD_crossEntropyCost() instead > -- >lib/zstd/compress/zstd_ldm.c:584: warning: Function parameter or member > 'rawSeqStore' not described in 'maybeSplitSequence' >lib/zstd/compress/zstd_ldm.c:584: warning: Function parameter or member > 'remaining' not described in 'maybeSplitSequence' >lib/zstd/compress/zstd_ldm.c:584: warning: Function parameter or member > 'minMatch' not described in 'maybeSplitSequence' > >> lib/zstd/compress/zstd_ldm.c:584: warning: expecting prototype for If the > >> sequence length is longer than remaining then the sequence is split(). > >> Prototype was for maybeSplitSequence() instead > -- > >> lib/zstd/decompress/zstd_decompress.c:992: warning: wrong kernel-doc > >> id
[PATCH v8 2/3] lib: zstd: Add decompress_sources.h for decompress_unzstd
From: Nick Terrell Adds decompress_sources.h which includes every .c file necessary for zstd decompression. This is used in decompress_unzstd.c so the internal structure of the library isn't exposed. This allows us to upgrade the zstd library version without modifying any callers. Instead we just need to update decompress_sources.h. Signed-off-by: Nick Terrell --- lib/decompress_unzstd.c | 6 +- lib/zstd/decompress_sources.h | 14 ++ 2 files changed, 15 insertions(+), 5 deletions(-) create mode 100644 lib/zstd/decompress_sources.h diff --git a/lib/decompress_unzstd.c b/lib/decompress_unzstd.c index dab2d55cf08d..e6897a5063a7 100644 --- a/lib/decompress_unzstd.c +++ b/lib/decompress_unzstd.c @@ -68,11 +68,7 @@ #ifdef STATIC # define UNZSTD_PREBOOT # include "xxhash.c" -# include "zstd/entropy_common.c" -# include "zstd/fse_decompress.c" -# include "zstd/huf_decompress.c" -# include "zstd/zstd_common.c" -# include "zstd/decompress.c" +# include "zstd/decompress_sources.h" #endif #include diff --git a/lib/zstd/decompress_sources.h b/lib/zstd/decompress_sources.h new file mode 100644 index ..d2fe10af0043 --- /dev/null +++ b/lib/zstd/decompress_sources.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +/* + * This file includes every .c file needed for decompression. + * It is used by lib/decompress_unzstd.c to include the decompression + * source into the translation-unit, so it can be used for kernel + * decompression. + */ + +#include "entropy_common.c" +#include "fse_decompress.c" +#include "huf_decompress.c" +#include "zstd_common.c" +#include "decompress.c" -- 2.31.0
[PATCH v8 0/3] Update to zstd-1.4.10
From: Nick Terrell Please pull from g...@github.com:terrelln/linux.git tags/v8-zstd-1.4.10 to get these changes. Alternatively the patchset is included. This patchset upgrades the zstd library to the latest upstream release. The current zstd version in the kernel is a modified version of upstream zstd-1.3.1. At the time it was integrated, zstd wasn't ready to be used in the kernel as-is. But, it is now possible to use upstream zstd directly in the kernel. I have not yet released zstd-1.4.10 upstream. I want the zstd version in the kernel to match up with a known upstream release, so we know exactly what code is running. Whenever this patchset is ready for merge, I will cut a release at the upstream commit that gets merged. This should not be necessary for future releases. The kernel zstd library is automatically generated from upstream zstd. A script makes the necessary changes and imports it into the kernel. The changes are: 1. Replace all libc dependencies with kernel replacements and rewrite includes. 2. Remove unncessary portability macros like: #if defined(_MSC_VER). 3. Use the kernel xxhash instead of bundling it. This automation gets tested every commit by upstream's continuous integration. When we cut a new zstd release, we will submit a patch to the kernel to update the zstd version in the kernel. I've updated zstd to upstream with one big patch because every commit must build, so that precludes partial updates. Since the commit is 100% generated, I hope the review burden is lightened. I considered replaying upstream commits, but that is not possible because there have been ~3500 upstream commits since the last zstd import, and the commits don't all build individually. The bulk update preserves bisectablity because bugs can be bisected to the zstd version update. At that point the update can be reverted, and we can work with upstream to find and fix the bug. After this big switch in how the kernel consumes zstd, future patches will be smaller, because they will only have one upstream release worth of changes each. This patchset adds a new kernel-style wrapper around zstd. This wrapper API is functionally equivalent to the subset of the current zstd API that is currently used. The wrapper API changes to be kernel style so that the symbols don't collide with zstd's symbols. The update to zstd-1.4.10 maintains the same API and preserves the semantics, so that none of the callers need to be updated. This patchset comes in 2 parts: 1. The first 2 patches prepare for the zstd upgrade. The first patch adds the new kernel style API so zstd can be upgraded without modifying any callers. The second patch adds an indirection for the lib/decompress_unzstd.c including of all decompression source files. 2. Import zstd-1.4.10. This patch is completely generated from upstream using automated tooling. I tested every caller of zstd on x86_64. I tested both after the 1.4.10 upgrade using the compatibility wrapper, and after the final patch in this series. I tested kernel and initramfs decompression in i386 and arm. I ran benchmarks to compare the current zstd in the kernel with zstd-1.4.10. I benchmarked on x86_64 using QEMU with KVM enabled on an Intel i9-9900k. I found: * BtrFS zstd compression at levels 1 and 3 is 5% faster * BtrFS zstd decompression+read is 15% faster * SquashFS zstd decompression+read is 15% faster * F2FS zstd compression+write at level 3 is 8% faster * F2FS zstd decompression+read is 20% faster * ZRAM decompression+read is 30% faster * Kernel zstd decompression is 35% faster * Initramfs zstd decompression+build is 5% faster The latest zstd also offers bug fixes. For example the problem with large kernel decompression has been fixed upstream for over 2 years https://lkml.org/lkml/2020/9/29/27. Please let me know if there is anything that I can do to ease the way for these patches. I think it is important because it gets large performance improvements, contains bug fixes, and is switching to a more maintainable model of consuming upstream zstd directly, making it easy to keep up to date. Best, Nick Terrell v1 -> v2: * Successfully tested F2FS with help from Chao Yu to fix my test. * (1/9) Fix ZSTD_initCStream() wrapper to handle pledged_src_size=0 means unknown. This fixes F2FS with the zstd-1.4.6 compatibility wrapper, exposed by the test. v2 -> v3: * (3/9) Silence warnings by Kernel Test Robot: https://github.com/facebook/zstd/pull/2324 Stack size warnings remain, but these aren't new, and the functions it warns on are either unused or not in the maximum stack path. This patchset reduces zstd compression stack usage by 1 KB overall. I've gotten the low hanging fruit, and more stack reduction would require significant changes that have the potential to introduce new bugs. However, I do hope to continue to reduce zstd stack usage in future versions. v3 -> v4: * (3/9) Fix errors an
[PATCH v8 1/3] lib: zstd: Add kernel-specific API
From: Nick Terrell This patch: - Moves `include/linux/zstd.h` -> `include/linux/zstd_lib.h` - Adds a new API in `include/linux/zstd.h` that is functionally equivalent to the in-use subset of the current API. Functions are renamed to avoid symbol collisions with zstd, to make it clear it is not the upstream zstd API, and to follow the kernel style guide. - Updates all callers to use the new API. There are no functional changes in this patch. Since there are no functional change, I felt it was okay to update all the callers in a single patch. Once the API is approved, the callers are mechanically changed. This patch is preparing for the 3rd patch in this series, which updates zstd to version 1.4.10. Since the upstream zstd API is no longer exposed to callers, the update can happen transparently. Signed-off-by: Nick Terrell --- crypto/zstd.c | 28 +- fs/btrfs/zstd.c| 68 +- fs/f2fs/compress.c | 56 +- fs/pstore/platform.c |2 +- fs/squashfs/zstd_wrapper.c | 16 +- include/linux/zstd.h | 1218 include/linux/zstd_lib.h | 1157 ++ lib/decompress_unzstd.c| 42 +- lib/zstd/compress.c| 107 ++-- lib/zstd/decompress.c | 112 ++-- 10 files changed, 1657 insertions(+), 1149 deletions(-) create mode 100644 include/linux/zstd_lib.h diff --git a/crypto/zstd.c b/crypto/zstd.c index 1a3309f066f7..154a969c83a8 100644 --- a/crypto/zstd.c +++ b/crypto/zstd.c @@ -18,22 +18,22 @@ #define ZSTD_DEF_LEVEL 3 struct zstd_ctx { - ZSTD_CCtx *cctx; - ZSTD_DCtx *dctx; + zstd_cctx *cctx; + zstd_dctx *dctx; void *cwksp; void *dwksp; }; -static ZSTD_parameters zstd_params(void) +static zstd_parameters zstd_params(void) { - return ZSTD_getParams(ZSTD_DEF_LEVEL, 0, 0); + return zstd_get_params(ZSTD_DEF_LEVEL, 0); } static int zstd_comp_init(struct zstd_ctx *ctx) { int ret = 0; - const ZSTD_parameters params = zstd_params(); - const size_t wksp_size = ZSTD_CCtxWorkspaceBound(params.cParams); + const zstd_parameters params = zstd_params(); + const size_t wksp_size = zstd_cctx_workspace_bound(¶ms.cParams); ctx->cwksp = vzalloc(wksp_size); if (!ctx->cwksp) { @@ -41,7 +41,7 @@ static int zstd_comp_init(struct zstd_ctx *ctx) goto out; } - ctx->cctx = ZSTD_initCCtx(ctx->cwksp, wksp_size); + ctx->cctx = zstd_init_cctx(ctx->cwksp, wksp_size); if (!ctx->cctx) { ret = -EINVAL; goto out_free; @@ -56,7 +56,7 @@ static int zstd_comp_init(struct zstd_ctx *ctx) static int zstd_decomp_init(struct zstd_ctx *ctx) { int ret = 0; - const size_t wksp_size = ZSTD_DCtxWorkspaceBound(); + const size_t wksp_size = zstd_dctx_workspace_bound(); ctx->dwksp = vzalloc(wksp_size); if (!ctx->dwksp) { @@ -64,7 +64,7 @@ static int zstd_decomp_init(struct zstd_ctx *ctx) goto out; } - ctx->dctx = ZSTD_initDCtx(ctx->dwksp, wksp_size); + ctx->dctx = zstd_init_dctx(ctx->dwksp, wksp_size); if (!ctx->dctx) { ret = -EINVAL; goto out_free; @@ -152,10 +152,10 @@ static int __zstd_compress(const u8 *src, unsigned int slen, { size_t out_len; struct zstd_ctx *zctx = ctx; - const ZSTD_parameters params = zstd_params(); + const zstd_parameters params = zstd_params(); - out_len = ZSTD_compressCCtx(zctx->cctx, dst, *dlen, src, slen, params); - if (ZSTD_isError(out_len)) + out_len = zstd_compress_cctx(zctx->cctx, dst, *dlen, src, slen, ¶ms); + if (zstd_is_error(out_len)) return -EINVAL; *dlen = out_len; return 0; @@ -182,8 +182,8 @@ static int __zstd_decompress(const u8 *src, unsigned int slen, size_t out_len; struct zstd_ctx *zctx = ctx; - out_len = ZSTD_decompressDCtx(zctx->dctx, dst, *dlen, src, slen); - if (ZSTD_isError(out_len)) + out_len = zstd_decompress_dctx(zctx->dctx, dst, *dlen, src, slen); + if (zstd_is_error(out_len)) return -EINVAL; *dlen = out_len; return 0; diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c index 8e9626d63976..14418b02c189 100644 --- a/fs/btrfs/zstd.c +++ b/fs/btrfs/zstd.c @@ -28,10 +28,10 @@ /* 307s to avoid pathologically clashing with transaction commit */ #define ZSTD_BTRFS_RECLAIM_JIFFIES (307 * HZ) -static ZSTD_parameters zstd_get_btrfs_parameters(unsigned int level, +static zstd_parameters zstd_get_btrfs_parameters(unsigned int level, size_t src_len) { - ZSTD_parameters params = ZSTD_getParams(level, src_len, 0); + zstd_parameters params = zstd_get_params(level, src_len);
High-quality Original IELTS certificate
Greetings, We are the leading provider of IELTS CERTIFICATE ONLINE. You don’t have to worry if you can’t pass in the IELTS exam, for we are here to issue you this certificate without constrain. We make it possible for you to get the IELTS certificate even without appearing for the exam or even if you have failed to pass in the exam with us you can obtain a genuine IELTS certificate. All our IELTS certificate for sale are verified. Don’t believe us? Contact us and see for yourself. You will receive a test report card once we issue your certificate, in which there will be all your candidate details. If you want to buy registered IELTS certificate online, we are the first name that comes to mind. We let you buy real British Council certificates. All our certificates are registered at the British Council database system and we make sure that you can verify your IELTS certificate online on the official website of the British Council or IDP. Isn’t it what you were looking for? Contact us: ujam1...@gmail.com
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 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 -- 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