Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CI and sync with Linux 6.12 #23

Merged
merged 12 commits into from
Nov 22, 2024

Conversation

bonzini
Copy link
Contributor

@bonzini bonzini commented Nov 4, 2024

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
new_uninit is stable as of 1.82.0, there is no need to require it.

Signed-off-by: Paolo Bonzini <[email protected]>
Some of the error messages have changed in nightly Rust, since the
.stderr files were last checked.  Update everything.

Signed-off-by: Paolo Bonzini <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
@bonzini bonzini force-pushed the cleanup-pre-stable branch from 8dafd4c to b74dc46 Compare November 4, 2024 14:47
@bonzini
Copy link
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
@y86-dev
Copy link
Member

y86-dev 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
Contributor Author

bonzini commented Nov 7, 2024

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

@y86-dev
Copy link
Member

y86-dev 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
Contributor Author

bonzini commented Nov 7, 2024

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

@bonzini
Copy link
Contributor Author

bonzini commented Nov 19, 2024

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
Copy link
Member

@y86-dev y86-dev 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!

examples/error.rs Show resolved Hide resolved
tests/ring_buf.rs Outdated Show resolved Hide resolved
bonzini and others added 8 commits November 22, 2024 14:20
Use the same implementation for both examples/error.rs and
examples/pthread_mutex.rs.

Signed-off-by: Paolo Bonzini <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Tested-by: Andreas Hindborg <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
(cherry picked from commit dee1396a486cf2b6e7840322f6d104680649f2ff)
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 <[email protected]>
Reviewed-by: Boqun Feng <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Link: https://lore.kernel.org/rust-for-linux/[email protected]/ [1]
Link: https://lore.kernel.org/r/[email protected]
[ Added Benno's link and Gary's simplified example. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
(cherry picked from commit 9218cf826f1dbacbb857e6eabfae164d8ba05dea)
Fix spelling mistakes in code comments.

Signed-off-by: Michael Vetter <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Reviewed-by: Benno Lossin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[ Reworded slightly. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
(cherry picked from commit 0ff8f3f0979559b0d7494d580f2597beab3f159b)
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/[email protected]/ [1]
Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Boqun Feng <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
(cherry picked from commit 6d1c22d0ace31d096b0dab5318c6a0d3219d6456)
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 <[email protected]>
Co-developed-by: Alice Ryhl <[email protected]>
Signed-off-by: Alice Ryhl <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[ Replaced `compile_fail` with `ignore` and a TODO note. Removed
  `pub` from example to clean `unreachable_pub` lint. - Miguel ]
Signed-off-by: Miguel Ojeda <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
(cherry picked from commit 0528ca0a4f858da3369d405af8c76b8248dfeb7b)
@bonzini bonzini requested a review from y86-dev November 22, 2024 13:23
@y86-dev y86-dev merged commit 1769d7f into Rust-for-Linux:main Nov 22, 2024
14 checks passed
@y86-dev
Copy link
Member

y86-dev 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
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants