Re: BUG: sleeping function called from invalid context at include/linux/sched/mm.h:256
Thank Matthew for correcting me On Sun, Feb 13, 2022 at 12:09 PM Matthew Wilcox wrote: > > On Sun, Feb 13, 2022 at 11:19:09AM +0800, Zhouyi Zhou wrote: > > I think the key to the problem lies in your attached console.log > > (pasted below), at times 0.014987 and 0.015995, I see there are two > > locks (cpu_hotplug_lock and jump_label_mutex) holded while > > kmem_cache_alloc calls __might_resched (0.023356). > > Those are both sleeping locks (a percpu_rwsem and mutex, respectively). > There is no problem with sleeping while holding a mutex or rwsem. >From console.log, I see [0.012154][T1] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:256 [0.013128][T1] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0 >From ___might_sleep, I see 9506 if ((preempt_count_equals(preempt_offset) && !irqs_disabled() && 9507 !is_idle_task(current) && !current->non_block_count) || 9508 system_state == SYSTEM_BOOTING || system_state > SYSTEM_RUNNING || 9509 oops_in_progress) 9510 return; I guess it is irq_disable which cause the bug. Thanks Zhouyi
Re: BUG: sleeping function called from invalid context at include/linux/sched/mm.h:256
On Sun, Feb 13, 2022 at 11:19:09AM +0800, Zhouyi Zhou wrote: > I think the key to the problem lies in your attached console.log > (pasted below), at times 0.014987 and 0.015995, I see there are two > locks (cpu_hotplug_lock and jump_label_mutex) holded while > kmem_cache_alloc calls __might_resched (0.023356). Those are both sleeping locks (a percpu_rwsem and mutex, respectively). There is no problem with sleeping while holding a mutex or rwsem.
Re: BUG: sleeping function called from invalid context at include/linux/sched/mm.h:256
Dear Paul I think the key to the problem lies in your attached console.log (pasted below), at times 0.014987 and 0.015995, I see there are two locks (cpu_hotplug_lock and jump_label_mutex) holded while kmem_cache_alloc calls __might_resched (0.023356). I guess PowerPC's map_patch_area should temporary release above two locks before invoking map_kernel_page (which will call kmem_cache_alloc): static int map_patch_area(void *addr, unsigned long text_poke_addr) { unsigned long pfn; int err; if (is_vmalloc_or_module_addr(addr)) pfn = vmalloc_to_pfn(addr); else pfn = __pa_symbol(addr) >> PAGE_SHIFT; err = map_kernel_page(text_poke_addr, (pfn << PAGE_SHIFT), PAGE_KERNEL); pr_devel("Mapped addr %lx with pfn %lx:%d\n", text_poke_addr, pfn, err); if (err) return -1; return 0; } I will try to think it over in the coming days. [0.012154][T1] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:256 [0.013128][T1] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: swapper/0 [0.014015][T1] preempt_count: 0, expected: 0 [0.014505][T1] 2 locks held by swapper/0/1: [0.014987][T1] #0: c26108a0 (cpu_hotplug_lock){.+.+}-{0:0}, at: static_key_enable+0x24/0x50 [0.015995][T1] #1: c27416c8 (jump_label_mutex){+.+.}-{3:3}, at: static_key_enable_cpuslocked+0x88/0x120 [0.017107][T1] irq event stamp: 46 [0.017507][T1] hardirqs last enabled at (45): [] _raw_spin_unlock_irqrestore+0x94/0xd0 [0.018549][T1] hardirqs last disabled at (46): [] do_patch_instruction+0x3b4/0x4a0 [0.019549][T1] softirqs last enabled at (0): [] copy_process+0x8d0/0x1df0 [0.020474][T1] softirqs last disabled at (0): [<>] 0x0 [0.021200][T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.17.0-rc3-00349-gd3a9fd9fed88 #34 [0.022115][T1] Call Trace: [0.022443][T1] [c84837d0] [c0961aac] dump_stack_lvl+0xa0/0xec (unreliable) [0.023356][T1] [c8483820] [c019b314] __might_resched+0x2f4/0x310 [0.024174][T1] [c84838b0] [c04c0c70] kmem_cache_alloc+0x220/0x4b0 [0.025000][T1] [c8483920] [c0448af4] __pud_alloc+0x74/0x1d0 [0.025772][T1] [c8483970] [c008fe3c] hash__map_kernel_page+0x2cc/0x390 [0.026643][T1] [c8483a20] [c00a9944] do_patch_instruction+0x134/0x4a0 [0.027511][T1] [c8483a70] [c00559d4] arch_jump_label_transform+0x64/0x78 [0.028401][T1] [c8483a90] [c03d6288] __jump_label_update+0x148/0x180 [0.029254][T1] [c8483b30] [c03d6800] static_key_enable_cpuslocked+0xd0/0x120 [0.030179][T1] [c8483ba0] [c03d6880] static_key_enable+0x30/0x50 [0.030996][T1] [c8483bd0] [c200a8f4] check_kvm_guest+0x60/0x88 [0.031799][T1] [c8483c00] [c2027744] pSeries_smp_probe+0x54/0xb0 [0.032617][T1] [c8483c30] [c2011db8] smp_prepare_cpus+0x3e0/0x430 [0.033444][T1] [c8483cd0] [c2004908] kernel_init_freeable+0x20c/0x43c [0.034307][T1] [c8483db0] [c0012c00] kernel_init+0x30/0x1a0 [0.035078][T1] [c8483e10] [c000cd64] ret_from_kernel_thread+0x5c/0x64 Thanks for your trust in me! Cheers Zhouyi On Sun, Feb 13, 2022 at 7:05 AM Paul Menzel wrote: > > Dear Linux folks, > > > Running rcutorture on the POWER8 system IBM S822LC with Ubuntu 20.10, it > found the bug below. I more or less used rcu/dev (0ba8896d2fd7 > (lib/irq_poll: Declare IRQ_POLL softirq vector as ksoftirqd-parking > safe)) [1]. The bug manifested for the four configurations below. > > 1. results-rcutorture-kasan/SRCU-T > 2. results-rcutorture-kasan/TINY02 > 3. results-rcutorture/SRCU-T > 4. results-rcutorture/TINY02 > > For example, the attached > > > /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.11-22.00.51-torture/results-rcutorture-kasan/SRCU-T/console.log > > contains: > > ``` > [0.012154][T1] BUG: sleeping function called from invalid > context at include/linux/sched/mm.h:256 > [0.013128][T1] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, > pid: 1, name: swapper/0 > [0.014015][T1] preempt_count: 0, expected: 0 > [0.014505][T1] 2 locks held by swapper/0/1: > [0.014987][T1] #0: c26108a0 > (cpu_hotplug_lock){.+.+}-{0:0}, at: static_key_enable+0x24/0x50 > [0.015995][T1] #1: c27416c8 > (jump_label_mutex){+.+.}-{3:3}, at: static_key_enable_cpuslocked+0x88/0x120 > [0.017107][T1] irq event stamp: 46 > [0.017507][T1] hardirqs last enabled at (45): > [] _raw_spin_unlock_irqrestore+0x94/0xd0 > [0.018549][T1] hardirqs last disabled at (46): > [] do_patch_instruction+0
RE: [PATCH 2/2] powerpc/32: Implement csum_sub
From: Christophe Leroy > Sent: 11 February 2022 10:25 > > When building kernel with CONFIG_CC_OPTIMISE_FOR_SIZE, several > copies of csum_sub() are generated, with the following code: > > 0170 : >170: 7c 84 20 f8 not r4,r4 >174: 7c 63 20 14 addcr3,r3,r4 >178: 7c 63 01 94 addze r3,r3 >17c: 4e 80 00 20 blr > > Let's define a PPC32 version with subc/addme, and for it's inlining. > > It will return 0 instead of 0x when subtracting 0x8000 to itself, > this is not an issue as 0 and ~0 are equivalent, refer to RFC 1624. They are not always equivalent. In particular in the UDP checksum field one of them is (0?) 'checksum not calculated'. I think all the Linux functions have to return a non-zero value (for non-zero input). If the csum is going to be converted to 16 bit, inverted, and put into a packet the code usually has to have a check that changes 0 to 0x. However if the csum functions guarantee never to return zero they can feed an extra 1 into the first csum_partial() then just invert and add 1 at the end. Because (~csum_partion(buffer, 1) + 1) is the same as ~csum_partial(buffer, 0) except when the buffer's csum is 0x. I did do some experiments and the 64bit value can be reduced directly to 16bits using '% 0x'. This is different because it returns 0 not 0x. However gcc 'randomly' picks between the fast 'multiply by reciprocal' and slow divide instruction paths. The former is (probably) faster than reducing using shifts and adc. The latter definitely slower. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
RE: [PATCH] net: Remove branch in csum_shift()
From: Christophe Leroy > Sent: 11 February 2022 08:48 > > Today's implementation of csum_shift() leads to branching based on > parity of 'offset' > > 02f8 : >2f8: 70 a5 00 01 andi. r5,r5,1 >2fc: 41 a2 00 08 beq 304 >300: 54 84 c0 3e rotlwi r4,r4,24 >304: 7c 63 20 14 addcr3,r3,r4 >308: 7c 63 01 94 addze r3,r3 >30c: 4e 80 00 20 blr > > Use first bit of 'offset' directly as input of the rotation instead of > branching. > > 02f8 : >2f8: 54 a5 1f 38 rlwinm r5,r5,3,28,28 >2fc: 20 a5 00 20 subfic r5,r5,32 >300: 5c 84 28 3e rotlw r4,r4,r5 >304: 7c 63 20 14 addcr3,r3,r4 >308: 7c 63 01 94 addze r3,r3 >30c: 4e 80 00 20 blr > > And change to left shift instead of right shift to skip one more > instruction. This has no impact on the final sum. > > 02f8 : >2f8: 54 a5 1f 38 rlwinm r5,r5,3,28,28 >2fc: 5c 84 28 3e rotlw r4,r4,r5 >300: 7c 63 20 14 addcr3,r3,r4 >304: 7c 63 01 94 addze r3,r3 >308: 4e 80 00 20 blr That is ppc64. What happens on x86-64? Trying to do the same in the x86 ipcsum code tended to make the code worse. (Although that test is for an odd length fragment and can just be removed.) David > > Signed-off-by: Christophe Leroy > --- > include/net/checksum.h | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/include/net/checksum.h b/include/net/checksum.h > index 5218041e5c8f..9badcd5532ef 100644 > --- a/include/net/checksum.h > +++ b/include/net/checksum.h > @@ -83,9 +83,7 @@ static inline __sum16 csum16_sub(__sum16 csum, __be16 > addend) > static inline __wsum csum_shift(__wsum sum, int offset) > { > /* rotate sum to align it with a 16b boundary */ > - if (offset & 1) > - return (__force __wsum)ror32((__force u32)sum, 8); > - return sum; > + return (__force __wsum)rol32((__force u32)sum, (offset & 1) << 3); > } > > static inline __wsum > -- > 2.34.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH v4 16/20] Kbuild: add Rust support
Having all the new files in place, we now enable Rust support in the build system, including `Kconfig` entries related to Rust, the Rust configuration printer, the target definition files, the version detection script and a few other bits. In the future, we will likely want to generate the target files on the fly via a script. Co-developed-by: Alex Gaynor Signed-off-by: Alex Gaynor 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 Co-developed-by: Michael Ellerman Signed-off-by: Michael Ellerman Co-developed-by: Sven Van Asbroeck Signed-off-by: Sven Van Asbroeck Co-developed-by: Gary Guo Signed-off-by: Gary Guo Co-developed-by: Boris-Chengbiao Zhou Signed-off-by: Boris-Chengbiao Zhou Co-developed-by: Boqun Feng Signed-off-by: Boqun Feng Co-developed-by: Douglas Su Signed-off-by: Douglas Su Co-developed-by: Dariusz Sosnowski Signed-off-by: Dariusz Sosnowski Co-developed-by: Antonio Terceiro Signed-off-by: Antonio Terceiro Co-developed-by: Daniel Xu Signed-off-by: Daniel Xu Co-developed-by: Miguel Cano Signed-off-by: Miguel Cano Signed-off-by: Miguel Ojeda --- .gitignore | 5 + .rustfmt.toml| 12 + Makefile | 154 +++- arch/arm/rust/target.json| 27 ++ arch/arm64/rust/target.json | 34 ++ arch/powerpc/rust/target.json| 29 ++ arch/riscv/Makefile | 1 + arch/riscv/rust/rv32ima.json | 36 ++ arch/riscv/rust/rv32imac.json| 36 ++ arch/riscv/rust/rv64ima.json | 36 ++ arch/riscv/rust/rv64imac.json| 36 ++ arch/x86/rust/target.json| 36 ++ init/Kconfig | 45 ++- lib/Kconfig.debug| 144 +++ rust/.gitignore | 7 + rust/Makefile| 374 +++ rust/bindgen_parameters | 13 + scripts/Kconfig.include | 6 +- scripts/Makefile.build | 65 scripts/Makefile.debug | 10 + scripts/Makefile.lib | 12 + scripts/Makefile.modfinal| 8 +- scripts/cc-version.sh| 12 +- scripts/is_rust_module.sh| 13 + scripts/kconfig/confdata.c | 75 scripts/min-tool-version.sh | 6 + scripts/rust-is-available-bindgen-libclang.h | 2 + scripts/rust-is-available.sh | 158 28 files changed, 1372 insertions(+), 20 deletions(-) create mode 100644 .rustfmt.toml create mode 100644 arch/arm/rust/target.json create mode 100644 arch/arm64/rust/target.json create mode 100644 arch/powerpc/rust/target.json create mode 100644 arch/riscv/rust/rv32ima.json create mode 100644 arch/riscv/rust/rv32imac.json create mode 100644 arch/riscv/rust/rv64ima.json create mode 100644 arch/riscv/rust/rv64imac.json create mode 100644 arch/x86/rust/target.json create mode 100644 rust/.gitignore create mode 100644 rust/Makefile create mode 100644 rust/bindgen_parameters create mode 100755 scripts/is_rust_module.sh create mode 100644 scripts/rust-is-available-bindgen-libclang.h create mode 100755 scripts/rust-is-available.sh diff --git a/.gitignore b/.gitignore index 7afd412dadd2..48c68948f476 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,7 @@ *.o *.o.* *.patch +*.rmeta *.s *.so *.so.dbg @@ -96,6 +97,7 @@ modules.order !.gitattributes !.gitignore !.mailmap +!.rustfmt.toml # # Generated include files @@ -161,3 +163,6 @@ x509.genkey # Documentation toolchain sphinx_*/ + +# Rust analyzer configuration +/rust-project.json diff --git a/.rustfmt.toml b/.rustfmt.toml new file mode 100644 index ..3de5cc497465 --- /dev/null +++ b/.rustfmt.toml @@ -0,0 +1,12 @@ +edition = "2021" +newline_style = "Unix" + +# Unstable options that help catching some mistakes in formatting and that we may want to enable +# when they become stable. +# +# They are kept here since they are useful to run from time to time. +#format_code_in_doc_comments = true +#reorder_impl_items = true +#comment_width = 100 +#wrap_comments = true +#normalize_comments = true diff --git a/Makefile b/Makefile index ceb987e5c87b..1975d47e74e6 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,13 @@ endif export KBUILD_CHECKSRC +# Enable "clippy" (a linter) as part of the Rust compilation. +# +# Use 'make CLIPPY=1' to enable it. +ifeq ("$(origin CLIPPY)", "command line") + KBUILD_CLIPPY := $(CLIPPY) +endif + # Use make M=dir or set the environment variable KBUILD_EXTMOD to specify the # directory of external module to build.
[PATCH v4 00/20] Rust support
Rust support This is the patch series (v4) to add support for Rust as a second language to the Linux kernel. 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 As usual, special thanks go to ISRG (Internet Security Research Group) and Google for their financial support on this endeavor. Cheers, Miguel -- # Rust support This cover letter explains the major changes and updates done since the previous ones. For those, please see: RFC: https://lore.kernel.org/lkml/20210414184604.23473-1-oj...@kernel.org/ v1: https://lore.kernel.org/lkml/20210704202756.29107-1-oj...@kernel.org/ v2: https://lore.kernel.org/lkml/20211206140313.5653-1-oj...@kernel.org/ v3: https://lore.kernel.org/lkml/20220117053349.6804-1-oj...@kernel.org/ ## Infrastructure updates There have been several improvements to the overall Rust support: - The Intel 0DAY/LKP kernel test robot is setting up Rust in their CI -- we already got the first reports from the bot which we could act upon, very useful! - KernelCI is also looking forward to enabling Rust in their runs. - The GitHub CI we use for quick pre-merge testing has been moved to use containers. - Kernel modules do not need to write the crate attributes `#![no_std]` and `#![feature(...)]` anymore, removing boilerplate. - Added single target support, including `.o`, `.s`, `.ll` and `.i` (i.e. macro expanded, similar to C preprocessed sources). - Explanation of the `helpers.c` file and licensing for helpers and exports. - The documentation logo is now vector based (SVG). In addition, a vector version of Tux has been proposed for upstream, and an RFC for improved custom logo support has been submitted to upstream Rust. - Added coding guidelines on comments (`//`) and code documentation (`///`). - `is_rust_module.sh` rework. - Generation of `.rmeta` for leaf modules is skipped now. - Other cleanups, fixes and improvements. ## Abstractions and driver updates Some of the improvements to the abstractions and example drivers are: - Added support for static (global shared variables) synchronization primitives. `CONFIG_CONSTRUCTORS` are used for the implementation. - Simplification of the lock guards by using marker types, i.e. `Guard` and `GuardMut` are unified into a single parametrized type. If the marker is `WriteLock`, then `Guard` implements `DerefMut` (only implemented by `GuardMut` previously). - Optional parameters added to the registration of misc devices, following the builder pattern., e.g. miscdev::Options::new() .mode(0o600) .minor(10) .parent(parent) .register(reg, c_str!("sample"), ()) - Added `RwSemaphore` abstraction that wraps the C side `struct rw_semaphore`. - New `mm` module and VMA abstraction (wrapping the C side `struct vm_area_struct`) for use with `mmap`. - GPIO PL061 now uses the recently added `dev_*!` Rust macros. - Support the `!CONFIG_PRINTK` case. - Other cleanups, fixes and improvements. ## Patch series status The Rust support is still to be considered experimental. However, support is good enough that kernel developers can start working on the Rust abstractions for subsystems and write drivers and other modules. The current series has just arrived in `linux-next`, as usual. Similarly, the preview docs for this series can be seen at: https://rust-for-linux.github.io/docs/kernel/ As usual, please see the following link for the live list of unstable Rust features we are using: https://github.com/Rust-for-Linux/linux/issues/2 ## Acknowledgements The signatures in the main commits correspond to the people that wrote code that has ended up in them at the present time. For details on contributions to code and discussions, please see our repository: https://github.com/Rust-for-Linux/linux However, we would like to give credit to everyone that has contributed in one way or another to the Rust for Linux project. Since the previous cover letter: - Philip Li, Yujie Liu et. al. for setting the Intel 0DAY/LKP kernel test robot with Rust support. - Maciej Falkowski for continuing his work on the Hardware Random Number Generator subsystem, Samsung Exynos true random number generator, clock subsystem, etc. - bjorn3 for an extensive re-review of the previous round. - Jonathan Corbet for his feedback on the previous round. - Garrett LeSage and IFo Hancroft for agreeing to the usage of their vector version of Tux within the kernel tree with the same license as the existing one from Larry Ewing. - Nathan Chancellor for noticing a case where a C module was being identified as a Rust one for purposes of skipping BTF generation and suggesting a few improvements
Re: [PATCH v4 16/20] Kbuild: add Rust support
Hi John Paul, On Sat, Feb 12, 2022 at 7:27 PM John Paul Adrian Glaubitz wrote: > > Is there any particular reason why this list excludes MIPS*, i386, big-endian > PowerPC and SPARC targets which are already supported by the Rust programming > language? The variations we have so far were intended to showcase the Rust support in several major architectures, rather than cover everything. But as long as LLVM (& the kernel, i.e. ClangBuiltLinux) supports the target (and as long there are no compiler/toolchain issues), you should be able to try it. > Are the arch/$ARCH/rust/target.json files everything that's needed for > supporting > the other targets? Mostly -- there is also `rust/kernel/c_types.rs` and you may need to tweak `rust/compiler_builtins.rs`, but not much more. Note that for the target spec files, the short-term plan is to generate dynamically the target spec file according to what the architecture requests, instead of using these static files. Longer term, we need a Rust-stable way to setup custom targets from upstream `rustc` (though it is not clear yet how it will look, e.g. it could be via command-line flags). Cheers, Miguel
Re: [PATCH v4 16/20] Kbuild: add Rust support
On Sat, Feb 12, 2022 at 5:19 PM Russell King (Oracle) wrote: > > Right, so why made it dependent on CPU_32v6 || CPU_32v6K if ARMv7 is > supported? What about CPU_32v7? What about CPU_32v7M? > > I think it would be saner to use the CPU_V6, CPU_V6K, CPU_V7 and maybe > CPU_V7M here - even bettern to select "HAVE_RUST" from these symbols, > since I'm sure you'd start to see the issue behind my "HAVE_RUST" > suggestion as it means having four symbols just for 32-bit ARM on your > dependency line. To support arch variations properly we also have to configure the compiler via filling a target spec on the fly, but so far we only have a few static variations as an example. This is one of the missing parts of the arch support. I will let you know when we have something ready. > Interestingly, it does not list arm-unknown-linux-gnueabihf, which > is the "tuple" commonly used to build 32-bit ARM kernels. I see it there (Tier 2). > Probably because people incorrectly think it's required or some other > minor reason. As I say: In that case, we should remove them and warn about those instances, assuming the preferred style is to not have it. > so using the argument > that there are "500+ instances" and therefore should be seen as > correct is completely misguided. I did not use any such argument. > I mean, if we end up with, e.g. a filesystem coded in Rust, that > filesystem will not be available on architectures that the kernel As long as that filesystem is an optional feature (or as long as there is a C version), it should be fine. > supports until either (a) Rust gains support for that architecture For this, it would be ideal if entities behind some of the architectures could support LLVM & ClangBuiltLinux, or the GCC Rust frontend, or the GCC backend for `rustc`. For instance, Arm is supporting both LLVM and the Rust project. > or (b) someone re-codes the filesystem in C - at which point, what > is the point of having Rust in the kernel? Having a C version of some system does not mean a Rust version would not offer advantages. In fact, we are adding Rust precisely because we believe it offers some advantages over C, for both end users and maintainers. (Please see the RFC [1], previous discussions, etc.) [1] https://lore.kernel.org/lkml/20210414184604.23473-1-oj...@kernel.org/ Cheers, Miguel
Re: [PATCH v4 16/20] Kbuild: add Rust support
Hi! On Sat, Feb 12, 2022 at 02:03:42PM +0100, Miguel Ojeda wrote: > +config RUST > + bool "Rust support" > + depends on RUST_IS_AVAILABLE > + depends on ARM64 || CPU_32v6 || CPU_32v6K || (PPC64 && > CPU_LITTLE_ENDIAN) || X86_64 || RISCV Is there any particular reason why this list excludes MIPS*, i386, big-endian PowerPC and SPARC targets which are already supported by the Rust programming language? Are the arch/$ARCH/rust/target.json files everything that's needed for supporting the other targets? Thanks, Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PATCH v4 16/20] Kbuild: add Rust support
On Sat, Feb 12, 2022 at 04:47:33PM +0100, Miguel Ojeda wrote: > Hi Russell, > > On Sat, Feb 12, 2022 at 3:17 PM Russell King (Oracle) > wrote: > > > > Please don't use CPU_32v6* here. > > > > It probably makes more sense to add a symbol "HAVE_RUST" and have the > > appropriate architecture Kconfig files select HAVE_RUST. > > We can do it whatever way arch maintainers prefer, of course. Why > would you prefer one over the other? It would be cleaner, rather than the "depends" line getting longer and longer over time - and if different architecture maintainers change it, it will lead to conflicts. > > Does Rust support Thumb on ARMv6 and ARMv7 architectures? > > Yes, the main backend is LLVM. Some built-in targets and their support > level are listed here, if you want to take a look: Right, so why made it dependent on CPU_32v6 || CPU_32v6K if ARMv7 is supported? What about CPU_32v7? What about CPU_32v7M? I think it would be saner to use the CPU_V6, CPU_V6K, CPU_V7 and maybe CPU_V7M here - even bettern to select "HAVE_RUST" from these symbols, since I'm sure you'd start to see the issue behind my "HAVE_RUST" suggestion as it means having four symbols just for 32-bit ARM on your dependency line. > https://doc.rust-lang.org/nightly/rustc/platform-support.html Interestingly, it does not list arm-unknown-linux-gnueabihf, which is the "tuple" commonly used to build 32-bit ARM kernels. > > Please remove every utterance of "default n" from your patch; n is the > > default default which default defaults to, so you don't need to specify > > default n to make the option default to n. It will default to n purely > > because n is the default when no default is specified. > > Certainly. I am curious, though: is there a reason for most of the > other 500+ instances in the kernel tree? Probably because people incorrectly think it's required or some other minor reason. As I say: config FOO bool/tristate ... always defaults to 'n' without needing "default n" to be specified. Let's do some proper research on this. There are 19148 "config" statements in the kernel tree, 521 "default n" and 4818 that specify any kind of "default". That means there are about 14330 config statements that do not specify any kind of default. So, there are about 27 times more config statements that specify no default than those that specify "default n", so using the argument that there are "500+ instances" and therefore should be seen as correct is completely misguided. > > As Rust doesn't support all the architectures that the kernel supports, > > Rust must not be used for core infrastructure. > > Yeah, although I am not sure I understand what you are getting at here. I mean, if we end up with, e.g. a filesystem coded in Rust, that filesystem will not be available on architectures that the kernel supports until either (a) Rust gains support for that architecture or (b) someone re-codes the filesystem in C - at which point, what is the point of having Rust in the kernel? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v4 16/20] Kbuild: add Rust support
Hi Russell, On Sat, Feb 12, 2022 at 3:17 PM Russell King (Oracle) wrote: > > Please don't use CPU_32v6* here. > > It probably makes more sense to add a symbol "HAVE_RUST" and have the > appropriate architecture Kconfig files select HAVE_RUST. We can do it whatever way arch maintainers prefer, of course. Why would you prefer one over the other? > Does Rust support Thumb on ARMv6 and ARMv7 architectures? Yes, the main backend is LLVM. Some built-in targets and their support level are listed here, if you want to take a look: https://doc.rust-lang.org/nightly/rustc/platform-support.html There is also a GCC backend in `rustc` [1], which is making good progress and may eventually give us access to architectures supported by GCC. Furthermore, a from-scratch Rust frontend for GCC [2] is being worked on, though this effort will likely take more time to reach a point where it may be used for the kernel. [1] https://github.com/rust-lang/rust/tree/master/compiler/rustc_codegen_gcc [2] https://github.com/Rust-GCC/gccrs > Please remove every utterance of "default n" from your patch; n is the > default default which default defaults to, so you don't need to specify > default n to make the option default to n. It will default to n purely > because n is the default when no default is specified. Certainly. I am curious, though: is there a reason for most of the other 500+ instances in the kernel tree? > As Rust doesn't support all the architectures that the kernel supports, > Rust must not be used for core infrastructure. Yeah, although I am not sure I understand what you are getting at here. Cheers, Miguel
Re: [PATCH v4 16/20] Kbuild: add Rust support
On Sat, Feb 12, 2022 at 02:03:42PM +0100, Miguel Ojeda wrote: > +config RUST > + bool "Rust support" > + depends on RUST_IS_AVAILABLE > + depends on ARM64 || CPU_32v6 || CPU_32v6K || (PPC64 && > CPU_LITTLE_ENDIAN) || X86_64 || RISCV Please don't use CPU_32v6* here. It probably makes more sense to add a symbol "HAVE_RUST" and have the appropriate architecture Kconfig files select HAVE_RUST. Does Rust support Thumb on ARMv6 and ARMv7 architectures? > + depends on !MODVERSIONS > + depends on !GCC_PLUGIN_RANDSTRUCT > + select CONSTRUCTORS > + default n Please remove every utterance of "default n" from your patch; n is the default default which default defaults to, so you don't need to specify default n to make the option default to n. It will default to n purely because n is the default when no default is specified. > + help > + Enables Rust support in the kernel. > + > + This allows other Rust-related options, like drivers written in Rust, > + to be selected. As Rust doesn't support all the architectures that the kernel supports, Rust must not be used for core infrastructure. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
[PATCH] powerpc: Fix STACKTRACE=n build
Our skiroot_defconfig doesn't enable FTRACE, and so doesn't get STACKTRACE enabled either. That leads to a build failure since commit 1614b2b11fab ("arch: Make ARCH_STACKWALK independent of STACKTRACE") made stacktrace.c build even when STACKTRACE=n. arch/powerpc/kernel/stacktrace.c: In function ‘handle_backtrace_ipi’: arch/powerpc/kernel/stacktrace.c:171:2: error: implicit declaration of function ‘nmi_cpu_backtrace’ 171 | nmi_cpu_backtrace(regs); | ^ arch/powerpc/kernel/stacktrace.c: In function ‘arch_trigger_cpumask_backtrace’: arch/powerpc/kernel/stacktrace.c:226:2: error: implicit declaration of function ‘nmi_trigger_cpumask_backtrace’ 226 | nmi_trigger_cpumask_backtrace(mask, exclude_self, raise_backtrace_ipi); | ^ This happens because our headers haven't defined arch_trigger_cpumask_backtrace, which causes lib/nmi_backtrace.c not to build nmi_cpu_backtrace(). The code in question doesn't actually depend on STACKTRACE=y, that was just added because arch_trigger_cpumask_backtrace() lived in stacktrace.c for convenience. So drop the dependency on CONFIG_STACKTRACE, that causes lib/nmi_backtrace.c to build nmi_cpu_backtrace() etc. and fixes the build. Fixes: 1614b2b11fab ("arch: Make ARCH_STACKWALK independent of STACKTRACE") Signed-off-by: Michael Ellerman --- arch/powerpc/include/asm/nmi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/include/asm/nmi.h b/arch/powerpc/include/asm/nmi.h index 160abcb8e9fa..ea0e487f87b1 100644 --- a/arch/powerpc/include/asm/nmi.h +++ b/arch/powerpc/include/asm/nmi.h @@ -9,7 +9,7 @@ long soft_nmi_interrupt(struct pt_regs *regs); static inline void arch_touch_nmi_watchdog(void) {} #endif -#if defined(CONFIG_NMI_IPI) && defined(CONFIG_STACKTRACE) +#ifdef CONFIG_NMI_IPI extern void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self); #define arch_trigger_cpumask_backtrace arch_trigger_cpumask_backtrace -- 2.34.1