Re: [RFC PATCH 4/8] btrfs: simplify function protections with guards
From: Gladyshev Ilya
Date: Thu Nov 13 2025 - 05:06:52 EST
On 11/13/25 11:43, David Sterba wrote:
On Wed, Nov 12, 2025 at 09:49:40PM +0300, Gladyshev Ilya wrote:I thought it would be optimized out by the compiler in the end, I will probably recheck this
Replaces cases like
void foo() {
spin_lock(&lock);
... some code ...
spin_unlock(&lock)
}
with
void foo() {
guard(spinlock)(&lock);
... some code ...
}
While it doesn't has any measurable impact,
There is impact, as Daniel mentioned elsewhere, this also adds the
variable on stack. It's measuable on the stack meter, one variable is
"nothing" but combined wherever the guards are used can add up. We don't
mind adding variables where needed, occasional cleanups and stack
reduction is done. Here it's a systematic stack use increase, not a
reason to reject the guards but still something I cannot just brush off
it makes clear that whole
function body is protected under lock and removes future errors with
additional cleanup paths.
The pattern above is the one I find problematic the most, namely in
longer functions or evolved code. Using your example as starting point
a followup change adds code outside of the locked section:
void foo() {
spin_lock(&lock);
... some code ...
spin_unlock(&lock)
new code;
}
with
void foo() {
guard(spinlock)(&lock);
... some code ...
new code;
}
This will lead to longer critical sections or even incorrect code
regarding locking when new code must not run under lock.
The fix is to convert it to scoped locking, with indentation and code
churn to unrelated code to the new one.
Suggestions like refactoring to make minimal helpers and functions is
another unecessary churn and breaks code reading flow.
What if something like C++ unique_lock existed? So code like this would be possible:
void foo() {
GUARD = unique_lock(&spin);
if (a)
// No unlocking required -> it will be done automatically
return;
unlock(GUARD);
... unlocked code ...
// Guard undestands that it's unlocked and does nothing
return;
}
It has similar semantics to scoped_guard [however it has weaker protection -- goto from locked section can bypass `unlock` and hold lock until return], but it doesn't introduce diff noise correlated with indentations.
Another approach is allowing scoped_guards to have different indentation codestyle to avoid indentation of internal block [like goto labels for example].
However both of this approaches has their own downsides and are pure proposals