Re: [PATCH v3 1/4] btrfs: make sure all ordered extents beyond EOF is properly truncated

Re: [PATCH v3 1/4] btrfs: make sure all ordered extents beyond EOF is properly truncated

[Qu Wenruo]

在 2025/11/25 08:55, David Sterba 写道:
> On Sat, Nov 22, 2025 at 07:16:06AM +1030, Qu Wenruo wrote:
[...]
>> Then check fs/*.c.
>>
>> There are guard(rcu) and rcu_read_lock() usage mixed in different files.
>>
>> E.g. in fs/namespace.c it's using guard(rcu) and scoped_guard(rcu),
>> meanwhile still having regular rcu_read_lock().
>>
>> There are more counter-examples than you know.
>> We're not the first subsystem to face the new RAII styles, and I hope we
>> will not be the last either.
> 
> We take inspiration from other subsystems but that some coding style is
> done in another subsystem does not mean we have to do that too. If
> fs/*.c people decide to use it everywhere then so be it.
> 
> I was hesitant to introduce the auto cleaning for btrfs_path or kfree
> but so far I found it working. The locking guards are quite different to
> that and I don't seem to get any liking in it

They are all RAII, I didn't see much difference between them.

Although the path auto-freeing is hiding quite some bad designs.

The path should only be allocated when first utilized (normally by 
btrfs_search_slot()).
But our btrfs_search_slot() design also needs to reuse pre-allocated 
paths for certain call sites, thus we have to treat path auto-freeing 
just like auto-freeing a pointer. It is just a compromise.

So you're just saying, "I'm fine with a compromise, but not the properly 
deisnged usage".


I won't argue that guard() may be a little tricky to read and expand in 
the future, but I see the readability cost is still way better than 
potential missing unlocks.

Not to mention we have scoped_guard(), which is making the critical 
section much easier to expose, and still easy to expand.

[...]
>> We're just a regular subsystem in the kernel, not some god-chosen
>> special one, we do not live in a bubble.
> 
> Honestly, this is a weird argument to make. There are local coding
> styles that are their own bubbles, look at net/* with their own special
> commenting style and mandatory reverse xmass tree sort of variables (I
> think that one has been adopted by other subsystems too). Contributing to
> those subsystems means following their style. If you want an example
> of a filesystem with unique code formatting style then go no further
> than XFS.

This is not coding style, but whether to adapt an existing 
practices/interfaces utilized by several programming languages and 
subsystems.

A lot of core synchronization code is already supporting RAII, and VFS 
is also trying pushing new RAII based APIs for filesystems, e.g:

https://lore.kernel.org/linux-btrfs/20251104-work-guards-v1-0-5108ac78a171@kernel.org/

You can go whatever code style you like, but in the end we're still 
depending on the infrastructures from VFS/MM/Block layers, you can not 
resist the change from them, or go the path to a deprecated fs in the 
future.

> 
> I think we've been doing fine in btrfs with the style consistency and
> incremental updates of old code when possible. This means we have fewer
> choices for personal "creative expression" how the code is written but
> in the long run it makes the code look better.

Then there is no conflicts with RAII, it's all about to write better and 
more correct code.

And RAII even provides less "creative way" on how to release a resource.
No creative tag names or whatever.

There may be some cases like holding different locks for a critical 
section, thus we may want to choose between things like scoped_guard() 
{guard()} or { guard(); guard(); }, but that's way less creative than 
manual GC with tags.

> 
> The problem with the guards is that there's no one good way for their
> general use, i.e. the problem of mixing with explicit lock/unlock,

Yes, I see some bugs during the above VFS RFC patch.

But that's the common thing of development. Code changes will bring 
bugs, but in the long run I believe RAII is way safer than manual GC, 
and encourages us to do better design of variable lifespan.

> trylock,

There are already "*_try" guards defines, although I only find very few 
users though.

E.g. drivers/input/keyboard/maple_keyb.c:

	scoped_guard(mutex_try, &maple_keyb_mutex) {


> unlock/lock order,

Isn't that the advantage of RAII? Compilers ensure the reserve order of 
releasing when exiting the scope.

Other than human-error-prone manual GC.


> etc. Allowing both styles will lead to
> inconsistent style based on personal preferences again.

You're asking for a huge code change on a large code base.

You should know it's not going to be done in a sudden, mixed RAII and 
manual GC will stay for a while.

I'm not encourage mixing, but that's just unavoidable.

I'd suggest not to mix any RAII and manual GC inside the same function.

> 
> As said on slack, we'll get to a conclusion.

At least I hope it's a future-proof conclusion.

Thanks,
Qu