Re: [RFC PATCH 0/8] use cleanup.h in btrfs

[Gladyshev Ilya <foxido@foxido.dev>]

On 11/12/25 23:55, Qu Wenruo wrote:
> 在 2025/11/13 05:19, Gladyshev Ilya 写道:
>> This series represents my experimentation with refactoring with
>> cleanup guards. In my opinion, RAII-style locking improves readability
>> in most cases and also improves code robustness for future code changes,
>> so I tried to refactor simple cases that really benefits from lock 
>> guards.
> 
> Although I totally agree with the guard usages, it's not yet determined 
> we will fully embrace guard usages.
> 
>>
>> However readability is a subjective concept, so you can freely disagree
>> and reject any of those changes, I won't insist on any. Please note that
>> patches 1-3 can be useful even without lock guards.
>>
>> I didn't know how to split this series, mostly because it's just a lot of
>> small changes... so I tried to split it by types of transformation:
> 
> And even if we're determined to go guard path, I doubt if it should be 
> done in such a rushed way.
> 
> There are already some cases where scope based auto-cleanup conversion 
> led to some regressions, no matter how trivial they seem.
> Thankfully they are all caught early, but we have to ask one critical 
> question:
> 
>    Have you run the full fstest test cases?
> 
> If not, please run it first. Such huge change is not really that easy to 
> review.

No, because it's RFC only [however I tried to verify that no locking 
semantics/scopes changed and I tried not to touch any really complex 
scenarios.]
> Although I love the new scope based auto cleanup, I still tend to be 
> more cautious doing the conversion.
> 
> Thus my recommendation on the conversion would be:
> 
> - Up to the author/expert on the involved field
>    E.g. if Filipe wants to use guards for send, he is 100% fine to
>    send out dedicated patches to do the conversion.
> 
>    This also ensures reviewablity, as such change will only involve one
>    functionality.
> 
> - During other refactors of the code
>    This is pretty much the same for any code-style fixups.
>    We do not accept dedicated patches just fixing up whitespace/code-
>    style errors.
>    But if one is refactoring some code, it's recommended to fix any code-
>    style related problems near the touched part.

Personally I don't like this approach for correctness-sensitive 
refactoring. When it's something dedicated and standalone, it's easier 
to focus and verify that all changes are valid

> So I'm afraid we're not yet at the stage to accept huge conversions yet.

I would be surprised if such patchset would be accepted as one thing, 
honestly) For now, it's only purpose is to show how code _can_ be 
refactored in theory [not _should_]. And then, for example, if there is 
positive feedback on scoped guards, but not on full-function guards, I 
will send smaller, fully tested patch with only approved approaches.

Probably I should've been more clear on that in the cover letter, sorry...