Received-SPF: pass (google.com: domain of linux-kernel+bounces-134854-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.117.160 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.117.160; helo=mail.nvidia.com; pr=C
Date: Mon, 8 Apr 2024 09:47:38 +0300
From: Zhi Wang <zhiw@nvidia.com>
To: Wedson Almeida Filho <wedsonaf@gmail.com>
CC: <rust-for-linux@vger.kernel.org>, Miguel Ojeda <ojeda@kernel.org>, "Alex
 Gaynor" <alex.gaynor@gmail.com>, Boqun Feng <boqun.feng@gmail.com>, Gary Guo
	<gary@garyguo.net>, =?UTF-8?Q?Bj=C3=B6rn?= Roy Baron
	<bjorn3_gh@protonmail.com>, Benno Lossin <benno.lossin@proton.me>, "Andreas
 Hindborg" <a.hindborg@samsung.com>, Alice Ryhl <aliceryhl@google.com>,
	<linux-kernel@vger.kernel.org>, Wedson Almeida Filho
	<walmeida@microsoft.com>, John Hubbard <jhubbard@nvidia.com>, Neo Jia
	<cjia@nvidia.com>, Andy Currid <acurrid@nvidia.com>
Subject: Re: [PATCH v3 00/10] Allocation APIs
Message-ID: <20240408094738.00005e59.zhiw@nvidia.com>
In-Reply-To: <20240328013603.206764-1-wedsonaf@gmail.com>
References: <20240328013603.206764-1-wedsonaf@gmail.com>
Organization: NVIDIA
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2024 06:47:56.1013
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: d81087c0-40f8-4919-289f-08dc5797d27b
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.160];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource:
	CH1PEPF0000A345.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB6543

On Wed, 27 Mar 2024 22:35:53 -0300
Wedson Almeida Filho <wedsonaf@gmail.com> wrote:

> From: Wedson Almeida Filho <walmeida@microsoft.com>
> 
> Revamp how we use the `alloc` crate.
> 
> We currently have a fork of the crate with changes to `Vec`; other
> changes have been upstreamed (to the Rust project). This series
> removes the fork and exposes all the functionality as extension
> traits.
> 
> Additionally, it also introduces allocation flag parameters to all
> functions that may result in allocations (e.g., `Box::new`,
> `Arc::new`, `Vec::push`, etc.) without the `try_` prefix -- the names
> are available because we build `alloc` with `no_global_oom_handling`.
> 

IMO, It seems the allocation flag here means GFP flags according to
Patch 5 and I understand the benefit of introducing the flags.

I am interested in the future plan. With this change, will Vec and Box
stick to kernel memory application APIs with GFP flags in the future?
e.g. GUP, kmalloc, those mostly allocates continuous memory for small
objects. Is that the future for Vec and Box in kernel?

Is there any plan for having vmalloc() in rust kernel? Currently, if I
push a large object into a Vec, kernel MM will complain for sure. And
literally Vec/Box themselves don't imply to the user that they allocate
memory with limitations.

Kernel uses different MM alloc APIs for different usages. For rust,
should we have a different kind of "Vec/Box" or having a Vec/Box with
different set of allocation flags that covers both GFP MM APIs and
vmalloc()? I am curious about the plan.

> Lastly, the series also removes our reliance on the `allocator_api`
> unstable feature.
> 
> Long term, we still want to make such functionality available in
> upstream Rust, but this allows us to make progress now and reduces our
> maintainance burden.
> 
> In summary:
> 1. Removes `alloc` fork
> 2. Removes use of `allocator_api` unstable feature
> 3. Introduces flags (e.g., GFP_KERNEL, GFP_ATOMIC) when allocating
> 
> ---
> 
> Changes in v3:
> - Rebased on top of the latest `rust-next` branch.
> - Updated `krealloc_aligned` to use `Flags` instead of
> `bindings::gfp_t`.
> - Added __GFP_ZERO to flags, as part of the previous change.
> - Avoiding temporary stack value in `Box::new_uninit`.
> - Implement `Box::new` using `Box::new_uninit` (so only one of them
> actually allocates).
> - Added examples/tests to `VecExt` methods.
> - Fixed bug in length in `extend_from_slice`
> - Link to v2:
> https://lore.kernel.org/rust-for-linux/20240327023531.187880-1-wedsonaf@gmail.com/T/#t
> 
> Changes in v2:
> - Updated description of `alloc` crate.
> - Renamed vecext and boxext modules to vec_ext and box_ext.
> - Added derive directive to `AllocError`.
> - Updated safety comment in `BoxExt::new`.
> - Updated `VecExt::push` and `VecExt::extend_from_slice` to use
>   `spare_capacity_mut`
> - Added directive to not compile `destructure` and `rebuild` when
> `test` or `testlib` are configured. Otherwise we have a warning
> because `push` and `extend_from_slice` don't use them anymore.
> - Updated indentation in `Arc::new_uninit`
> - Moved the removal of `TryReserveError` convesion to `Error` to
> patch 7, where usage of `TryReserveError` is actually removed.
> - Link to v1:
> https://lore.kernel.org/rust-for-linux/20240325195418.166013-1-wedsonaf@gmail.com/T/#t
> 
> Wedson Almeida Filho (10):
>   rust: kernel: move `allocator` module under `alloc`
>   rust: alloc: introduce the `VecExt` trait
>   kbuild: use the upstream `alloc` crate
>   rust: alloc: remove our fork of the `alloc` crate
>   rust: alloc: introduce allocation flags
>   rust: alloc: introduce the `BoxExt` trait
>   rust: alloc: update `VecExt` to take allocation flags
>   rust: sync: update `Arc` and `UniqueArc` to take allocation flags
>   rust: init: update `init` module to take allocation flags
>   rust: kernel: remove usage of `allocator_api` unstable feature
> 
>  rust/Makefile                        |   16 +-
>  rust/alloc/README.md                 |   36 -
>  rust/alloc/alloc.rs                  |  452 ----
>  rust/alloc/boxed.rs                  | 2463 -----------------
>  rust/alloc/collections/mod.rs        |  160 --
>  rust/alloc/lib.rs                    |  288 --
>  rust/alloc/raw_vec.rs                |  611 -----
>  rust/alloc/slice.rs                  |  890 -------
>  rust/alloc/vec/drain.rs              |  255 --
>  rust/alloc/vec/extract_if.rs         |  115 -
>  rust/alloc/vec/into_iter.rs          |  454 ----
>  rust/alloc/vec/is_zero.rs            |  204 --
>  rust/alloc/vec/mod.rs                | 3683
> -------------------------- rust/alloc/vec/partial_eq.rs         |
> 49 - rust/alloc/vec/set_len_on_drop.rs    |   35 -
>  rust/alloc/vec/spec_extend.rs        |  119 -
>  rust/bindings/bindings_helper.h      |    3 +
>  rust/kernel/alloc.rs                 |   74 +
>  rust/kernel/{ => alloc}/allocator.rs |   17 +-
>  rust/kernel/alloc/box_ext.rs         |   59 +
>  rust/kernel/alloc/vec_ext.rs         |  176 ++
>  rust/kernel/error.rs                 |   13 +-
>  rust/kernel/init.rs                  |   57 +-
>  rust/kernel/lib.rs                   |    5 +-
>  rust/kernel/prelude.rs               |    2 +
>  rust/kernel/str.rs                   |    6 +-
>  rust/kernel/sync/arc.rs              |   50 +-
>  rust/kernel/sync/condvar.rs          |    2 +-
>  rust/kernel/sync/lock/mutex.rs       |    2 +-
>  rust/kernel/sync/lock/spinlock.rs    |    2 +-
>  rust/kernel/types.rs                 |    4 +-
>  rust/kernel/workqueue.rs             |   14 +-
>  samples/rust/rust_minimal.rs         |    6 +-
>  samples/rust/rust_print.rs           |    4 +-
>  scripts/generate_rust_analyzer.py    |    2 +-
>  35 files changed, 405 insertions(+), 9923 deletions(-)
>  delete mode 100644 rust/alloc/README.md
>  delete mode 100644 rust/alloc/alloc.rs
>  delete mode 100644 rust/alloc/boxed.rs
>  delete mode 100644 rust/alloc/collections/mod.rs
>  delete mode 100644 rust/alloc/lib.rs
>  delete mode 100644 rust/alloc/raw_vec.rs
>  delete mode 100644 rust/alloc/slice.rs
>  delete mode 100644 rust/alloc/vec/drain.rs
>  delete mode 100644 rust/alloc/vec/extract_if.rs
>  delete mode 100644 rust/alloc/vec/into_iter.rs
>  delete mode 100644 rust/alloc/vec/is_zero.rs
>  delete mode 100644 rust/alloc/vec/mod.rs
>  delete mode 100644 rust/alloc/vec/partial_eq.rs
>  delete mode 100644 rust/alloc/vec/set_len_on_drop.rs
>  delete mode 100644 rust/alloc/vec/spec_extend.rs
>  create mode 100644 rust/kernel/alloc.rs
>  rename rust/kernel/{ => alloc}/allocator.rs (86%)
>  create mode 100644 rust/kernel/alloc/box_ext.rs
>  create mode 100644 rust/kernel/alloc/vec_ext.rs
> 
> 
> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db