Re: [RFC PATCH 4/8] btrfs: simplify function protections with guards

[Gladyshev Ilya <foxido@foxido.dev>]

On 11/13/25 11:43, David Sterba wrote:
> On Wed, Nov 12, 2025 at 09:49:40PM +0300, Gladyshev Ilya wrote:
>> 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
I thought it would be optimized out by the compiler in the end, I will 
probably recheck this

>> 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