Message-ID: <20230419133800.0da0857a.gary@garyguo.net>
Date:   Wed, 19 Apr 2023 13:38:00 +0100
From:   Gary Guo <gary@...yguo.net>
To:     Miguel Ojeda <ojeda@...nel.org>
Cc:     Wedson Almeida Filho <wedsonaf@...il.com>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        Benno Lossin <benno.lossin@...ton.me>,
        Josh Stone <jistone@...hat.com>,
        William Brown <william.brown@...e.com>,
        Georgy Yakovlev <gyakovlev@...too.org>,
        Jan Alexander Steffens <jan.steffens@...il.com>,
        rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
        patches@...ts.linux.dev
Subject: Re: [PATCH 3/3] rust: upgrade to Rust 1.68.2

On Tue, 18 Apr 2023 23:43:47 +0200
Miguel Ojeda <ojeda@...nel.org> wrote:

> This is the first upgrade to the Rust toolchain since the initial Rust
> merge, from 1.62.0 to 1.68.2 (i.e. the latest).
> 
> # Context
> 
> The kernel currently supports only a single Rust version [1] (rather
> than a minimum) given our usage of some "unstable" Rust features [2]
> which do not promise backwards compatibility.
> 
> The goal is to reach a point where we can declare a minimum version for
> the toolchain. For instance, by waiting for some of the features to be
> stabilized. Therefore, the first minimum Rust version that the kernel
> will support is "in the future".
> 
> # Upgrade policy
> 
> Given we will eventually need to reach that minimum version, it would be
> ideal to upgrade the compiler from time to time to be as close as
> possible to that goal and find any issues sooner. In the extreme, we
> could upgrade as soon as a new Rust release is out. Of course, upgrading
> so often is in stark contrast to what one normally would need for GCC
> and LLVM, especially given the release schedule: 6 weeks for Rust vs.
> half a year for LLVM and a year for GCC.
> 
> Having said that, there is no particular advantage to updating slowly
> either: kernel developers in "stable" distributions are unlikely to be
> able to use their distribution-provided Rust toolchain for the kernel
> anyway [3]. Instead, by routinely upgrading to the latest instead,
> kernel developers using Linux distributions that track the latest Rust
> release may be able to use those rather than Rust-provided ones,
> especially if their package manager allows to pin / hold back /
> downgrade the version for some days during windows where the version may
> not match. For instance, Arch, Fedora, Gentoo and openSUSE all provide
> and track the latest version of Rust as they get released every 6 weeks.
> 
> Then, when the minimum version is reached, we will stop upgrading and
> decide how wide the window of support will be. For instance, a year of
> Rust versions. We will probably want to start small, and then widen it
> over time, just like the kernel did originally for LLVM, see commit
> 3519c4d6e08e ("Documentation: add minimum clang/llvm version").
> 
> # Unstable features stabilized
> 
> This upgrade allows us to remove the following unstable features since
> they were stabilized:
> 
>   - `feature(explicit_generic_args_with_impl_trait)` (1.63).
>   - `feature(core_ffi_c)` (1.64).
>   - `feature(generic_associated_types)` (1.65).
>   - `feature(const_ptr_offset_from)` (1.65, *).
>   - `feature(bench_black_box)` (1.66, *).
>   - `feature(pin_macro)` (1.68).
> 
> The ones marked with `*` apply only to our old `rust` branch, not
> mainline yet, i.e. only for code that we may potentially upstream.
> 
> With this patch applied, the only unstable feature allowed to be used
> outside the `kernel` crate is `new_uninit`, though other code to be
> upstreamed may increase the list.
> 
> Please see [2] for details.
> 
> # Other required changes
> 
> Since 1.63, `rustdoc` triggers the `broken_intra_doc_links` lint for
> links pointing to exported (`#[macro_export]`) `macro_rules`. An issue
> was opened upstream [4], but it turns out it is intended behavior. For
> the moment, just add an explicit reference for each link. Later we can
> revisit this if `rustdoc` removes the compatibility measure.
> 
> Nevertheless, this was helpful to discover a link that was pointing to
> the wrong place unintentionally. Since that one was actually wrong, it
> is fixed in a previous commit independently.
> 
> Another change was the addition of `cfg(no_rc)` and `cfg(no_sync)` in
> upstream [5], thus remove our original changes for that.
> 
> Similarly, upstream now tests that it compiles successfully with
> `#[cfg(not(no_global_oom_handling))]` [6], which allow us to get rid
> of some changes, such as an `#[allow(dead_code)]`.
> 
> In addition, remove another `#[allow(dead_code)]` due to new uses
> within the standard library.
> 
> Finally, add `try_extend_trusted` and move the code in `spec_extend.rs`
> since upstream moved it for the infallible version.
> 
> # `alloc` upgrade and reviewing
> 
> There are a large amount of changes, but the vast majority of them are
> due to our `alloc` fork being upgraded at once.
> 
> There are two kinds of changes to be aware of: the ones coming from
> upstream, which we should follow as closely as possible, and the updates
> needed in our added fallible APIs to keep them matching the newer
> infallible APIs coming from upstream.
> 
> Instead of taking a look at the diff of this patch, an alternative
> approach is reviewing a diff of the changes between upstream `alloc` and
> the kernel's. This allows to easily inspect the kernel additions only,
> especially to check if the fallible methods we already have still match
> the infallible ones in the new version coming from upstream.
> 
> Another approach is reviewing the changes introduced in the additions in
> the kernel fork between the two versions. This is useful to spot
> potentially unintended changes to our additions.
> 
> To apply these approaches, one may follow steps similar to the following
> to generate a pair of patches that show the differences between upstream
> Rust and the kernel (for the subset of `alloc` we use) before and after
> applying this patch:
> 
>     # Get the difference with respect to the old version.
>     git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
>     git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
>         cut -d/ -f3- |
>         grep -Fv README.md |
>         xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
>     git -C linux diff --patch-with-stat --summary -R > old.patch
>     git -C linux restore rust/alloc
> 
>     # Apply this patch.
>     git -C linux am rust-upgrade.patch
> 
>     # Get the difference with respect to the new version.
>     git -C rust checkout $(linux/scripts/min-tool-version.sh rustc)
>     git -C linux ls-tree -r --name-only HEAD -- rust/alloc |
>         cut -d/ -f3- |
>         grep -Fv README.md |
>         xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH
>     git -C linux diff --patch-with-stat --summary -R > new.patch
>     git -C linux restore rust/alloc
> 
> Now one may check the `new.patch` to take a look at the additions (first
> approach) or at the difference between those two patches (second
> approach). For the latter, a side-by-side tool is recommended.
> 
> Link: https://rust-for-linux.com/rust-version-policy [1]
> Link: https://github.com/Rust-for-Linux/linux/issues/2 [2]
> Link: https://lore.kernel.org/rust-for-linux/CANiq72mT3bVDKdHgaea-6WiZazd8Mvurqmqegbe5JZxVyLR8Yg@mail.gmail.com/ [3]
> Link: https://github.com/rust-lang/rust/issues/106142 [4]
> Link: https://github.com/rust-lang/rust/pull/89891 [5]
> Link: https://github.com/rust-lang/rust/pull/98652 [6]
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>

I took this opportunity to re-review our diff with upstream alloc crate.

All the try_ series methods added to `Vec` and `RawVec` looks correct to
me.

Reviewed-by: Gary Guo <gary@...yguo.net>

> ---
>  Documentation/process/changes.rst |   2 +-
>  rust/alloc/alloc.rs               |  55 ++--
>  rust/alloc/boxed.rs               | 446 ++++++++++++++++++++++++++--
>  rust/alloc/collections/mod.rs     |   5 +-
>  rust/alloc/lib.rs                 |  71 +++--
>  rust/alloc/raw_vec.rs             |  16 +-
>  rust/alloc/slice.rs               | 447 ++++------------------------
>  rust/alloc/vec/drain.rs           |  81 +++++-
>  rust/alloc/vec/drain_filter.rs    |  60 +++-
>  rust/alloc/vec/into_iter.rs       | 125 ++++++--
>  rust/alloc/vec/is_zero.rs         |  96 ++++++-
>  rust/alloc/vec/mod.rs             | 464 +++++++++++++++++++++++-------
>  rust/alloc/vec/set_len_on_drop.rs |   5 +
>  rust/alloc/vec/spec_extend.rs     |  63 +---
>  rust/bindings/lib.rs              |   1 -
>  rust/kernel/build_assert.rs       |   2 +
>  rust/kernel/init.rs               |   5 +
>  rust/kernel/lib.rs                |   4 -
>  rust/kernel/std_vendor.rs         |   2 +
>  scripts/Makefile.build            |   2 +-
>  scripts/min-tool-version.sh       |   2 +-
>  21 files changed, 1274 insertions(+), 680 deletions(-)

Powered by blists - more mailing lists