But the total footprint of the interfaces that typical drivers use is fairly small. When a rusty shim is provided that provides access to those interfaces, you can write pure rust drivers.
But the total footprint of the interfaces that typical drivers use is fairly small. When a rusty shim is provided that provides access to those interfaces, you can write pure rust drivers.
For example look at typical audio codec drivers. They barely have any procedural code (and when they do it's just some form of register value mapping anyway), locking/concurency and almost all resource management is handled at the higher level and the driver is not exposed to it.
Another kind of a driver that features a very limited dependency on Linux interfaces is a SoC clock/reset driver. And clock drivers are even more boring in this aspect. Code vs data ratio is probably < 0.1 in many of those.
I guess some drivers that do more complicated stuff, and deal with concurrency issues or more complicated resource management may benefit, but they'll likely need to touch much larger API surface, too.
And I dunno, even the sample driver in the patchset has things like this:
let mut data = kernel::new_device_data!(
gpio::RegistrationWithIrqChip::new(),
PL061Resources {
// SAFETY: This device doesn't support DMA.
base: unsafe { IoMem::try_new(res)? },
parent_irq: irq,
},
// SAFETY: We call `spinlock_init` below.
unsafe { SpinLock::new(PL061Data::default()) },
"PL061::Registrations"
)?;
// SAFETY: General part of the data is pinned when `data` is.
let gen = unsafe { data.as_mut().map_unchecked_mut(|d| &mut **d) };
kernel::spinlock_init!(gen, "PL061::General");
let data = Ref::<DeviceData>::from(data);
It's barely readable. I don't know what type `data` is, what `gen` is or how it's different from `data`. Variable names don't really help. `data.as_mut().map_unchecked_mut(|d| &mut *d)` also looks like some kind of WTF to the uninitiated, that is somehow retrieving some 'general part of data' whatever that is.When I look at the kernel's spinlock type https://elixir.bootlin.com/linux/latest/source/include/linux... that doesn't really help me determine why whatever this `gen` [variable?] is needs to be passed to `spinlock_init`.
And what's with the two `let data`. Are they the same type or is the type now different and the first data variable now hidden? Because it looks like second `let data` gets somehow transformed from the first `data` into something else. So now we have two identically named `data` variables with different types in the same function?
Wasn't this supposed to be some simpler language to draw more people to the kernel development? At least with C each variable has a type that's quickly identifiable.
I guess after 15 minutes I see what this is trying to do. It passes some newly created spinlock object to `new_device_data` function and then calls `spinlock_init` on some part of the data returned from `new_device_data` that needs to be extracted from it via some really obtuse transformation, just to initialize the spinlock later instead of directly in `SpinLock::new` for some obscure reason, using two unsafe {} blocks in the process. But oh my god, if this kind of obfuscation is going to be the new standard for the kernel,... At least C is the devil I know. :)
If you want to practice, check out https://github.com/immunant/ioq3/blob/transpiled/quake3-rs/q... or whatever.
You are judging Rust by looking at what idiomatic C code looks like in Rust.
That said, idiomatic Rust is difficult for me to read/understand as well. I do not expect device drivers in Rust to be readable (by me), and if they have lots of unsafe blocks then we are back to square one I would say.
pub static mut g_color_table: [crate::src::qcommon::q_shared::vec4_t; 8] = [
[0f32, 0f32, 0f32, 1f32],
[1f32, 0f32, 0f32, 1f32],
[0f32, 1f32, 0f32, 1f32],
[1f32, 1f32, 0f32, 1f32],
[0f32, 0f32, 1f32, 1f32],
[0f32, 1f32, 1f32, 1f32],
[1f32, 0f32, 1f32, 1f32],
[1f32, 1f32, 1f32, 1f32],
];
would maybe look like pub static mut g_color_table: [[f32; 4]; 8] = [
[0, 0, 0, 1],
[1, 0, 0, 1],
[0, 1, 0, 1],
[1, 1, 0, 1],
[0, 0, 1, 1],
[0, 1, 1, 1],
[1, 0, 1, 1],
[1, 1, 1, 1],
];
but it still looks weird to me (a static mut would be uncommon in a design, and not having a new-type to represent these would also be uncommon).If you are interested in looking what Rust written from the ground up in the same domain looks like, you can maybe look at Bevy[1].
The biggest source of "noise" in Rust is explicit type conversions, FP constructs and longer types (`.into()`/`.as_mut_ref()`/`.map()`/`Result<Option<T>, E>`).
[1]: https://github.com/bevyengine/bevy/blob/main/crates/bevy_ren...
BufferInfo {
buffer_usage: BufferUsage::VERTEX,
..Default::default()
},
What is "..Default" here? impl<T> Default for Option<T> {
fn default() -> Option<T> {
None
}
}
If every field of a struct impl Default, then you can you can use the `#[derive(Default)]` annotation to auto-fill that impl for that struct, otherwise you have to write it by hand.The .. is the spread operator, and it takes every field of the value on the right and applies it to every field of the struct constructor on the left. This only works for the same type (it's not only looking at the field names).
`Default::default()` is a qualified call on the trait equivalent to `<_ as Default>::default()`, where `_` is inferred from context to be `BufferInfo`.
What that code is doing is saying "build a default BufferInfo with field buffer_usage set to VERTEX".
The spread operator has a limitation you might have intuited from the description that it can't work for types where some field(s) should always be set but not others. To get around that you can use the builder pattern
BufferInfo::with_buffer_usage(BufferUsage::VERTEX).build()
and there's a pre-RFC in flight to become an RFC to add support for struct Foo {
field: i32 = 42,
}
let foo = Foo { .. };
[1]: https://doc.rust-lang.org/std/default/trait.Default.htmlSpecifically, when you initialise a structure like this, the .. is "struct update syntax" and is followed by an instance of that structure from which remaining fields will be copied. Default is a Rust trait (similar to an interface if you've never seen Rust traits before) and it has the method default() which gives you a default of whatever type.
Rust has type inference, if you don't specify a type, but there is only one possibility that could work, the type inference might (and in this case does) see that and you needn't bother laboriously writing down the type. This can only happen where there was no possible ambiguity. In this case, the only type you can use for struct update syntax is the structure type you're updating of course, BufferInfo, and so Default::default() must mean the default BufferInfo.
If you found the definition of BufferInfo it probably says #[derive(Default)] which means, using a macro just implement Default for this structure in the obvious way, although it might actually have an explicit Default written out instead.
Where do you see hacks being used in, say, the refactored code? If you meant generally, then sure, they do.
// SAFETY: This device doesn't support DMA.
let base = unsafe { IoMem::try_new(res)? };
let mut data = new_device_data!(
RegistrationWithIrqChip::new(),
PL061Resources { base, parent_irq: irq },
// SAFETY: We call `spinlock_init` below.
unsafe { SpinLock::new(PL061Data::default()) },
"PL061::Registrations"
)?;
// SAFETY: General part of the data is pinned when `data` is.
let gen = unsafe { data.as_mut().map_unchecked_mut(|d| &mut **d) };
spinlock_init!(gen, "PL061::General");
let data: Ref<DeviceData> = data.into();
I imagine code like this would be the body of a small function that does all this unsafe preparations and gives back a Result<Ref<DeviceData>, Error> once it's done, which would change the end to be Ok(data.into())Most Rust is written with the implicit assumption that it will be read in an IDE that displays types, so explicit type annotations are usually kept to a minimum. Doesn't always translate well outside of the IDE, so I wouldn't be surprised if the consensus for kernel code shifts to more annotations.
> When I look at the kernel's spinlock type [...] that doesn't really help me determine why whatever this `gen` [variable?] is needs to be passed to `spinlock_init`
The rust version of the documentation makes an attempt at explaining it, though honestly it could be a lot clearer https://rust-for-linux.github.io/docs/kernel/sync/struct.Spi...
> And what's with the two `let data`.
That's pretty ideomatic in rust, probably borrowed from functional languages. The new definition hides the previous one, and the implication behind reusing the name is that it's fundamentally the same data, just with a different type, or different mutability (here it was converted from mutable DeviceData to an immutable Ref with the same DeviceData).
Now, some of that is just a matter of learning curve; once you have learned the Rust-in-Linux abstractions the above will probably be somewhat more readable.
Coming from someone who's more familiar with Rust, and less so with the kernel, here are a few notes.
* It might not be the best idea to post the example drivers with inferred types, for this familiarization reason. While there are a few types (such as those involving closures) that you can't write and can only use via type inference, or others that are too large to want to write out, these types should be fairly easy to write and it would improve readability during the familiarization process
* A lot of these things that you were having trouble with look like they are basically just dereferencing and accessing some data contained within the spinlock, and casting what is only guaranteed by the API to be an immutable (shared) reference into a mutable (unique) reference in an `unsafe` block. This is a place where trying to use simple Rust wrappers over core kernel structures which weren't designed for Rust makes things a bit more difficult to read, as you need to do a bit of an unidiomatic dance to tell the compiler "yes, the language and type don't gurantee uniqueness, but I promise this reference is unique and I won't misuse it."
* One of the small little humps to get over when learning Rust is to learn that "mut" actually means "unique", and unmarked (immutable) references means "potentially shared." But since you do need to sometimes write to objects that are potentially shared, it just means that you need to either guarantee yourself or use some kind of synchronization mechanism to ensure that at any given moment of time that data is only mutable in one place at a time.
So, I don't know the details of these APIs myself, but when I look at this here's what I see
* we're constructing some device data object protected by a spinlock
* we're providing some brief justification for why this particular usage is safe, since the API and compiler can't guarantee that it's safe
* we're extracting a mutable reference from it, for which we need to uphold the single exclusive access principle without assistance from the compiler
* we're wrapping that in a new reference to device data type which presumably provides a more idiomatic interface than what the raw primitives provide
I'm guessing that the following code which actually uses this Ref::<DeviceData> object will then be a bit more readable and require fewer uses of unsafe, as it looks like this code is what is providing that translation between some core kernel primitives which are not terribly idiomatic to use in Rust and a safer, more idiomatic wrapper.
a `cargo fmt` option to add inferred types to the source code would have some advantages for that.