Skip to content

Fix CI and sync with Linux 6.12#23

Merged
BennoLossin merged 12 commits intoRust-for-Linux:mainfrom
bonzini:cleanup-pre-stable
Nov 22, 2024
Merged

Fix CI and sync with Linux 6.12#23
BennoLossin merged 12 commits intoRust-for-Linux:mainfrom
bonzini:cleanup-pre-stable

Conversation

@bonzini
Copy link
Copy Markdown
Contributor

@bonzini bonzini commented Nov 4, 2024 •
edited
Loading

I've decided to retarget this PR to just syncing with Linux 6.12 before making further changes. In particular this brings in the InPlaceWrite trait.

Old text

It is actually not that hard to make pinned_init usable with stable Rust. In particular:

  • new_uninit has been stabilized (and anyway in the end it was just a glorified Box::<MaybeUninit<T>>::new())
  • allocator_api is needed to have fallible allocation, but not if you just replace core::alloc::AllocError with core::convert::Infallible
  • get_mut_unchecked is needed if you need Arc, but otherwise is not necessary.

This pull request fixes CI and isolates a little bit more the pieces of code that refer to Arc or feature(allocator_api). The idea is that these are mostly uncontroversial changes that should not hamper readability and maintainability, and therefore can be committed separately.

Please let me know if I should submit separately the small changes to src/lib.rs, for example via the rust-for-linux mailing list.

@bonzini bonzini force-pushed the cleanup-pre-stable branch from a8ed63c to 8dafd4c Compare November 4, 2024 14:33
@bonzini
lib: do not mark new_uninit as required
3777821 
new_uninit is stable as of 1.82.0, there is no need to require it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@bonzini
cargo: add unexpected_cfgs support
e15b516 
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@bonzini
tests: update output for compile-fail tests
3f55f76 
Some of the error messages have changed in nightly Rust, since the
.stderr files were last checked.  Update everything.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@bonzini
treewide: remove unused imports
1926193 
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@bonzini bonzini force-pushed the cleanup-pre-stable branch from 8dafd4c to b74dc46 Compare November 4, 2024 14:47
@bonzini
Copy link
Copy Markdown
Contributor Author

bonzini commented Nov 4, 2024

Successful CI run at bonzini#1

@bonzini bonzini force-pushed the cleanup-pre-stable branch from b74dc46 to 3cc7086 Compare November 4, 2024 15:39
@BennoLossin
Copy link
Copy Markdown
Member

BennoLossin commented Nov 7, 2024

thanks for the PR! I am currently very busy, so I might take some time before getting this in

@bonzini
Copy link
Copy Markdown
Contributor Author

bonzini commented Nov 7, 2024

Thanks, could you at least approval the workflow so that I can fix any issues?

@BennoLossin
Copy link
Copy Markdown
Member

BennoLossin commented Nov 7, 2024

there is one issue with a ui test that the main branch is also failing at the moment, so I expect it to also fail here. I'll fix that (hopefully) soon

@bonzini
Copy link
Copy Markdown
Contributor Author

bonzini commented Nov 7, 2024

I think it's one of those that I fixed. :)

@bonzini
Copy link
Copy Markdown
Contributor Author

bonzini commented Nov 19, 2024 •
edited
Loading

Ping - is there anything I can do to help? I have changed the purpose of this pull request to sync pinned_init with upstream Linux, for example the inspect_err change I am making was already done in commit dee1396a486c, hopefully this removes some of the work from you.

@bonzini bonzini changed the title Misc cleanups before allowing usage with stable Rust Fix CI and sync with Linux 6.12 Nov 19, 2024
@bonzini bonzini force-pushed the cleanup-pre-stable branch 2 times, most recently from ba238fa to d3aab51 Compare November 19, 2024 15:53
@bonzini bonzini mentioned this pull request Nov 19, 2024
Merged
BennoLossin
BennoLossin requested changes Nov 22, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@BennoLossin BennoLossin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for keeping you waiting.

Thanks a lot for this, it's very helpful that you properly cherry picked the commits and added your signed-off-by! Very much appreciated!

Just two comments with regards to your changes. Other than that it looks good!

Comment thread examples/error.rs
Comment thread tests/ring_buf.rs Outdated
bonzini and others added 8 commits November 22, 2024 14:20
@bonzini
examples: converting from Infallible is unreachable
bcaf8e8 
Use the same implementation for both examples/error.rs and
examples/pthread_mutex.rs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@bonzini
tests: ring_buf: remove dependency on allocator_api
8f70649 
Reuse examples/error.rs so that the error returned by EvenU64::new2() is
not core::alloc::AllocError.  This will allow running the test without
the allocator_api feature.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@bonzini
examples: remove unnecessary feature
24acb56 
The big_struct_in_place example only uses init!, which has no dependency
on the allocator API (which provides the functionality of fallible
initialization).  Do not enable allocator_api.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@bonzini
rust: init: simplify from map_err to inspect_err
0385db4 
A new complexity lint, `manual_inspect` [1], has been introduced in
the upcoming Rust 1.81 (currently in nightly), which checks for uses of
`map*` which return the original item:

    error:
    --> rust/kernel/init.rs:846:23
        |
    846 |         (self.1)(val).map_err(|e| {
        |                       ^^^^^^^
        |
        = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
        = note: `-D clippy::manual-inspect` implied by `-D warnings`
        = help: to override `-D warnings` add `#[allow(clippy::manual_inspect)]`
    help: try
        |
    846 ~         (self.1)(val).inspect_err(|e| {
    847 |             // SAFETY: `slot` was initialized above.
    848 ~             unsafe { core::ptr::drop_in_place(slot) };
        |

Thus clean them up.

Link: https://rust-lang.github.io/rust-clippy/master/index.html#/manual_inspect [1]
Tested-by: Benno Lossin <benno.lossin@proton.me>
Tested-by: Andreas Hindborg <a.hindborg@samsung.com>
Link: https://lore.kernel.org/r/20240709160615.998336-3-ojeda@kernel.org
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit dee1396a486cf2b6e7840322f6d104680649f2ff)
@BennoLossin @bonzini
rust: init: change the generated name of guard variables
307b2bf 
The initializers created by the `[try_][pin_]init!` macros utilize the
guard pattern to drop already initialized fields, when initialization
fails mid-way. These guards are generated to have the same name as the
field that they handle. To prevent namespacing issues [1] when the
field name is the same as e.g. a constant name, add `__` as a prefix
and `_guard` as the suffix.

[ Gary says:

   "Here's the simplified example:

    ```
    macro_rules! f {
        () => {
            let a = 1;
            let _: u32 = a;
        }
    }

    const a: u64 = 1;

    fn main() {
        f!();
    }
    ```

    The `a` in `f` have a different hygiene so normally it is scoped to the
    macro expansion and wouldn't escape. Interestingly a constant is still
    preferred despite the hygiene so constants escaped into the macro,
    leading to the error."

  - Miguel ]

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/rust-for-linux/1e8a2a1f-abbf-44ba-8344-705a9cbb1627@proton.me/ [1]
Link: https://lore.kernel.org/r/20240403194321.88716-1-benno.lossin@proton.me
[ Added Benno's link and Gary's simplified example. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 9218cf826f1dbacbb857e6eabfae164d8ba05dea)
@jubalh @bonzini
rust: kernel: fix typos in code comments
e360d87 
Fix spelling mistakes in code comments.

Signed-off-by: Michael Vetter <jubalh@iodoru.org>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240819205731.2163-1-jubalh@iodoru.org
[ Reworded slightly. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 0ff8f3f0979559b0d7494d580f2597beab3f159b)
@BennoLossin @bonzini
rust: init: add write_[pin_]init functions
f3197bb 
Sometimes it is necessary to split allocation and initialization into
two steps. One such situation is when reusing existing allocations
obtained via `Box::drop_contents`. See [1] for an example.

In order to support this use case add `write_[pin_]init` functions to the
pin-init API. These functions operate on already allocated smart
pointers that wrap `MaybeUninit<T>`.

Link: https://lore.kernel.org/rust-for-linux/f026532f-8594-4f18-9aa5-57ad3f5bc592@proton.me/ [1]
Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/r/20240819112415.99810-2-benno.lossin@proton.me
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 6d1c22d0ace31d096b0dab5318c6a0d3219d6456)
@BennoLossin @bonzini
rust: init: add assert_pinned macro
aba4422 
Add a macro to statically check if a field of a struct is marked with
`#[pin]` ie that it is structurally pinned. This can be used when
`unsafe` code needs to rely on fields being structurally pinned.

The macro has a special "inline" mode for the case where the type
depends on generic parameters from the surrounding scope.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Co-developed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20240814-linked-list-v5-1-f5f5e8075da0@google.com
[ Replaced `compile_fail` with `ignore` and a TODO note. Removed
  `pub` from example to clean `unreachable_pub` lint. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 0528ca0a4f858da3369d405af8c76b8248dfeb7b)
@bonzini bonzini requested a review from BennoLossin November 22, 2024 13:23
@BennoLossin BennoLossin merged commit 1769d7f into Rust-for-Linux:main Nov 22, 2024
@BennoLossin
Copy link
Copy Markdown
Member

BennoLossin commented Nov 22, 2024

thanks a lot! this was really helpful!

@bonzini bonzini deleted the cleanup-pre-stable branch December 2, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BennoLossin BennoLossin BennoLossin approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bonzini @BennoLossin @jubalh