Message-ID: <CAM_RzfavjM9VPJhTi1czOSC0BFOc3FUHRMdzFrU9fJE+WJMKbw@mail.gmail.com>
Date: Sun, 29 Sep 2024 10:30:42 -0300
From: Guilherme Giácomo Simões <trintaeoitogc@...il.com>
To: rafael@...nel.org, ojeda@...nel.org, alex.gaynor@...il.com, 
	boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com, 
	benno.lossin@...ton.me, aliceryhl@...gle.com, mcgrof@...nel.org, 
	russ.weight@...ux.dev, dakr@...hat.com, a.hindborg@...nel.org, 
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
Cc: rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Guilherme Giácomo Simões <trintaeoitogc@...il.com>
Subject: [PATCH 1/2] device: rust: change the name function

Why did I make this change?
This function "Device::from_raw()" increments the refcount by this command
"bindings::get_device(prt)". This can be confused because the function
Arc::from_raw() from the standard library, doesn't increment the refcount.

This discussion is in
https://rust-for-linux.zulipchat.com/#narrow/stream/291566-Library/topic/Inconsistency.20of.20.60from_raw.60.2E

The options can be:
1) Rename the function for don't make confusing with the Arc::from_raw().
2) Remove this function and use the unsafe { Device::as_ref(ptr) }.into()
when I need the get pointer for the device.


Proposed solution
I like the first option. Because, how was will commented by Boqun Feng
, when the
people write the unsafe { Device::as_ref(ptr) }.into() again, again and again...
inevitably anybody will create a help function for this.

Then I think that we should rename this function for
Device::get_from_raw() or maybe
Device::get_device() and I like more of the second option because,
this will be equal
the get_device() function that already exists in .c code.


How do I test this:
I create this simple file in sample/rust/device.rs
""""""""
use kernel::device::Device;
use kernel::prelude::*;
use kernel::types::ARef;

module! {
    type: DeviceTest,
    name: "device_test",
    author: "Test device",
    description: "A simple module for test device",
    license: "GPL",
}

struct DeviceTest;

impl kernel::Module for DeviceTest {
    fn init(_module: &'static ThisModule) -> Result<Self> {
        pr_info!("initial device test");
        let device = create_and_get_device();
        pr_info!("device created");

        Ok(DeviceTest)
    }
}

impl Drop for DeviceTest {
    fn drop(&mut self) {
        pr_info!("bye bye driver test");
    }
}

fn create_and_get_device() -> ARef<Device> {
    let device = unsafe { Device::get_device(core::ptr::null_mut()) };
    device
}
""""""""

I set this in Kconfig
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b0f74a81c8f9..7779969e7dd6 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -37,4 +37,9 @@ config SAMPLE_RUST_HOSTPROGS

          If unsure, say N.

+config SAMPLE_DEVICE_TEST
+       tristate "Device test"
+       help
+               This option is for device test
+
 endif # SAMPLES_RUST


and in Makefile
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..85a8b30100e7 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -2,5 +2,6 @@

 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)              += rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)                        += rust_print.o
+obj-$(CONFIG_SAMPLE_DEVICE_TEST)                       += device.o

 subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)         += hostprogs


Then I enable this in menu config... compile the kernel e run this in a qemu
qemu-system-x86_64 -kernel bzImage -initrd initramfs.img -m 2G
-machine q35 -device ich9-ahci,id=sata -drive
id=disk,file=rootfs.img,if=none,format=raw -device
ide-hd,drive=disk,bus=sata.0 -append "root=/dev/sda console=ttyS0"
-nographic -monitor telnet:127.0.0.1:5555,server,nowai

the expected print is showing
[    2.786174] device_test: initial device test
[    2.786541] device_test: device created

Powered by blists - more mailing lists