On 14/07/2023 19.13, Alice Ryhl wrote:
Asahi Lina <lina@xxxxxxxxxxxxx> writes:
Begone, lock classes!
As discussed in meetings/etc, we would really like to support implicit
lock class creation for Rust code. Right now, lock classes are created
using macros and passed around (similar to C). Unfortunately, Rust
macros don't look like Rust functions, which means adding lockdep to a
type is a breaking API change. This makes Rust mutex creation rather
ugly, with the new_mutex!() macro and friends.
Implicit lock classes have to be unique per instantiation code site.
Notably, with Rust generics and monomorphization, this is not the same
as unique per generated code instance. If this weren't the case, we
could use inline functions and asm!() magic to try to create lock
classes that have the right uniqueness semantics. But that doesn't work,
since it would create too many lock classes for the same actual lock
creation in the source code.
But Rust does have one trick we can use: it can track the caller
location (as file:line:column), across multiple functions. This works
using an implicit argument that gets passed around, which is exactly the
thing we do for lock classes. The tricky bit is that, while the value of
these Location objects has the semantics we want (unique value per
source code location), there is no guarantee that they are deduplicated
in memory.
So we use a hash table, and map Location values to lock classes. Et
voila, implicit lock class support!
This lets us clean up the Mutex & co APIs and make them look a lot more
Rust-like, but it also means we can now throw Lockdep into more APIs
without breaking the API. And so we can pull a neat trick: adding
Lockdep support into Arc<T>. This catches cases where the Arc Drop
implementation could create a locking correctness violation only when
the reference count drops to 0 at that particular drop site, which is
otherwise not detectable unless that condition actually happens at
runtime. Since Drop is "magic" in Rust and Drop codepaths very difficult
to audit, this helps a lot.
For the initial RFC, this implements the new API only for Mutex. If this
looks good, I can extend it to CondVar & friends in the next version.
This series also folds in a few related minor dependencies / changes
(like the pin_init mutex stuff).
I'm not convinced that this is the right compromise. Moving lockdep
class creation to runtime sounds unfortunate, especially since this
makes them fallible due to memory allocations (I think?).
I would be inclined to keep using macros for this.
Most people were very enthusiastic about this change in the meetings...
it wasn't even my own idea ^^
I don't think the fallibility is an issue. Lockdep is a debugging tool,
and it doesn't have to handle all possible circumstances perfectly. If
you are debugging normal lock issues you probably shouldn't be running
out of RAM, and if you are debugging OOM situations the lock keys would
normally have been created long before you reach an OOM situation, since
they would be created the first time a relevant lock class is used. More
objects of the same class don't cause any more allocations. And the code
has a fallback for the OOM case, where it just uses the Location object
as a static lock class. That's not ideal and degrades the quality of the
lockdep results, but it shouldn't completely break anything.
The advantages of being able to throw lockdep checking into arbitrary
types, like the Arc<T> thing, are pretty significant. It closes a major
correctness checking issue we have with Rust and its automagic Drop
implementations that are almost impossible to properly audit for
potential locking issues. I think that alone makes this worth it, even
if you don't use it for normal mutex creation...
~~ Lina
