All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Hui Zhu" <hui.zhu@linux.dev>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>, <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	<linux-kernel@vger.kernel.org>, <rust-for-linux@vger.kernel.org>,
	"Hui Zhu" <zhuhui@kylinos.cn>,
	"Geliang Tang" <geliang@kernel.org>,
	"Lorenzo Stoakes" <lorenzo.stoakes@oracle.com>,
	"Vlastimil Babka" <vbabka@suse.cz>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	"Uladzislau Rezki" <urezki@gmail.com>
Subject: Re: [PATCH v2] rust: add a sample alloc usage
Date: Thu, 17 Jul 2025 13:19:05 +0200	[thread overview]
Message-ID: <DBEAFCFHFU35.1IZI3ZSJSIRAY@kernel.org> (raw)
In-Reply-To: <20250717095053.49239-1-hui.zhu@linux.dev>

(Cc: Lorenzo, Vlastimil, Liam, Uladzislau)

On Thu Jul 17, 2025 at 11:50 AM CEST, Hui Zhu wrote:
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index bd2faad63b4f..7c3e68d9ada5 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_AUXILIARY)	+= rust_driver_auxiliary.o
>  obj-$(CONFIG_SAMPLE_RUST_CONFIGFS)		+= rust_configfs.o
> +obj-$(CONFIG_SAMPLE_RUST_ALLOC)		+= rust_alloc.o

I think adding an example for large alignment is fine, but let's do this in a
doc-test on VBox in rust/kernel/alloc/kbox.rs. I think adding a separate module
for this is overkill.

Note that doc-tests are executed on boot if CONFIG_RUST_KERNEL_DOCTESTS=y.

> diff --git a/samples/rust/rust_alloc.rs b/samples/rust/rust_alloc.rs
> new file mode 100644
> index 000000000000..61efde37b5f2
> --- /dev/null
> +++ b/samples/rust/rust_alloc.rs
> @@ -0,0 +1,88 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (c) 2025, Kylin Software
> +
> +//! Rust alloc sample.
> +
> +use kernel::bindings;
> +use kernel::prelude::*;
> +
> +module! {
> +    type: RustAlloc,
> +    name: "rust_alloc",
> +    authors: ["Rust for Linux Contributors"],
> +    description: "Rust alloc sample",
> +    license: "GPL",
> +}
> +
> +const VBOX_SIZE: usize = 1024;
> +const VBOX_LARGE_ALIGN: usize = bindings::PAGE_SIZE * 4;

Please use kernel::page::PAGE_SIZE instead.

> +const KVEC_VAL: [usize; 3] = [10, 20, 30];
> +
> +#[repr(align(128))]
> +struct VboxBlob([u8; VBOX_SIZE]);
> +
> +// This structure is used to test the allocation of alignments larger
> +// than PAGE_SIZE.
> +// Since this is not yet supported, avoid accessing the contents of
> +// the structure for now.
> +#[allow(dead_code)]
> +#[repr(align(8192))]
> +struct VboxLargeAlignBlob([u8; VBOX_LARGE_ALIGN]);
> +
> +struct RustAlloc {
> +    vbox_blob: VBox<VboxBlob>,
> +    kvec_blob: KVec<usize>,
> +}
> +
> +fn check_align(addr: usize, align: usize) -> bool {
> +    debug_assert!(align.is_power_of_two());
> +    if addr & (align - 1) != 0 {
> +        pr_err!("Address {:#x} is not aligned with {:#x}.\n", addr, align);
> +        false
> +    } else {
> +        true
> +    }
> +}

I think check_align() is unnecessary -- we don't need to test this in this
context.

Instead, I suggest to add some doc-tests to the `Allocator` impls for `Kmalloc`,
`Vmalloc` and `KVmalloc` in rust/kernel/alloc/allocator.rs testing that they
return the expected alignment. This would be a valuable test case for those. If
you're interested in adding them, please feel free to do so. :)

> +impl kernel::Module for RustAlloc {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        pr_info!("Rust allocator sample (init)\n");
> +
> +        let vbox_blob = VBox::<VboxBlob>::new_uninit(GFP_KERNEL)?;
> +        if !check_align(vbox_blob.as_ptr() as usize, 128) {
> +            return Err(EINVAL);
> +        }
> +        let vbox_blob = vbox_blob.write(VboxBlob([0xfeu8; VBOX_SIZE]));

Given that you initialize it with a value anyways, you can just do

	let blob = VBox::new(Blob([0xfeu8; SIZE]), GFP_KERNEL)?;

instead.

> +        if let Ok(_) = VBox::<VboxLargeAlignBlob>::new_uninit(GFP_KERNEL) {
> +            pr_err!("Allocations for VBox with alignments larger than PAGE_SIZE should fail, but here it succeeded.\n");
> +            return Err(EINVAL);
> +        }

This can just be

	assert!(VBox::<VboxLargeAlignBlob>::new_uninit(GFP_KERNEL).is_err());

within a doc-test.

> +        let mut kvec_blob = KVec::new();
> +        kvec_blob.extend_from_slice(&KVEC_VAL, GFP_KERNEL)?;

We already have doc-tests for KVec::new() and KVec::extend_from_slice() in
rust/kernel/alloc/kvec.rs.

> +        Ok(Self {
> +            vbox_blob,
> +            kvec_blob,
> +        })
> +    }
> +}
> +
> +impl Drop for RustAlloc {
> +    fn drop(&mut self) {
> +        pr_info!("Rust allocator sample (exit)\n");
> +
> +        // check the values
> +        for b in self.vbox_blob.0.as_slice().iter() {
> +            if *b != 0xfeu8 {
> +                pr_err!("vbox_blob contains wrong values\n");
> +            }
> +        }
> +
> +        if self.kvec_blob.as_slice() != KVEC_VAL {
> +            pr_err!("kvec_blob contains wrong values\n");
> +        }

Here you're basically testing Vec::as_slice(). We don't have doc-tests for
as_slice() and as_slice_mut() in rust/kernel/alloc/kvec.rs. Please feel free to
add them. :)

> +    }
> +}


  reply	other threads:[~2025-07-17 11:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-17  9:50 [PATCH v2] rust: add a sample alloc usage Hui Zhu
2025-07-17 11:19 ` Danilo Krummrich [this message]
2025-07-17 16:14   ` Boqun Feng
2025-07-24  8:06     ` Hui Zhu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DBEAFCFHFU35.1IZI3ZSJSIRAY@kernel.org \
    --to=dakr@kernel.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=gary@garyguo.net \
    --cc=geliang@kernel.org \
    --cc=hui.zhu@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=urezki@gmail.com \
    --cc=vbabka@suse.cz \
    --cc=zhuhui@kylinos.cn \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.