Expand the complexity of the sample driver by providing the ability to
get and set an integer. The value is protected by a mutex.
Here is a simple userspace program that fully exercises the sample
driver's capabilities.
int main() {
int value, new_value;
int fd, ret;
// Open the device file
printf("Opening /dev/rust-misc-device for reading and writing\n");
fd = open("/dev/rust-misc-device", O_RDWR);
if (fd < 0) {
perror("open");
return errno;
}
// Make call into driver to say "hello"
printf("Calling Hello\n");
ret = ioctl(fd, RUST_MISC_DEV_HELLO, NULL);
if (ret < 0) {
perror("ioctl: Failed to call into Hello");
close(fd);
return errno;
}
// Get initial value
printf("Fetching initial value\n");
ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &value);
if (ret < 0) {
perror("ioctl: Failed to fetch the initial value");
close(fd);
return errno;
}
value++;
// Set value to something different
printf("Submitting new value (%d)\n", value);
ret = ioctl(fd, RUST_MISC_DEV_SET_VALUE, &value);
if (ret < 0) {
perror("ioctl: Failed to submit new value");
close(fd);
return errno;
}
// Ensure new value was applied
printf("Fetching new value\n");
ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &new_value);
if (ret < 0) {
perror("ioctl: Failed to fetch the new value");
close(fd);
return errno;
}
if (value != new_value) {
printf("Failed: Committed and retrieved values are different (%d - %d)\n", value, new_value);
close(fd);
return -1;
}
// Call the unsuccessful ioctl
printf("Attempting to call in to an non-existent IOCTL\n");
ret = ioctl(fd, RUST_MISC_DEV_FAIL, NULL);
if (ret < 0) {
perror("ioctl: Succeeded to fail - this was expected");
} else {
printf("ioctl: Failed to fail\n");
close(fd);
return -1;
}
// Close the device file
printf("Closing /dev/rust-misc-device\n");
close(fd);
printf("Success\n");
return 0;
}
And here is the output (manually spliced together):
USERSPACE: Opening /dev/rust-misc-device for reading and writing
KERNEL: rust_misc_device: Opening Rust Misc Device Sample
USERSPACE: Calling Hello
KERNEL: rust_misc_device: IOCTLing Rust Misc Device Sample
KERNEL: rust_misc_device: -> Hello from the Rust Misc Device
USERSPACE: Fetching initial value
KERNEL: rust_misc_device: IOCTLing Rust Misc Device Sample
KERNEL: rust_misc_device: -> Copying data to userspace (value: 0)
USERSPACE: Submitting new value (1)
KERNEL: rust_misc_device: IOCTLing Rust Misc Device Sample
KERNEL: rust_misc_device: -> Copying data from userspace (value: 1)
USERSPACE: Fetching new value
KERNEL: rust_misc_device: IOCTLing Rust Misc Device Sample
KERNEL: rust_misc_device: -> Copying data to userspace (value: 1)
USERSPACE: Attempting to call in to an non-existent IOCTL
KERNEL: rust_misc_device: IOCTLing Rust Misc Device Sample
KERNEL: rust_misc_device: -> IOCTL not recognised: 20992
USERSPACE: ioctl: Succeeded to fail - this was expected: Inappropriate ioctl for device
USERSPACE: Closing /dev/rust-misc-device
KERNEL: rust_misc_device: Exiting the Rust Misc Device Sample
USERSPACE: Success
Signed-off-by: Lee Jones <lee@kernel.org>
---
samples/rust/rust_misc_device.rs | 82 ++++++++++++++++++++++++++------
1 file changed, 67 insertions(+), 15 deletions(-)
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index c94b56c454fb..8145cf072640 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -4,14 +4,21 @@
//! Rust misc device sample.
+use core::pin::Pin;
+
use kernel::{
c_str,
- ioctl::_IO,
+ ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
+ new_mutex,
prelude::*,
+ sync::Mutex,
+ uaccess::{UserSlice, UserSliceReader, UserSliceWriter},
};
const RUST_MISC_DEV_HELLO: u32 = _IO('|' as u32, 0x80);
+const RUST_MISC_DEV_GET_VALUE: u32 = _IOR::<i32>('|' as u32, 0x81);
+const RUST_MISC_DEV_SET_VALUE: u32 = _IOW::<i32>('|' as u32, 0x82);
module! {
type: RustMiscDeviceModule,
@@ -41,39 +48,84 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
}
}
-struct RustMiscDevice;
+struct Inner {
+ value: i32,
+}
+
+#[pin_data(PinnedDrop)]
+struct RustMiscDevice {
+ #[pin]
+ inner: Mutex<Inner>,
+}
#[vtable]
impl MiscDevice for RustMiscDevice {
- type Ptr = KBox<Self>;
+ type Ptr = Pin<KBox<Self>>;
- fn open() -> Result<KBox<Self>> {
+ fn open() -> Result<Pin<KBox<Self>>> {
pr_info!("Opening Rust Misc Device Sample\n");
- Ok(KBox::new(RustMiscDevice, GFP_KERNEL)?)
+ KBox::try_pin_init(
+ try_pin_init! {
+ RustMiscDevice { inner <- new_mutex!( Inner{ value: 0_i32 } )}
+ },
+ GFP_KERNEL,
+ )
}
- fn ioctl(
- _device: <Self::Ptr as kernel::types::ForeignOwnable>::Borrowed<'_>,
- cmd: u32,
- _arg: usize,
- ) -> Result<isize> {
+ fn ioctl(device: Pin<&RustMiscDevice>, cmd: u32, arg: usize) -> Result<isize> {
pr_info!("IOCTLing Rust Misc Device Sample\n");
+ let size = _IOC_SIZE(cmd);
+
match cmd {
- RUST_MISC_DEV_HELLO => pr_info!("Hello from the Rust Misc Device\n"),
+ RUST_MISC_DEV_GET_VALUE => device.get_value(UserSlice::new(arg, size).writer())?,
+ RUST_MISC_DEV_SET_VALUE => device.set_value(UserSlice::new(arg, size).reader())?,
+ RUST_MISC_DEV_HELLO => device.hello()?,
_ => {
- pr_err!("IOCTL not recognised: {}\n", cmd);
+ pr_err!("-> IOCTL not recognised: {}\n", cmd);
return Err(ENOTTY);
}
- }
+ };
Ok(0)
}
}
-impl Drop for RustMiscDevice {
- fn drop(&mut self) {
+#[pinned_drop]
+impl PinnedDrop for RustMiscDevice {
+ fn drop(self: Pin<&mut Self>) {
pr_info!("Exiting the Rust Misc Device Sample\n");
}
}
+
+impl RustMiscDevice {
+ fn set_value(&self, mut reader: UserSliceReader) -> Result<isize> {
+ let new_value = reader.read::<i32>()?;
+ let mut guard = self.inner.lock();
+
+ pr_info!("-> Copying data from userspace (value: {})\n", new_value);
+
+ guard.value = new_value;
+ Ok(0)
+ }
+
+ fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
+ let guard = self.inner.lock();
+ let value = guard.value;
+
+ // Refrain from calling write() on a locked resource
+ drop(guard);
+
+ pr_info!("-> Copying data to userspace (value: {})\n", &value);
+
+ writer.write::<i32>(&value)?;
+ Ok(0)
+ }
+
+ fn hello(&self) -> Result<isize> {
+ pr_info!("-> Hello from the Rust Misc Device\n");
+
+ Ok(0)
+ }
+}
--
2.47.0.338.g60cca15819-googOn Fri, Dec 06, 2024 at 12:42:14PM +0000, Lee Jones wrote:
> Expand the complexity of the sample driver by providing the ability to
> get and set an integer. The value is protected by a mutex.
>
> Here is a simple userspace program that fully exercises the sample
> driver's capabilities.
nit, subject line should have "samples" not "sample" :)
> + fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
> + let guard = self.inner.lock();
> + let value = guard.value;
> +
> + // Refrain from calling write() on a locked resource
> + drop(guard);
> +
> + pr_info!("-> Copying data to userspace (value: {})\n", &value);
> +
> + writer.write::<i32>(&value)?;
> + Ok(0)
> + }
I don't understand why you have to drop the mutex before calling
pr_info() and write (i.e. copy_to_user())? It's a mutex, not a
spinlock, so you can hold it over that potentially-sleeping call, right?
Or is there some other reason why here?
thanks,
greg k-h
On Fri, 06 Dec 2024, Greg KH wrote:
> On Fri, Dec 06, 2024 at 12:42:14PM +0000, Lee Jones wrote:
> > Expand the complexity of the sample driver by providing the ability to
> > get and set an integer. The value is protected by a mutex.
> >
> > Here is a simple userspace program that fully exercises the sample
> > driver's capabilities.
>
> nit, subject line should have "samples" not "sample" :)
Ack.
> > + fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
> > + let guard = self.inner.lock();
> > + let value = guard.value;
> > +
> > + // Refrain from calling write() on a locked resource
> > + drop(guard);
> > +
> > + pr_info!("-> Copying data to userspace (value: {})\n", &value);
> > +
> > + writer.write::<i32>(&value)?;
> > + Ok(0)
> > + }
>
> I don't understand why you have to drop the mutex before calling
> pr_info() and write (i.e. copy_to_user())? It's a mutex, not a
> spinlock, so you can hold it over that potentially-sleeping call, right?
> Or is there some other reason why here?
This was a request from Alice to demonstrate how to unlock a mutex.
--
Lee Jones [李琼斯]
On Fri, 06 Dec 2024, Lee Jones wrote:
> On Fri, 06 Dec 2024, Greg KH wrote:
>
> > On Fri, Dec 06, 2024 at 12:42:14PM +0000, Lee Jones wrote:
> > > Expand the complexity of the sample driver by providing the ability to
> > > get and set an integer. The value is protected by a mutex.
> > >
> > > Here is a simple userspace program that fully exercises the sample
> > > driver's capabilities.
> >
> > nit, subject line should have "samples" not "sample" :)
>
> Ack.
>
> > > + fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
> > > + let guard = self.inner.lock();
> > > + let value = guard.value;
> > > +
> > > + // Refrain from calling write() on a locked resource
> > > + drop(guard);
> > > +
> > > + pr_info!("-> Copying data to userspace (value: {})\n", &value);
> > > +
> > > + writer.write::<i32>(&value)?;
> > > + Ok(0)
> > > + }
> >
> > I don't understand why you have to drop the mutex before calling
> > pr_info() and write (i.e. copy_to_user())? It's a mutex, not a
> > spinlock, so you can hold it over that potentially-sleeping call, right?
> > Or is there some other reason why here?
>
> This was a request from Alice to demonstrate how to unlock a mutex.
It's common practice to apply guards only around the protected value.
Why would this be different?
--
Lee Jones [李琼斯]
On Fri, Dec 06, 2024 at 01:06:30PM +0000, Lee Jones wrote:
> On Fri, 06 Dec 2024, Lee Jones wrote:
> > On Fri, 06 Dec 2024, Greg KH wrote:
> > > On Fri, Dec 06, 2024 at 12:42:14PM +0000, Lee Jones wrote:
> > > > + fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
> > > > + let guard = self.inner.lock();
> > > > + let value = guard.value;
> > > > +
> > > > + // Refrain from calling write() on a locked resource
> > > > + drop(guard);
> > > > +
> > > > + pr_info!("-> Copying data to userspace (value: {})\n", &value);
> > > > +
> > > > + writer.write::<i32>(&value)?;
> > > > + Ok(0)
> > > > + }
> > >
> > > I don't understand why you have to drop the mutex before calling
> > > pr_info() and write (i.e. copy_to_user())? It's a mutex, not a
> > > spinlock, so you can hold it over that potentially-sleeping call, right?
> > > Or is there some other reason why here?
> >
> > This was a request from Alice to demonstrate how to unlock a mutex.
>
> It's common practice to apply guards only around the protected value.
>
> Why would this be different?
It isn't, it's just that you are implying that the guard has to be
dropped because of the call to write(), which is confusing. It's only
"needed" because you want to guard a single cpu instruction that is
guaranteed atomic by the processor :)
As this is an example driver, documentation is essential, so maybe the
comment should be:
// Drop the mutex as we can now use our local copy
or something like that.
thanks,
greg k-h
On Fri, 06 Dec 2024, Greg KH wrote:
> On Fri, Dec 06, 2024 at 01:06:30PM +0000, Lee Jones wrote:
> > On Fri, 06 Dec 2024, Lee Jones wrote:
> > > On Fri, 06 Dec 2024, Greg KH wrote:
> > > > On Fri, Dec 06, 2024 at 12:42:14PM +0000, Lee Jones wrote:
> > > > > + fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
> > > > > + let guard = self.inner.lock();
> > > > > + let value = guard.value;
> > > > > +
> > > > > + // Refrain from calling write() on a locked resource
> > > > > + drop(guard);
> > > > > +
> > > > > + pr_info!("-> Copying data to userspace (value: {})\n", &value);
> > > > > +
> > > > > + writer.write::<i32>(&value)?;
> > > > > + Ok(0)
> > > > > + }
> > > >
> > > > I don't understand why you have to drop the mutex before calling
> > > > pr_info() and write (i.e. copy_to_user())? It's a mutex, not a
> > > > spinlock, so you can hold it over that potentially-sleeping call, right?
> > > > Or is there some other reason why here?
> > >
> > > This was a request from Alice to demonstrate how to unlock a mutex.
> >
> > It's common practice to apply guards only around the protected value.
> >
> > Why would this be different?
>
> It isn't, it's just that you are implying that the guard has to be
> dropped because of the call to write(), which is confusing. It's only
> "needed" because you want to guard a single cpu instruction that is
> guaranteed atomic by the processor :)
>
> As this is an example driver, documentation is essential, so maybe the
> comment should be:
> // Drop the mutex as we can now use our local copy
> or something like that.
Sounds reasonable.
I've ran out of time this week. I'll take another peek next week.
--
Lee Jones [李琼斯]
© 2016 - 2025 Red Hat, Inc.