[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <35e1977e-7cca-4ea3-9df8-0a2b43bc0f85@foxido.dev>
Date: Thu, 13 Nov 2025 13:06:42 +0300
From: Gladyshev Ilya <foxido@...ido.dev>
To: dsterba@...e.cz
Cc: Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>,
linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 4/8] btrfs: simplify function protections with guards
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
Powered by blists - more mailing lists