Re: [PATCH v0] cgroup: Add lock guard support

[Johannes Weiner <hannes@cmpxchg.org>]

On Thu, Jun 05, 2025 at 05:54:15PM -0700, Alexei Starovoitov wrote:
> On Thu, Jun 5, 2025 at 2:11 PM Jemmy Wong <jemmywong512@gmail.com> wrote:
> >
> > This change replaces manual lock acquisition and release with lock guards
> > to improve code robustness and reduce the risk of lock mismanagement.
> > No functional changes to the cgroup logic are introduced.
> >
> > Signed-off-by: Jemmy Wong <jemmywong512@gmail.com>
> >
> > ---
> >  include/linux/cgroup.h     |   7 +
> >  kernel/bpf/cgroup.c        |  96 +++---
> >  kernel/bpf/local_storage.c |  12 +-
> 
> Nack for bpf bits.
> It only uglifies the code.

I agree with this.

The extra indentation from scoped guard is unfortunate.

The guard with implicit unlock gives me the heebeejeebees - it's
asymmetric and critical sections don't stand out visually at all.

Adjusting critical section boundaries with guard means either:
* indentation churn to convert to scoped guard,
* forcing abstraction splits along critical sections (non-API
  _locked functions), which makes the code flow harder to follow,
* or straight-up violating abstraction layering and adding
  non-critical stuff to callers where it doesn't make sense.

I don't remember the last production bug from forgetting to drop a
lock. Meanwhile, the things that people actually seem to struggle with
when it comes to locks appear to become harder with those primitives.