So arguably both camps are correct. Those who advocate Rust rewrites, and those who are against it too.
I would presume the ratio of safe to unsafe code leads to less unsafe code being written over time as the full ”kernel standard library” gets built out allowing all other parts to replace their hand rolled implementations with the standard one.
I'm sure the people who wrote this code had their reasons to write the code like this (probably simplicity or performance), but this type of kernel code could be done safely.
> The previous patches in this series illustrate why the List::remove method is really dangerous. I think the real takeaway here is to replace the linked lists with a different data structure without this unsafe footgun, but for now we fix the bugs and add a warning to the docs.
Then the safety comment can easily bias the reader into believing that the author has fully understood the problem and all edge cases.
The fact that this survived review is the worrying part. Unsafe blocks are intentionally small and localized in Rust precisely so the safety argument can be checked. If the stated safety argument is incomplete and still passes review, that suggests reviewers are relying on the comment as the proof, rather than rederiving the invariants themselves. Unless of course the wrong people are reviewing these changes. Why rewrite in Rust if we don't apply extreme scrutiny to the tiny subset (presumably) that should be scrutinized.
To be clear, I think this is a failure of process, not Rust of course.
I think the safety comment might have been more on-point than you think. If you look at the original code, it did something like:
- Take a lock - Swap a `Node`'s `death_list` (i.e., a list of `NodeDeath`s) with an empty one - Release the lock - Iterate over the taken `death_list`
While in another thread, you have a `NodeDeath`:
- Take a lock - Get its parent's `death_list` - Remove itself from said list. - Release the lock
The issue is what happens when a `NodeDeath` from the original list tries to remove itself after the parent Node swapped its `death_list`. In that case, the `NodeDeath` grabs the replacement list from its parent node, and the subsequent attempt to remove itself from the replacement list violates the precondition in the safety comment.
> Why rewrite in Rust if we don't apply extreme scrutiny to the tiny subset (presumably) that should be scrutinized.
That "extreme scrutiny" was applied does not guarantee that all possible bugs will be found. Humans are only human, after all.
That's roughly 100% of unsafe code because a lint in the compiler asks for it.