[RFC] Add Optional Safe-to-Use Flavours of Mutex/CondVar

[TheSven73]

Request For Comments Only
Note that I am NOT proposing any changes to Binder

Our Mutex and CondVar abstractions push a lot of Pin reasoning and unsafe blocks onto its users. This creates plenty of rope for less experienced driver writers to hang themselves, and is therefore not necessarily an improvement over C.

Try this one on for size:

linux/samples/rust/rust_miscdev.rs

Lines 40 to 56 in 5dd07d5

	impl SharedState {
	fn try_new() -> Result<Pin<Arc<Self>>> {
	let state = Arc::try_pin(Self {
	// SAFETY: `condvar_init!` is called below.
	state_changed: unsafe { CondVar::new() },
	// SAFETY: `mutex_init!` is called below.
	inner: unsafe { Mutex::new(SharedStateInner { token_count: 0 }) },
	})?;
	// SAFETY: `state_changed` is pinned behind `Pin<Arc>`.
	let state_changed = unsafe { Pin::new_unchecked(&state.state_changed) };
	kernel::condvar_init!(state_changed, "SharedState::state_changed");
	// SAFETY: `inner` is pinned behind `Pin<Arc>`.
	let inner = unsafe { Pin::new_unchecked(&state.inner) };
	kernel::mutex_init!(inner, "SharedState::inner");
	Ok(state)
	}
	}

The proposed Optional Safe-to-Use flavours reduce this complexity, without requiring code changes anywhere else:
(actually code complexity will be reduced further, as there is no longer a need to Pin the returned structure)

impl SharedState {
    fn try_new() -> Result<Arc<Self>> {
        let state = Arc::try_new(Self {
            state_changed: boxed_condvar!("SharedState::state_changed")?,
            inner: boxed_mutex!(SharedStateInner { token_count: 0 }, "SharedState::inner")?,
        })?;
        Ok(state)
    }
}

The downside here is, obviously, the extra heap allocations, heap dereferences and loss of cache locality. But we only guess these are problematic. How can we verify that in the real world?

Turns out that @wedsonaf's proposed Binder ping benchmark appears quite sensitive to Mutex performance, as witnessed by the fact that we can measure a regression when adding one extra function call overhead to Mutex::lock. See:
#346 (comment)
#346 (comment)

So I regression tested Wedson's benchmark on arm 32-bit (cortex-a9 + Raspberri Pi Zero) and... no apparent change in performance.

Perhaps we will see a performance regression on x86 or RISC-V? If so, are the safety gains worth it?

[ksquirrel]

Review of 0276fe9f96ff:

• ⚠️ This PR adds/deletes more than 500 lines.
• ✔️ Commit cdc5556: Looks fine!
• ✔️ Commit b4b5942: Looks fine!
• ⚠️ Commit 4d2dba4: The commit committer does not match the commit author.
• ❌ Commit 4d2dba4: The last line of the commit message must be a Signed-off-by and must match the submitter of the PR.
• ❌ Commit 4d2dba4: Invalid tag format.
• ⚠️ Commit 4579e90: The commit committer does not match the commit author.
• ❌ Commit 4579e90: The commit message needs to be at least 3 lines long: title, empty line, signature.
• ⚠️ Commit c53984e: The commit committer does not match the commit author.
• ❌ Commit c53984e: The commit message needs to be at least 3 lines long: title, empty line, signature.
• ⚠️ Commit 0276fe9: The commit committer does not match the commit author.
• ❌ Commit 0276fe9: The commit message needs to be at least 3 lines long: title, empty line, signature.

[nbdd0121]

I am currently investigating ways to improve ergonomics of my pin-init crate, maybe I should also do a POC that uses pin-init so we can compare the ergonomics/usability/performance side-by-side.

[nbdd0121]

This is what the code would look like with pin-init: nbdd0121@549b8e3

I think it looks pretty similar to boxed code except for some additional macros calls.

[TheSven73]

No consensus that this is worthwhile.