rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>
To: gregkh@linuxfoundation.org, rafael@kernel.org, ojeda@kernel.org,
	alex.gaynor@gmail.com, boqun.feng@gmail.com, gary@garyguo.net,
	bjorn3_gh@protonmail.com, benno.lossin@proton.me,
	aliceryhl@google.com, mcgrof@kernel.org, russ.weight@linux.dev,
	dakr@redhat.com, a.hindborg@kernel.org
Cc: Guilherme Giacomo Simoes <trintaeoitogc@gmail.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v3 0/1] rust: device: rename "Device::from_raw()"
Date: Mon, 30 Sep 2024 11:43:26 -0300	[thread overview]
Message-ID: <20240930144328.51098-1-trintaeoitogc@gmail.com> (raw)

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


Guilherme Giacomo Simoes (1):
  rust: device: rename "Device::from_raw()"

 rust/kernel/device.rs   | 2 +-
 rust/kernel/firmware.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 

Changes from v2:
 - Refactored commit message style

             reply	other threads:[~2024-09-30 14:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-30 14:43 Guilherme Giacomo Simoes [this message]
2024-09-30 14:43 ` [PATCH v3 1/1] rust: device: rename "Device::from_raw()" Guilherme Giacomo Simoes
2024-09-30 14:52   ` Greg KH
2024-09-30 14:57     ` Guilherme Giácomo Simões
2024-09-30 15:04       ` Greg KH
2024-09-30 16:39         ` Guilherme Giácomo Simões
2024-09-30 17:27           ` Miguel Ojeda
2024-09-30 15:05       ` Danilo Krummrich
2024-09-30 15:55       ` Alice Ryhl
2024-09-30 16:45         ` Guilherme Giácomo Simões
2024-09-30 15:13   ` Danilo Krummrich

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=20240930144328.51098-1-trintaeoitogc@gmail.com \
    --to=trintaeoitogc@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@redhat.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=russ.weight@linux.dev \
    --cc=rust-for-linux@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).