fishinabarrel / linux-kernel-module-rust

Framework for writing Linux kernel modules in safe Rust
GNU General Public License v2.0
1.33k stars 119 forks source link

Add RCU read bindings (#143) and each_process() iterator #250

Open geofft opened 4 years ago

geofft commented 4 years ago

Add an RcuReadGuard object for calling functions that expect to be RCU-protected, and add a Rust equivalent of for_each_process and very light task_struct bindings sufficient to write a simple test/demo.

geofft commented 4 years ago

Obviously the test case needs work but I'd like some feedback on this design. I'm liking it, but at this point I've made myself familiar with how RCU works :)

geofft commented 4 years ago

Is there a good reference on RCUs to get up to speed?

There's an overview and some links in #143.

I'm about 80% sure that "crossbeam, but you guarantee epochs end because the kernel has precise control over scheduling" is a good tl;dr.

The basic idea is it's an extremely reader-optimized reader-writer lock pattern. A "writer" can't modify an RCU-protected structure directly. Instead, it makes a copy of the thing it wants to modify, updates that, atomically updates a pointer, waits for all readers of the old thing to be done, and then frees the old thing. If you know that readers exit quickly, then you can do this with almost no overhead on the reader side and without having an actual lock in memory to point to.

So it's mostly useful for protecting linked lists and other structures, like the list of processes in this example. It does not protect data that isn't behind a pointer, which means A) it's not a tool for multiple writers to coordinate (you want a real lock for that) and B) as in this example, actually accessing data inside a specific item in the linked list in a race-free way usually requires taking a real lock.

nelhage commented 4 years ago

@nelhage's comment leaves me a bit concerned that it's incredibly easy to accidentally violate the rules leading to deadlocks. Is that just life in C as well?

This is correct. The kernel rules about (a) when it's safe to schedule(), and (b) what operations will or won't schedule() are subtle and very easy to violate. And, of course, not uniformly documented, just a matter of oral tradition and lore and smatterings of references in different documents.

geofft commented 4 years ago

I think this is an equivalent problem to only calling async-signal-safe functions from a signal handler, or only calling kmalloc(GFP_NOWAIT) instead of kmalloc(GFP_KERNEL) in an interrupt. It seems like it's theoretically possible for Rust to enforce this by putting annotations on functions, I just don't know how.

geofft commented 4 years ago

https://github.com/japaric/cargo-call-stack might be some inspiration for "you can't call function Y from function X". It seems to work by parsing LLVM IR (it has its own parser) and constructing a call graph.

geofft commented 4 years ago

I think this is ready to land - any concerns about the current API?

alex commented 4 years ago

Only that I need to read all the RCU docs and think hard :-)

On Fri, Aug 21, 2020 at 10:33 AM Geoffrey Thomas notifications@github.com wrote:

I think this is ready to land - any concerns about the current API?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/fishinabarrel/linux-kernel-module-rust/pull/250#issuecomment-678322396, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBBCLNQJJZZ3LCUOH4LSB2AU7ANCNFSM4QBHQJIA .

-- All that is necessary for evil to succeed is for good people to do nothing.