Re: [PATCH v1 0/3] cgroup: Add lock guard support

[Jemmy Wong <jemmywong512@gmail.com>]

Hi Michal, Tejun, Johannes,

Thank you, Michal, for supporting this modernization effort to adopt guard constructs.

With 3,326 instances already in use across the kernel upstream,
guards offer improved safety and readability.

I look forward to working with the community to integrate them into cgroup 
and welcome feedback to ensure a smooth transition.

Best,
Jemmy

> On Jun 17, 2025, at 5:08 PM, Michal Koutný <mkoutny@suse.com> wrote:
> 
> Hello.
> 
> I understand why this might have been controversial but I'm surprised it
> remains so when the lock guards are already used in the kernel code.
> Documentation/process/coding-style.rst isn't partisan in either way.
> 
> Johannes:
>> heebeejeebees - it's asymmetric and critical sections don't stand out
>> visually at all.
> - I'd say that's the point of it for functions that are made to
>  manipulate data structures under the lock. Not to spoil the code.
> - Yes, it's a different coding style that one has to get used to.
> 
>> extra indentation
> - deeper indentation == deeper locking, wary of that
> - although I admit, in some cases of the reworked code, it's overly deep
> 
> Further reasoning is laid out in include/linux/cleanup.h. I noticed
> there exists no_free_ptr() macro and that suggests an idea for analogous
> no_exit_class() (or unguard()) macro (essential an unlock + signal for
> compiler to not call cleanup function after this BB).
> 
> Tejun:
>> There are no practical benefits to converting the code base at this point.
> 
> I'd expect future backports (into such code) to be more robust wrt
> pairing errors.
> At the same time this is also my biggest concern about this change, the
> wide-spread diff would make current backporting more difficult.  (But
> I'd counter argue that one should think forward here.)
> 
> 
> Further benefits I see:
> - it is fewer lines (code is to the point),
> - reliable cleanup paths,
> - it is modern and evolution step forward (given such constructs
>  eventually emerge in various languages).
> 
> 
> On Sat, Jun 07, 2025 at 12:18:38AM +0800, Jemmy Wong <jemmywong512@gmail.com> wrote:
>> v1 changes:
>> - remove guard support for BPF
>> - split patch into parts
>> 
>> v0 link:
>> https://lore.kernel.org/all/20250605211053.19200-1-jemmywong512@gmail.com/
>> 
>> Jemmy Wong (3):
>>  cgroup: add lock guard support for cgroup_muetx
>>  cgroup: add lock guard support for css_set_lock and rcu
>>  cgroup: add lock guard support for others
> 
> As for the series in general
> - I'm still in favor of pursuing it (with remarks to individual
>  patches),
> - it's a good opportunity to also to append sparse __acquires/__releases
>  cleanup to it (see also [1]).
> 
> Regards,
> Michal
> 
> [1] https://lore.kernel.org/r/Z_6z2-qqLI7dbl8h@slm.duckdns.org
>