Add XArray abstraction #1019

matthewtgilbride · 2023-06-29T13:29:15Z

updates documentation for installing bindgen (now bindgen-cli)
adds the XArray abstraction

Currently the KernelAllocator simply passes the size of the type Layout to krealloc(), and in theory the alignment requirement from the type Layout may be larger than the guarantee provided by SLAB, which means the allocated object is mis-aligned. Fixes this by adjusting the allocation size to the nearest power of two, which SLAB always guarantees a size-aligned allocation. And because Rust guarantees that original size must be a multiple of alignment and the alignment must be a power of two, then the alignment requirement is satisfied. Suggested-by: Vlastimil Babka <vbabka@suse.cz> Co-developed-by: Andreas Hindborg (Samsung) <nmi@metaspace.dk> Signed-off-by: Andreas Hindborg (Samsung) <nmi@metaspace.dk> Signed-off-by: Boqun Feng <boqun.feng@gmail.com> Cc: stable@vger.kernel.org # v6.1+ Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Gary Guo <gary@garyguo.net> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20230613164258.3831917-1-boqun.feng@gmail.com

When combining `UnsafeCell` with `MaybeUninit`, it is idiomatic to use `UnsafeCell` as the outer type. Intuitively, this is because a `MaybeUninit<T>` might not contain a `T`, but we always want the effect of the `UnsafeCell`, even if the inner value is uninitialized. Now, strictly speaking, this doesn't really make a difference. The compiler will always apply the `UnsafeCell` effect even if the inner value is uninitialized. But I think we should follow the convention here. Signed-off-by: Alice Ryhl <aliceryhl@google.com> Reviewed-by: Gary Guo <gary@garyguo.net> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Link: https://lore.kernel.org/r/20230614115328.2825961-1-aliceryhl@google.com

nbdd0121 · 2023-06-29T14:06:04Z

Documentation/rust/quick-start.rst

@@ -98,7 +98,7 @@ the ``bindgen`` tool. A particular version is required.

 Install it via (note that this will download and build the tool from source)::

-	cargo install --locked --version $(scripts/min-tool-version.sh bindgen) bindgen
+	cargo install --locked --version $(scripts/min-tool-version.sh bindgen) bindgen-cli


I also discovered this yesterday. Could you send this patch directly to the mailing list?

@ojeda probably should take this as a fix.

This is https://rust-for-linux.zulipchat.com/#narrow/stream/291565-Help/topic/Build.20errors/near/365685550 (applied at the rebased ojeda@698129c), i.e. I was going to fix it directly when we take the upgrade to 0.65.1.

fbq · 2023-06-29T15:13:23Z

rust/kernel/lib.rs

@@ -45,6 +45,7 @@ pub mod str;
 pub mod sync;
 pub mod task;
 pub mod types;
+pub mod xarray;


not urgent, but should we introduce a collections module and make xarray a submod of it?

fbq · 2023-06-29T15:17:54Z

rust/kernel/xarray.rs

+}
+
+/// Wrapper for a value owned by the `XArray` which holds the `XArray` lock until dropped.
+pub struct ValueGuard<'a, T: ForeignOwnable>(NonNull<T>, Pin<&'a XArray<T>>);


Given it's a guard for (a value in) XArrary, maybe call it XGuard? (Sorry couldn't resist ;-))

I don't think NonNull<T> is the correct type here, consider you have a XArray<Box<i32>>, then you have a NonNull<Box<i32>> here, that's a pointer to a Box (i.e. second-level pointer).

It should be NonNull<core::ffi::c_void> I think, of course the type here doesn't matter much, so another idea is to use NonNull<u32>, since xarray requires pointers to be 4-byte aligned

We probably need a new type to capture the 4-byte alignment invariant.

y86-dev · 2023-06-30T13:49:31Z

rust/kernel/xarray.rs

+        // SAFETY: We have just created `xa`. This data structure does not require
+        // pinning.


Why do all of the functions take Pin<&Self> then? wouldnt &self be ok if this statement is true?

Also if I read this correctly, then there is a spinlock inside of the xarray, that might requiring pinning.

¯_(ツ)_/¯

I will say it makes things a bit awkward if you have a &mut ref as the caller. You can't freely call the methods that take &self without converting over to a Pin<&mut Self>, which I think works in normal rust...correct?

I am not sure what you mean. At the moment the new function just returns a plain XArray. If it needs to be pinned to be used, you should just use pin-init. If it does not need to be pinned, then the functions should take &self instead of self: Pin<&Self>.

As discussed - will update new to return Pin<Self> (or whatever the right thing should be return for pin-init)

It should be impl PinInit<Self>

y86-dev · 2023-06-30T13:50:00Z

rust/kernel/xarray.rs

+            )
+        };
+
+        let ret = unsafe { bindings::xa_err(old) };


Missing SAFETY comment.

y86-dev · 2023-06-30T13:56:34Z

rust/kernel/xarray.rs

+            T::from_foreign(new);
+        });
+
+        // SAFETY: `self.xa` is always valid by the type invariant, and we are storing


This invariant should be documented.

y86-dev · 2023-06-30T14:09:39Z

rust/kernel/xarray.rs

+impl<'a, T: ForeignOwnable> Drop for ValueGuard<'a, T> {
+    fn drop(&mut self) {
+        // SAFETY: The XArray we have a reference to owns the C xarray object.
+        unsafe { bindings::xa_unlock(self.1.xa.get()) };


this should be a type invariant.

y86-dev · 2023-06-30T14:13:26Z

rust/kernel/xarray.rs

+
+    /// Removes and returns an entry, returning it if it existed.
+    pub fn remove(self: Pin<&Self>, index: usize) -> Option<T> {
+        let p = unsafe { bindings::xa_erase(self.xa.get(), index.try_into().ok()?) };


safety comment

y86-dev · 2023-06-30T14:17:47Z

rust/kernel/xarray.rs

+///
+/// assert!(xarr.get(0).is_none());
+///
+/// xarr.set(0, Box::try_new(0)?);


missing ? at the end.

y86-dev · 2023-06-30T14:18:49Z

rust/kernel/xarray.rs

+/// // `find` returns the index/value pair matching the index, the next larger
+/// // index/value pair if it doesn't exist, or `None` of no larger index exists.


I think it would also be good to know if the index wraps around, so if this is true:

let xarr: Box<XArray<Box<usize>>> = Box::try_new(XArray::new(LOCK_IRQ))?; let xarr: Pin<Box<XArray<Box<usize>>>> = Box::into_pin(xarr); let xarr: Pin<&XArray<Box<usize>>> = xarr.as_ref(); xarr.set(0, Box::try_new(42)?)?; let (i, v) = xarr.find(1).unwrap(); assert_eq!(i, 0); assert_eq!(v.borrow(), &42);

y86-dev · 2023-06-30T14:20:21Z

rust/kernel/xarray.rs

+/// In this example, we create a new `XArray` and demonstrate
+/// rudimentary read/write operations.


I think this example is very helpful! Additionally it might be a good idea to take each example and put it on the relevant functions as well, especially the find function.

I didn't realize that worked! I think at one point in time I could only get the kunit stuff working if the tests were on the top of the struct.

I see now that it does...what do we consider better, a long doctest at the top with interactions between different functions, or individual tests for each function? (Or both...with some repetition?)

I would prefer to have a examples on functions. Big examples only make sense if you want to demonstrate how things are used together. I would recommend this when the API is excessively complicated. I dont think that is the case here.

y86-dev · 2023-06-30T14:25:58Z

rust/kernel/xarray.rs

+    /// This guard blocks all other actions on the `XArray`. Callers are expected to drop the
+    /// `ValueGuardMut` eagerly to avoid blocking other users, such as by taking a clone of the value.


The receiver type is Pin<&mut Self>, so we have unique access. therefore other users need to wait for some other lock until they get access... I would just remove this.

y86-dev · 2023-06-30T14:31:38Z

rust/kernel/xarray.rs

+    ///
+    /// Returns a `Reservation` object, which can then be used to store a value at this index or
+    /// otherwise free it for reuse.
+    pub fn reserve_limits(self: Pin<&Self>, min: u32, max: u32) -> Result<Reservation<'_, T>> {


just unimportant bikeshedding: I would call this reserve_between and the other functions adjusted similarly. What do the others think?

Adds a `PhantomPinned` field to `Opaque<T>`. This removes the last Rust guarantee: the assumption that the type `T` can be freely moved. This is not the case for many types from the C side (e.g. if they contain a `struct list_head`). This change removes the need to add a `PhantomPinned` field manually to Rust structs that contain C structs which must not be moved. Signed-off-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Gary Guo <gary@garyguo.net> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20230630150216.109789-1-benno.lossin@proton.me

The -v option is passed when this script is invoked from Makefile, but not when invoked from Kconfig. As you can see in scripts/Kconfig.include, the 'success' macro suppresses stdout and stderr anyway, so this script does not need to be quiet. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Miguel Ojeda <ojeda@kernel.org> Tested-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Link: https://lore.kernel.org/r/20230109061436.3146442-1-masahiroy@kernel.org [ Reworded prefix to match the others in the patch series. ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-2-ojeda@kernel.org

…uments rust_is_available.sh uses cc-version.sh to identify which C compiler is in use, as scripts/Kconfig.include does. cc-version.sh isn't designed to be able to handle multiple arguments in one variable, i.e. "ccache clang". Its invocation in rust_is_available.sh quotes "$CC", which makes $1 == "ccache clang" instead of the intended $1 == ccache & $2 == clang. cc-version.sh could also be changed to handle having "ccache clang" as one argument, but it only has the one consumer upstream, making it simpler to fix the caller here. Signed-off-by: Russell Currey <ruscur@russell.cc> Fixes: 78521f3 ("scripts: add `rust_is_available.sh`") Link: Rust-for-Linux#873 [ Reworded title prefix and reflow line to 75 columns. ] Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-3-ojeda@kernel.org

Sometimes users need to tweak the finding process of `libclang` for `bindgen` via the `clang-sys`-provided environment variables. Thus add a paragraph to the setting up guide, including a reference to `clang-sys`'s relevant documentation. Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/ Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-4-ojeda@kernel.org

People trying out the Rust support in the kernel may get warnings and errors from `scripts/rust_is_available.sh` from the `rustavailable` target or the build step. Some of those users may be following the Quick Start guide, but others may not (likely those getting warnings from the build step instead of the target). While the messages are fairly clear on what the problem is, it may not be clear how to solve the particular issue, especially for those not aware of the documentation. We could add all sorts of details on the script for each one, but it is better to point users to the documentation instead, where it is easily readable in different formats. It also avoids duplication. Thus add a reference to the documentation whenever the script fails or there is at least a warning. Reviewed-by: Finn Behrens <fin@nyantec.com> Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-5-ojeda@kernel.org

`scripts/rust_is_available.sh` calls `bindgen` with a special header in order to check whether the `libclang` version in use is suitable. However, the invocation itself may fail if, for instance, `bindgen` cannot locate `libclang`. This is fine for Kconfig (since the script will still fail and therefore disable Rust as it should), but it is pretty confusing for users of the `rustavailable` target given the error will be unrelated: ./scripts/rust_is_available.sh: 21: arithmetic expression: expecting primary: "100000 * + 100 * + " make: *** [Makefile:1816: rustavailable] Error 2 Instead, run the `bindgen` invocation independently in a previous step, saving its output and return code. If it fails, then show the user a proper error message. Otherwise, continue as usual with the saved output. Since the previous patch we show a reference to the docs, and the docs now explain how `bindgen` looks for `libclang`, thus the error message can leverage the documentation, avoiding duplication here (and making users aware of the setup guide in the documentation). Reported-by: Nick Desaulniers <ndesaulniers@google.com> Link: https://lore.kernel.org/rust-for-linux/CAKwvOdm5JT4wbdQQYuW+RT07rCi6whGBM2iUAyg8A1CmLXG6Nw@mail.gmail.com/ Reported-by: François Valenduc <francoisvalenduc@gmail.com> Closes: Rust-for-Linux#934 Reported-by: Alexandru Radovici <msg4alex@gmail.com> Closes: Rust-for-Linux#921 Reported-by: Matthew Leach <dev@mattleach.net> Closes: https://lore.kernel.org/rust-for-linux/20230507084116.1099067-1-dev@mattleach.net/ Fixes: 78521f3 ("scripts: add `rust_is_available.sh`") Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-6-ojeda@kernel.org

Sometimes [1] users may attempt to setup the Rust support by checking what Kbuild does and they end up finding out about `scripts/rust_is_available.sh`. Inevitably, they run the script directly, but unless they setup the required variables, the result of the script is not meaningful. We could add some defaults to the variables, but that could be confusing for those that may override the defaults (compared to their kernel builds), and `$CC` would not be a simple default in any case. Therefore, instead, explicitly check whether the expected variables are set (`$RUSTC`, `$BINDGEN` and `$CC`). If not, print an explanation about the fact that the script is meant to be called from Kbuild, since that is the most likely cause for the variables not being set. Link: https://lore.kernel.org/oe-kbuild-all/Y6r4mXz5NS0+HVXo@zn.tnic/ [1] Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-7-ojeda@kernel.org

…e path `bindgen`'s output for `libclang`'s version check contains paths, which in turn may contain strings that look like version numbers [1][2]: .../6.1.0-dev/.../rust_is_available_bindgen_libclang.h:2:9: warning: clang version 11.1.0 [-W#pragma-messages], err: false which the script will pick up as the version instead of the latter. It is also the case that versions may appear after the actual version (e.g. distribution's version text), which was the reason behind `head` [3]: .../rust-is-available-bindgen-libclang.h:2:9: warning: clang version 13.0.0 (Fedora 13.0.0-3.fc35) [-W#pragma-messages], err: false Thus instead ask for a match after the `clang version` string. Reported-by: Jordan Isaacs <mail@jdisaacs.com> Closes: Rust-for-Linux#942 [1] Reported-by: Ethan D. Twardy <ethan.twardy@gmail.com> Closes: https://lore.kernel.org/rust-for-linux/20230528131802.6390-2-ethan.twardy@gmail.com/ [2] Reported-by: Tiago Lam <tiagolam@gmail.com> Closes: Rust-for-Linux#789 [3] Fixes: 78521f3 ("scripts: add `rust_is_available.sh`") Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-By: Ethan Twardy <ethan.twardy@gmail.com> Tested-By: Ethan Twardy <ethan.twardy@gmail.com> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-8-ojeda@kernel.org

In order to match the version string, `sed` is used in a couple cases, and `grep` and `head` in a couple others. Make the script more consistent and easier to understand by using the same method, `sed`, for all of them. This makes the version matching also a bit more strict for the changed cases, since the strings `rustc ` and `bindgen ` will now be required, which should be fine since `rustc` complains if one attempts to call it with another program name, and `bindgen` uses a hardcoded string. In addition, clarify why one of the existing `sed` commands does not provide an address like the others. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-9-ojeda@kernel.org

The script already checks if `$RUSTC` and `$BINDGEN` exists via `command`, but the environment variables may point to a non-executable file, or the programs may fail for some other reason. While the script successfully exits with a failure as it should, the error given can be quite confusing depending on the shell and the behavior of its `command`. For instance, with `dash`: $ RUSTC=./mm BINDGEN=bindgen CC=clang scripts/rust_is_available.sh scripts/rust_is_available.sh: 19: arithmetic expression: expecting primary: "100000 * + 100 * + " Thus detect failure exit codes when calling `$RUSTC` and `$BINDGEN` and print a better message, in a similar way to what we do when extracting the `libclang` version found by `bindgen`. Link: https://lore.kernel.org/rust-for-linux/CAK7LNAQYk6s11MASRHW6oxtkqF00EJVqhHOP=5rynWt-QDUsXw@mail.gmail.com/ Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-10-ojeda@kernel.org

The script already checks for `$RUSTC` and `$BINDGEN` existing and exiting without failure. However, one may still pass an unexpected binary that does not output what the later parsing expects. The script still successfully reports a failure as expected, but the error is confusing. For instance: $ RUSTC=true BINDGEN=bindgen CC=clang scripts/rust_is_available.sh scripts/rust_is_available.sh: 19: arithmetic expression: expecting primary: "100000 * + 100 * + " *** *** Please see Documentation/rust/quick-start.rst for details *** on how to set up the Rust support. *** Thus add an explicit check and a proper message for unexpected output from the called command. Similarly, do so for the `libclang` version parsing, too. Link: https://lore.kernel.org/rust-for-linux/CAK7LNAQYk6s11MASRHW6oxtkqF00EJVqhHOP=5rynWt-QDUsXw@mail.gmail.com/ Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-11-ojeda@kernel.org

The `rust_is_available.sh` script runs for everybody compiling the kernel, even if not using Rust. Therefore, it is important to ensure that the script is correct to avoid breaking people's compilation. In addition, the script needs to be able to handle a set of subtle cases, including parsing version strings of different tools. Therefore, maintenance of this script can be greatly eased with a set of tests. Thus add a test suite to cover hopefully most of the setups that the script may encounter in the wild. Extra setups can be easily added later on if missing. The script currently covers all the branches of the shell script, including several ways in which they may be entered. Python is used for this script, since the script under test does not depend on Rust, thus hopefully making it easier for others to use if the need arises. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230616001631.463536-12-ojeda@kernel.org

While there are default impls for these methods, using the respective C api's is faster. Currently neither the existing nor these new GlobalAlloc method implementations are actually called. Instead the __rust_* function defined below the GlobalAlloc impl are used. With rustc 1.71 these functions will be gone and all allocation calls will go through the GlobalAlloc implementation. Link: Rust-for-Linux#68 Signed-off-by: Björn Roy Baron <bjorn3_gh@protonmail.com> [boqun: add size adjustment for alignment requirement] Signed-off-by: Boqun Feng <boqun.feng@gmail.com> Link: https://lore.kernel.org/r/20230625232528.89306-1-boqun.feng@gmail.com

* Rationale: Upgrades bindgen to code-generation for anonymous unions, structs, and enums [7] for LLVM-16 based toolchains. The following upgrade also incorporates `noreturn` support from bindgen allowing us to remove useless `loop` calls which was placed as a workaround. * Truncated build logs on using bindgen `v0.56.0` prior to LLVM-16 toolchain: ``` $ make rustdoc LLVM=1 CLIPPY=1 -j$(nproc) RUSTC L rust/core.o BINDGEN rust/bindings/bindings_generated.rs BINDGEN rust/bindings/bindings_helpers_generated.rs BINDGEN rust/uapi/uapi_generated.rs thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', .../proc-macro2-1.0.24/src/fallback.rs:693:9 ... thread 'main' panicked at '"ftrace_branch_data_union_(anonymous_at__/_/include/linux/compiler_types_h_146_2)" is not a valid Ident', .../proc-macro2-1.0.24/src/fallback.rs:693:9 ... ``` * LLVM-16 Changes: API changes [1] were introduced such that libclang would emit names like "(unnamed union at compiler_types.h:146:2)" for unnamed unions, structs, and enums whereas it previously returned an empty string. * Bindgen Changes: Bindgen `v0.56.0` on LLVM-16 based toolchains hence was unable to process the anonymous union in `include/linux/compiler_types` `struct ftrace_branch_data` and caused subsequent panics as the new `libclang` API emitted name was not being handled. The following issue was fixed in Bindgen `v0.62.0` [2]. Bindgen `v0.58.0` changed the flags `--whitelist-*` and `--blacklist-*` to `--allowlist-*` and `--blocklist-*` respectively [3]. Bindgen `v0.61.0` added support for `_Noreturn`, `[[noreturn]]`, `__attribute__((noreturn))` [4], hence the empty `loop`s used to circumvent bindgen returning `!` in place of `()` for noreturn attributes have been removed completely. Bindgen `v0.61.0` also changed default functionality to bind `size_t` to `usize` [5] and added the `--no-size_t-is-usize` [5] flag to not bind `size_t` as `usize`. Bindgen `v0.65.0` removed `--size_t-is-usize` flag [6]. Link: llvm/llvm-project@19e984e [1] Link: rust-lang/rust-bindgen#2319 [2] Link: rust-lang/rust-bindgen#1990 [3] Link: rust-lang/rust-bindgen#2094 [4] Link: rust-lang/rust-bindgen@cc78b6f [5] Link: rust-lang/rust-bindgen#2408 [6] Closes: Rust-for-Linux#1013 [7] Signed-off-by: Aakash Sen Sharma <aakashsensharma@gmail.com> Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Ariel Miculas <amiculas@cisco.com> Link: https://lore.kernel.org/r/20230612194311.24826-1-aakashsensharma@gmail.com [boqun: resolve conflicts because of Rust version bump] Signed-off-by: Boqun Feng <boqun.feng@gmail.com>

Rust documentation tests are going to be build/run-tested with the KUnit integration added in a future patch, thus update them to make them compilable/testable so that we may start enforcing it. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com> Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20230614180837.630180-2-ojeda@kernel.org

Rust documentation tests are going to be build/run-tested with the KUnit integration added in a future patch, thus update them to make them compilable/testable so that we may start enforcing it. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com> Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20230614180837.630180-3-ojeda@kernel.org

Rust documentation tests are going to be build/run-tested with the KUnit integration added in a future patch, thus update them to make them compilable/testable so that we may start enforcing it. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com> Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20230614180837.630180-4-ojeda@kernel.org

Rust documentation tests are going to be build/run-tested with the KUnit integration added in a future patch, thus update them to make them compilable/testable so that we may start enforcing it. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com> Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20230614180837.630180-5-ojeda@kernel.org

Rust has documentation tests: these are typically examples of usage of any item (e.g. function, struct, module...). They are very convenient because they are just written alongside the documentation. For instance: /// Sums two numbers. /// /// ``` /// assert_eq!(mymod::f(10, 20), 30); /// ``` pub fn f(a: i32, b: i32) -> i32 { a + b } In userspace, the tests are collected and run via `rustdoc`. Using the tool as-is would be useful already, since it allows to compile-test most tests (thus enforcing they are kept in sync with the code they document) and run those that do not depend on in-kernel APIs. However, by transforming the tests into a KUnit test suite, they can also be run inside the kernel. Moreover, the tests get to be compiled as other Rust kernel objects instead of targeting userspace. On top of that, the integration with KUnit means the Rust support gets to reuse the existing testing facilities. For instance, the kernel log would look like: KTAP version 1 1..1 KTAP version 1 # Subtest: rust_doctests_kernel 1..59 # Doctest from line 13 ok 1 rust_doctest_kernel_build_assert_rs_0 # Doctest from line 56 ok 2 rust_doctest_kernel_build_assert_rs_1 # Doctest from line 122 ok 3 rust_doctest_kernel_init_rs_0 ... # Doctest from line 150 ok 59 rust_doctest_kernel_types_rs_2 # rust_doctests_kernel: pass:59 fail:0 skip:0 total:59 # Totals: pass:59 fail:0 skip:0 total:59 ok 1 rust_doctests_kernel Therefore, add support for running Rust documentation tests in KUnit. Some other notes about the current implementation and support follow. The transformation is performed by a couple scripts written as Rust hostprogs. Tests using the `?` operator are also supported as usual, e.g.: /// ``` /// # use kernel::{spawn_work_item, workqueue}; /// spawn_work_item!(workqueue::system(), || pr_info!("x"))?; /// # Ok::<(), Error>(()) /// ``` The tests are also compiled with Clippy under `CLIPPY=1`, just like normal code, thus also benefitting from extra linting. The names of the tests are currently automatically generated. This allows to reduce the burden for documentation writers, while keeping them fairly stable for bisection. This is an improvement over the `rustdoc`-generated names, which include the line number; but ideally we would like to get `rustdoc` to provide the Rust item path and a number (for multiple examples in a single documented Rust item). In order for developers to easily see from which original line a failed doctests came from, a KTAP diagnostic line is printed to the log. In the future, we may be able to use a proper KUnit facility to append this sort of information instead. A notable difference from KUnit C tests is that the Rust tests appear to assert using the usual `assert!` and `assert_eq!` macros from the Rust standard library (`core`). We provide a custom version that forwards the call to KUnit instead. Importantly, these macros do not require passing context, unlike the KUnit C ones (i.e. `struct kunit *`). This makes them easier to use, and readers of the documentation do not need to care about which testing framework is used. In addition, it may allow us to test third-party code more easily in the future. However, a current limitation is that KUnit does not support assertions in other tasks. Thus we presently simply print an error to the kernel log if an assertion actually failed. This should be revisited to properly fail the test, perhaps saving the context somewhere else, or letting KUnit handle it. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Tested-by: Sergio González Collado <sergio.collado@gmail.com> Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Tested-by: Matt Gilbride <mattgilbride@google.com> Reviewed-by: Alice Ryhl <aliceryhl@google.com> Link: https://lore.kernel.org/r/20230614180837.630180-6-ojeda@kernel.org [boqun: Add #include <linux/stddef.h>]

The KUnit maintainers would like to maintain these files on their side too (thanks!), so add them to their entry. With this in place, `scripts/get_maintainer.pl` prints both sets of maintainers/reviewers (i.e. KUnit and Rust) for those files, which is the behavior we are looking for. Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230614180837.630180-7-ojeda@kernel.org

If the trait has same function name, the `vtable` macro will redefine its `gen_const_name`, e.g.: ```rust #[vtable] pub trait Foo { #[cfg(CONFIG_X)] fn bar(); #[cfg(not(CONFIG_X))] fn bar(x: usize); } ``` Use `HashSet` to avoid this. Signed-off-by: Qingsong Chen <changxian.cqs@antgroup.com> Reviewed-by: Gary Guo <gary@garyguo.net> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> Link: https://lore.kernel.org/r/20230626074242.3945398-2-changxian.cqs@antgroup.com

This macro provides a flexible way to concatenated identifiers together and it allows the resulting identifier to be used to declare new items, which `concat_idents!` does not allow. It also allows identifiers to be transformed before concatenated. The `concat_idents!` example let x_1 = 42; let x_2 = concat_idents!(x, _1); assert!(x_1 == x_2); can be written with `paste!` macro like this: let x_1 = 42; let x_2 = paste!([<x _1>]); assert!(x_1 == x_2); However `paste!` macro is more flexible because it can be used to create a new variable: let x_1 = 42; paste!(let [<x _2>] = [<x _1>];); assert!(x_1 == x_2); While this is not possible with `concat_idents!`. This macro is similar to the `paste!` crate [1], but this is a fresh implementation to avoid vendoring large amount of code directly. Also, I have augmented it to provide a way to specify span of the resulting token, allowing precise control. For example, this code is broken because the variable is declared inside the macro, so Rust macro hygiene rules prevents access from the outside: macro_rules! m { ($id: ident) => { // The resulting token has hygiene of the macro. paste!(let [<$id>] = 1;) } } m!(a); let _ = a; In this versionn of `paste!` macro I added a `span` modifier to allow this: macro_rules! m { ($id: ident) => { // The resulting token has hygiene of `$id`. paste!(let [<$id:span>] = 1;) } } m!(a); let _ = a; Link: http://docs.rs/paste/ [1] Signed-off-by: Gary Guo <gary@garyguo.net> Reviewed-by: Björn Roy Baron <bjorn3_gh@protonmail.com> Reviewed-by: Benno Lossin <benno.lossin@proton.me> Link: https://lore.kernel.org/r/20230628171108.1150742-1-gary@garyguo.net

This commit provides the build flags for Rust for AArch64. The core Rust support already in the kernel does the rest. When disabling the neon and fp target features to avoid fp & simd registers. The use of fp-armv8 will cause a warning from rustc about an unknown feature that is specified. The target feature is still passed through to LLVM, this behaviour is documented as part of the warning. This will be fixed in a future version of the rustc toolchain. The Rust samples have been tested with this commit. Signed-off-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com> Link: https://lore.kernel.org/r/20230606145606.1153715-2-Jamie.Cunliffe@arm.com [boqun: resolve the conflicts with kunit test enablement]

Enable the PAC ret and BTI options in the Rust build flags to match the options that are used when building C. Signed-off-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com> Link: https://lore.kernel.org/r/20230606145606.1153715-3-Jamie.Cunliffe@arm.com

fbq and others added 2 commits June 25, 2023 16:49

nbdd0121 reviewed Jun 29, 2023

View reviewed changes

fbq reviewed Jun 29, 2023

View reviewed changes

y86-dev reviewed Jun 30, 2023

View reviewed changes

Benno Lossin and others added 24 commits June 30, 2023 13:24

fbq force-pushed the rust-dev branch from 01527f6 to f347fa1 Compare October 23, 2023 20:16

fbq force-pushed the rust-dev branch from f347fa1 to 14a70e5 Compare November 20, 2023 20:46

fbq force-pushed the rust-dev branch 2 times, most recently from 5e2b9bc to 5cd4b93 Compare December 14, 2023 20:06

fbq force-pushed the rust-dev branch 2 times, most recently from 03ec649 to 8f7e376 Compare December 28, 2023 19:38

fbq force-pushed the rust-dev branch from a9f81c2 to 779be36 Compare January 8, 2024 23:37

fbq force-pushed the rust-dev branch 2 times, most recently from 9bdbf5d to ddbd3ca Compare January 22, 2024 21:49

fbq force-pushed the rust-dev branch 3 times, most recently from 7b0b25e to ba2f66a Compare February 5, 2024 18:00

fbq force-pushed the rust-dev branch 3 times, most recently from 596f12c to 6bdd6e6 Compare February 19, 2024 01:41

fbq force-pushed the rust-dev branch 3 times, most recently from b27fcc3 to 9a03b3f Compare March 12, 2024 00:07

fbq force-pushed the rust-dev branch 5 times, most recently from 35767d5 to 6007000 Compare March 29, 2024 23:38

fbq force-pushed the rust-dev branch 2 times, most recently from 5d0c2b6 to 198cc1a Compare April 10, 2024 17:42

fbq force-pushed the rust-dev branch 3 times, most recently from 3f27cd3 to 8ccee0c Compare April 18, 2024 23:06

fbq force-pushed the rust-dev branch from 8ccee0c to c6bf072 Compare May 6, 2024 18:32

fbq force-pushed the rust-dev branch from c6bf072 to 4c3a73a Compare May 28, 2024 18:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add XArray abstraction #1019

Add XArray abstraction #1019

matthewtgilbride commented Jun 29, 2023 •

edited

nbdd0121 Jun 29, 2023

ojeda Jun 29, 2023 •

edited

fbq Jun 29, 2023

fbq Jun 29, 2023

fbq Jun 29, 2023 •

edited

nbdd0121 Jun 30, 2023

y86-dev Jun 30, 2023

matthewtgilbride Jul 5, 2023

y86-dev Jul 5, 2023

matthewtgilbride Jul 5, 2023

y86-dev Jul 5, 2023

y86-dev Jun 30, 2023

y86-dev Jun 30, 2023

y86-dev Jun 30, 2023

y86-dev Jun 30, 2023

y86-dev Jun 30, 2023

y86-dev Jun 30, 2023

y86-dev Jun 30, 2023

matthewtgilbride Jul 5, 2023

y86-dev Jul 5, 2023

y86-dev Jun 30, 2023

y86-dev Jun 30, 2023

		// SAFETY: We have just created `xa`. This data structure does not require
		// pinning.

		/// // `find` returns the index/value pair matching the index, the next larger
		/// // index/value pair if it doesn't exist, or `None` of no larger index exists.

		/// In this example, we create a new `XArray` and demonstrate
		/// rudimentary read/write operations.

		/// This guard blocks all other actions on the `XArray`. Callers are expected to drop the
		/// `ValueGuardMut` eagerly to avoid blocking other users, such as by taking a clone of the value.

Add XArray abstraction #1019

Are you sure you want to change the base?

Add XArray abstraction #1019

Conversation

matthewtgilbride commented Jun 29, 2023 • edited

Choose a reason for hiding this comment

ojeda Jun 29, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fbq Jun 29, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewtgilbride commented Jun 29, 2023 •

edited

ojeda Jun 29, 2023 •

edited

fbq Jun 29, 2023 •

edited