lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ