memcg_ext: memcg policy through cgroup-attached struct_ops

Two things in __cgroup_bpf_query() work only because CGROUP_TCP_SOCK_OPS is the only one struct_ops attach type.

It calls cgroup_bpf_enabled(atype) with an atype that find_atype_by_struct_ops_id() works out at runtime. That macro is an asm goto and needs a constant. Today the compiler can see there is only one value; add a second type and the build breaks with "impossible constraint in 'asm'". Add cgroup_bpf_enabled_runtime(), which reads the key instead, and use it here. This is a syscall path, so the cost does not matter.

And find_atype_by_struct_ops_id() matches on type_id alone. An attach type whose subsystem is not built keeps type_id 0, so a query for type 0 finds it and returns success with nothing instead of -ENOENT. Skip such slots.

Signed-off-by Shakeel Butt <[email protected]>

---
 include/linux/bpf-cgroup.h | 9 +++++++++
 kernel/bpf/cgroup.c        | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

include/linux/bpf-cgroup.h +9 -0

index 8a75a6cd7309..3b2c127d401d 100644
@@ -76,6 +76,14 @@ to_cgroup_bpf_attach_type(enum bpf_attach_type attach_type)
 
 extern struct static_key_false cgroup_bpf_enabled_key[MAX_CGROUP_BPF_ATTACH_TYPE];
 #define cgroup_bpf_enabled(atype) static_branch_unlikely(&cgroup_bpf_enabled_key[atype])
+/*
+ * Same test when @atype is not a constant.  cgroup_bpf_enabled() uses
+ * static_branch_unlikely which creates jump-label site and requires statically
+ * selected key. Since key is selected dynamically, use static_key_enabled here
+ * and keep it off fast paths.
+ */
+#define cgroup_bpf_enabled_runtime(atype) \
+       static_key_enabled(&cgroup_bpf_enabled_key[atype])
 
 struct bpf_cgroup_storage_map;
 

@@ -508,6 +516,7 @@ static inline int cgroup_bpf_struct_ops_attach(struct bpf_map *map,
 }
 
 #define cgroup_bpf_enabled(atype) (0)
+#define cgroup_bpf_enabled_runtime(atype) (0)
 #define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, atype, t_ctx) ({ 0; })
 #define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, uaddrlen, atype) ({ 0; })
 #define BPF_CGROUP_PRE_CONNECT_ENABLED(sk) (0)

kernel/bpf/cgroup.c +2 -2

index 696b27383974..99b4e96f5db7 100644
@@ -45,6 +45,7 @@ static enum cgroup_bpf_attach_type find_atype_by_struct_ops_id(u32 type_id)
 
        for (atype = 0; atype < MAX_CGROUP_BPF_ATTACH_TYPE; atype++) {
                if (cgroup_bpf_is_struct_ops_atype(atype) &&
+                   cgroup_struct_ops[atype].type_id &&
                    cgroup_struct_ops[atype].type_id == type_id)
                        return atype;
        }

@@ -1448,7 +1449,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
                        return -ENOENT;
                from_atype = to_atype = atype;
                flags = 0;
-               if (!cgroup_bpf_enabled(atype))
+               if (!cgroup_bpf_enabled_runtime(atype))
                        goto skip_count;
        } else if (type == BPF_LSM_CGROUP) {
                if (!effective_query && attr->query.prog_cnt &&
--

2.53.0-Meta

Before it returns, try_charge_memcg() calls __mem_cgroup_handle_over_high(), which reclaims and can throttle the task. That happens wherever the charge happens, so a task holding a kernel lock can be stuck there, and everything waiting on the lock is stuck behind it.

Let's add high_policy ops through which a program gets a read-only snapshot of the charge and returns a request. There is one so far:
BPF_MEMCG_HIGH_DEFER_INLINE skips the inline call.

A charge runs the policies of its cgroup and of every ancestor. task_struct::in_bpf_memcg stops a program that allocates from re-entering the charge path and the dispatcher with it.

A memcg outlives its cgroup while it has charges, and cgroup_bpf_release() frees the arrays when the cgroup goes. Take the reference for the walk.

Signed-off-by Shakeel Butt <[email protected]>

---
  MAINTAINERS                    |  1 +
  include/linux/bpf-cgroup.h     |  3 ++
  include/linux/bpf_memcontrol.h | 53 +++++++++++++++++++++-
  include/linux/cgroup.h         |  7 +++
  include/linux/sched.h          |  4 ++
  mm/bpf_memcontrol.c            | 82 +++++++++++++++++++++++++++++++++-
  mm/memcontrol.c                | 31 ++++++++++++-
  7 files changed, 176 insertions(+), 5 deletions(-)

MAINTAINERS +1 -0

index 6215fcb07770..0c84beab396f 100644
@@ -5032,6 +5032,7 @@ L:	[email protected]
  L:	[email protected]
  S:	Maintained
  F:	mm/bpf_memcontrol.c
+F:	include/linux/bpf_memcontrol.h
 
  BPF [MISC]
  L:	[email protected]

include/linux/bpf-cgroup.h +3 -0

index 4e8150848bd2..e19cf83e58f3 100644
@@ -517,6 +517,9 @@ static inline int cgroup_bpf_struct_ops_attach(struct bpf_map *map,
 
  #define cgroup_bpf_enabled(atype) (0)
  #define cgroup_bpf_enabled_runtime(atype) (0)
+/* Nothing can be attached, so the walk has nothing to walk. */
+#define bpf_cgroup_struct_ops_foreach(var, item, cgrp, atype)		\
+	for ((void)(cgrp), (item) = NULL, (var) = NULL; 0; )
  #define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, atype, t_ctx) ({ 0; })
  #define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, uaddrlen, atype) ({ 0; })
  #define BPF_CGROUP_PRE_CONNECT_ENABLED(sk) (0)

include/linux/bpf_memcontrol.h +51 -2

index 8204d894761e..76ea5d1c3d32 100644
@@ -5,13 +5,62 @@
   * A bpf_memcg_ops is attached to a cgroup.  A charge runs the policies of
   * that cgroup and of every ancestor, and the kernel combines what they
   * return.  BPF only picks between things the kernel already does.
- *
- * The type has no members yet; they come with the policies that use them.
   */
  #ifndef _LINUX_BPF_MEMCONTROL_H
  #define _LINUX_BPF_MEMCONTROL_H
 
+#include <linux/types.h>
+#include <linux/gfp_types.h>
+
+struct mem_cgroup;
+struct task_struct;
+
+/*
+ * What a policy can ask for when a cgroup is over memory.high.  The kernel
+ * ORs them, so one policy cannot undo another.
+ */
+enum bpf_memcg_high_request {
+	BPF_MEMCG_HIGH_NO_OPINION	= 0,
+	/*
+	 * Skip the inline reclaim and throttle.  The debt is kept and paid on
+	 * the way back to userspace, where no kernel locks are held.
+	 */
+	BPF_MEMCG_HIGH_DEFER_INLINE	= 1U << 0,
+};
+
+#define BPF_MEMCG_HIGH_VALID_MASK	BPF_MEMCG_HIGH_DEFER_INLINE
+
+/* Read-only snapshot.  Only values the caller already has. */
+struct bpf_memcg_ctx {
+	struct mem_cgroup	*memcg;			/* charged memcg */
+	struct mem_cgroup	*memcg_over_limit;	/* NULL if none found */
+	struct task_struct	*task;			/* current */
+	u64			cgroup_id;
+	u64			over_limit_cgroup_id;	/* 0 if none */
+	u64			nr_pages_over_high;
+	u32			gfp_flags;
+};
+
  struct bpf_memcg_ops {
+	/**
+	 * high_policy - say where memory.high should be enforced
+	 * @ctx: snapshot of the charge
+	 *
+	 * Return: bits from enum bpf_memcg_high_request, or 0.  Other bits
+	 * are dropped.
+	 */
+	u32 (*high_policy)(const struct bpf_memcg_ctx *ctx);
  };
 
+/*
+ * Run every high_policy on @memcg's cgroup and its ancestors, and return the
+ * combined request for the caller to act on.
+ *
+ * @memcg:	the memcg being charged, never NULL
+ * @over_limit:	first memcg found over memory.high or swap.high, or NULL
+ * @gfp_mask:	the charge's gfp mask
+ */
+u32 bpf_memcg_high_policy(struct mem_cgroup *memcg,
+			  struct mem_cgroup *over_limit, gfp_t gfp_mask);
+
  #endif /* _LINUX_BPF_MEMCONTROL_H */

include/linux/cgroup.h +7 -0

index 5dfa915a630e..cf92b6cec819 100644
@@ -959,10 +959,17 @@ static inline void cgroup_bpf_put(struct cgroup *cgrp)
  	percpu_ref_put(&cgrp->bpf.refcnt);
  }
 
+/* Fails once the cgroup is gone and its bpf state has been freed. */
+static inline bool cgroup_bpf_tryget_live(struct cgroup *cgrp)
+{
+	return percpu_ref_tryget_live_rcu(&cgrp->bpf.refcnt);
+}
+
  #else /* CONFIG_CGROUP_BPF */
 
  static inline void cgroup_bpf_get(struct cgroup *cgrp) {}
  static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
+static inline bool cgroup_bpf_tryget_live(struct cgroup *cgrp) { return false; }
 
  #endif /* CONFIG_CGROUP_BPF */
 

include/linux/sched.h +4 -0

index 8b3d47a325cc..4b20a346aaa8 100644
@@ -1031,6 +1031,10 @@ struct task_struct {
  #ifdef CONFIG_MEMCG_V1
  	unsigned			in_user_fault:1;
  #endif
+#ifdef CONFIG_MEMCG
+	/* A bpf_memcg_ops program is running; do not recurse into policy */
+	unsigned			in_bpf_memcg:1;
+#endif
  #ifdef CONFIG_LRU_GEN
  	/* whether the LRU algorithm may apply to this access */
  	unsigned			in_lru_fault:1;

mm/bpf_memcontrol.c +61 -1

index fd6dff150f01..cfd0f1d443c9 100644
@@ -246,8 +246,17 @@ static const struct btf_kfunc_id_set bpf_memcontrol_reclaim_kfunc_set = {
   * request and the kernel acts on it.  Nothing here reclaims or sleeps.
   */
 
-/* CFI stubs.  A slot points at these while its policy is being detached. */
+/*
+ * CFI stubs.  These really run: a slot points at them while its policy is
+ * being detached.  Return 0, the identity for the kernel's OR.
+ */
+static u32 high_policy_stub(const struct bpf_memcg_ctx *ctx)
+{
+	return BPF_MEMCG_HIGH_NO_OPINION;
+}
+
  static struct bpf_memcg_ops __bpf_memcg_ops = {
+	.high_policy = high_policy_stub,
  };
 
  static const struct bpf_func_proto *

@@ -326,6 +335,77 @@ static struct bpf_struct_ops bpf_memcg_ops_desc = {
  	 */
  };
 
+static void bpf_memcg_ctx_init(struct bpf_memcg_ctx *ctx,
+			       struct mem_cgroup *memcg,
+			       struct mem_cgroup *over_limit, gfp_t gfp_mask)
+{
+	ctx->memcg = memcg;
+	ctx->memcg_over_limit = over_limit;
+	ctx->task = current;
+	ctx->cgroup_id = cgroup_id(memcg->css.cgroup);
+	ctx->over_limit_cgroup_id = over_limit ?
+		cgroup_id(over_limit->css.cgroup) : 0;
+	ctx->nr_pages_over_high = current->memcg_nr_pages_over_high;
+	ctx->gfp_flags = (__force u32)gfp_mask;
+}
+
+u32 bpf_memcg_high_policy(struct mem_cgroup *memcg,
+			  struct mem_cgroup *over_limit, gfp_t gfp_mask)
+{
+	const struct bpf_prog_array_item *item;
+	const struct bpf_memcg_ops *ops;
+	struct bpf_memcg_ctx ctx;
+	u32 acc = BPF_MEMCG_HIGH_NO_OPINION;
+	struct cgroup *cgrp;
+
+	if (!cgroup_bpf_enabled(CGROUP_MEMCG_OPS))
+		return acc;
+
+	/*
+	 * Only the default hierarchy has a cgroup_bpf, and the static key is
+	 * global, so one policy anywhere turns this on for v1 memcgs too.  A
+	 * v1 memcg still cannot get here, because memory.high and swap.high
+	 * are both v2-only and so it never builds the debt that leads to this
+	 * call.  A hook on a path v1 can reach needs its own cgroup_on_dfl()
+	 * test: a v1 cgroup has no effective array and an uninitialised
+	 * cgrp->bpf.refcnt.
+	 */
+	cgrp = memcg->css.cgroup;
+
+	/*
+	 * A program can allocate and re-enter the charge path.  Skip the
+	 * nested call.  This guards the callbacks only.
+	 */
+	if (current->in_bpf_memcg)
+		return acc;
+	current->in_bpf_memcg = 1;
+
+	rcu_read_lock_dont_migrate();
+
+	/*
+	 * A memcg outlives its cgroup while it has charges, and
+	 * cgroup_bpf_release() frees the arrays when the cgroup goes.
+	 */

Quoted 57 lines

mm/bpf_memcontrol.c +57 -0

+static void bpf_memcg_ctx_init(struct bpf_memcg_ctx *ctx,
+			       struct mem_cgroup *memcg,
+			       struct mem_cgroup *over_limit, gfp_t gfp_mask)
+{
+	ctx->memcg = memcg;
+	ctx->memcg_over_limit = over_limit;
+	ctx->task = current;
+	ctx->cgroup_id = cgroup_id(memcg->css.cgroup);
+	ctx->over_limit_cgroup_id = over_limit ?
+		cgroup_id(over_limit->css.cgroup) : 0;
+	ctx->nr_pages_over_high = current->memcg_nr_pages_over_high;
+	ctx->gfp_flags = (__force u32)gfp_mask;
+}
+
+u32 bpf_memcg_high_policy(struct mem_cgroup *memcg,
+			  struct mem_cgroup *over_limit, gfp_t gfp_mask)
+{
+	const struct bpf_prog_array_item *item;
+	const struct bpf_memcg_ops *ops;
+	struct bpf_memcg_ctx ctx;
+	u32 acc = BPF_MEMCG_HIGH_NO_OPINION;
+	struct cgroup *cgrp;
+
+	if (!cgroup_bpf_enabled(CGROUP_MEMCG_OPS))
+		return acc;
+
+	/*
+	 * Only the default hierarchy has a cgroup_bpf, and the static key is
+	 * global, so one policy anywhere turns this on for v1 memcgs too.  A
+	 * v1 memcg still cannot get here, because memory.high and swap.high
+	 * are both v2-only and so it never builds the debt that leads to this
+	 * call.  A hook on a path v1 can reach needs its own cgroup_on_dfl()
+	 * test: a v1 cgroup has no effective array and an uninitialised
+	 * cgrp->bpf.refcnt.
+	 */
+	cgrp = memcg->css.cgroup;
+
+	/*
+	 * A program can allocate and re-enter the charge path.  Skip the
+	 * nested call.  This guards the callbacks only.
+	 */
+	if (current->in_bpf_memcg)
+		return acc;
+	current->in_bpf_memcg = 1;
+
+	rcu_read_lock_dont_migrate();
+
+	/*
+	 * A memcg outlives its cgroup while it has charges, and
+	 * cgroup_bpf_release() frees the arrays when the cgroup goes.
+	 */
+	if (!cgroup_bpf_tryget_live(cgrp))
+		goto out;
+
+	bpf_memcg_ctx_init(&ctx, memcg, over_limit, gfp_mask);
+
+	bpf_cgroup_struct_ops_foreach(ops, item, cgrp, CGROUP_MEMCG_OPS) {

mm/bpf_memcontrol.c static struct bpf_struct_ops bpf_memcg_ops_desc = { · line 398

Quoted 4 lines

mm/bpf_memcontrol.c +4 -0

+		if (ops->high_policy)
+			acc |= ops->high_policy(&ctx) &
+			       BPF_MEMCG_HIGH_VALID_MASK;
+	}

If I'm reading correctly, the gfp_mask at this point doesn't account for task restrictions, so the BPF program may see __GFP_FS, __GFP_IO, etc which may later be cleared when setting up the scan_control instance.

Before it returns, try_charge_memcg() calls __mem_cgroup_handle_over_high(), which reclaims and can throttle the task. That happens wherever the charge happens, so a task holding a kernel lock can be stuck there, and everything waiting on the lock is stuck behind it.

Let's add high_policy ops through which a program gets a read-only snapshot of the charge and returns a request. There is one so far:
BPF_MEMCG_HIGH_DEFER_INLINE skips the inline call.

A charge runs the policies of its cgroup and of every ancestor. task_struct::in_bpf_memcg stops a program that allocates from re-entering the charge path and the dispatcher with it.

A memcg outlives its cgroup while it has charges, and cgroup_bpf_release() frees the arrays when the cgroup goes. Take the reference for the walk.

Signed-off-by Shakeel Butt <[email protected]>

---
  MAINTAINERS                    |  1 +
  include/linux/bpf-cgroup.h     |  3 ++
  include/linux/bpf_memcontrol.h | 53 +++++++++++++++++++++-
  include/linux/cgroup.h         |  7 +++
  include/linux/sched.h          |  4 ++
  mm/bpf_memcontrol.c            | 82 +++++++++++++++++++++++++++++++++-
  mm/memcontrol.c                | 31 ++++++++++++-
  7 files changed, 176 insertions(+), 5 deletions(-)

MAINTAINERS +1 -0

index 6215fcb07770..0c84beab396f 100644
@@ -5032,6 +5032,7 @@ L:	[email protected]
  L:	[email protected]
  S:	Maintained
  F:	mm/bpf_memcontrol.c
+F:	include/linux/bpf_memcontrol.h
 
  BPF [MISC]
  L:	[email protected]

include/linux/bpf-cgroup.h +3 -0

index 4e8150848bd2..e19cf83e58f3 100644
@@ -517,6 +517,9 @@ static inline int cgroup_bpf_struct_ops_attach(struct bpf_map *map,
 
  #define cgroup_bpf_enabled(atype) (0)
  #define cgroup_bpf_enabled_runtime(atype) (0)
+/* Nothing can be attached, so the walk has nothing to walk. */
+#define bpf_cgroup_struct_ops_foreach(var, item, cgrp, atype)		\
+	for ((void)(cgrp), (item) = NULL, (var) = NULL; 0; )
  #define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, atype, t_ctx) ({ 0; })
  #define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, uaddrlen, atype) ({ 0; })
  #define BPF_CGROUP_PRE_CONNECT_ENABLED(sk) (0)

include/linux/bpf_memcontrol.h +51 -2

index 8204d894761e..76ea5d1c3d32 100644
@@ -5,13 +5,62 @@
   * A bpf_memcg_ops is attached to a cgroup.  A charge runs the policies of
   * that cgroup and of every ancestor, and the kernel combines what they
   * return.  BPF only picks between things the kernel already does.
- *
- * The type has no members yet; they come with the policies that use them.
   */
  #ifndef _LINUX_BPF_MEMCONTROL_H
  #define _LINUX_BPF_MEMCONTROL_H
 
+#include <linux/types.h>
+#include <linux/gfp_types.h>
+
+struct mem_cgroup;
+struct task_struct;
+
+/*
+ * What a policy can ask for when a cgroup is over memory.high.  The kernel
+ * ORs them, so one policy cannot undo another.
+ */
+enum bpf_memcg_high_request {
+	BPF_MEMCG_HIGH_NO_OPINION	= 0,
+	/*
+	 * Skip the inline reclaim and throttle.  The debt is kept and paid on
+	 * the way back to userspace, where no kernel locks are held.
+	 */
+	BPF_MEMCG_HIGH_DEFER_INLINE	= 1U << 0,
+};
+
+#define BPF_MEMCG_HIGH_VALID_MASK	BPF_MEMCG_HIGH_DEFER_INLINE
+
+/* Read-only snapshot.  Only values the caller already has. */
+struct bpf_memcg_ctx {
+	struct mem_cgroup	*memcg;			/* charged memcg */
+	struct mem_cgroup	*memcg_over_limit;	/* NULL if none found */
+	struct task_struct	*task;			/* current */
+	u64			cgroup_id;
+	u64			over_limit_cgroup_id;	/* 0 if none */
+	u64			nr_pages_over_high;
+	u32			gfp_flags;
+};
+
  struct bpf_memcg_ops {
+	/**
+	 * high_policy - say where memory.high should be enforced
+	 * @ctx: snapshot of the charge
+	 *
+	 * Return: bits from enum bpf_memcg_high_request, or 0.  Other bits
+	 * are dropped.
+	 */
+	u32 (*high_policy)(const struct bpf_memcg_ctx *ctx);
  };
 
+/*
+ * Run every high_policy on @memcg's cgroup and its ancestors, and return the
+ * combined request for the caller to act on.
+ *
+ * @memcg:	the memcg being charged, never NULL
+ * @over_limit:	first memcg found over memory.high or swap.high, or NULL
+ * @gfp_mask:	the charge's gfp mask
+ */
+u32 bpf_memcg_high_policy(struct mem_cgroup *memcg,
+			  struct mem_cgroup *over_limit, gfp_t gfp_mask);
+
  #endif /* _LINUX_BPF_MEMCONTROL_H */

include/linux/cgroup.h +7 -0

index 5dfa915a630e..cf92b6cec819 100644
@@ -959,10 +959,17 @@ static inline void cgroup_bpf_put(struct cgroup *cgrp)
  	percpu_ref_put(&cgrp->bpf.refcnt);
  }
 
+/* Fails once the cgroup is gone and its bpf state has been freed. */
+static inline bool cgroup_bpf_tryget_live(struct cgroup *cgrp)
+{
+	return percpu_ref_tryget_live_rcu(&cgrp->bpf.refcnt);
+}
+
  #else /* CONFIG_CGROUP_BPF */
 
  static inline void cgroup_bpf_get(struct cgroup *cgrp) {}
  static inline void cgroup_bpf_put(struct cgroup *cgrp) {}
+static inline bool cgroup_bpf_tryget_live(struct cgroup *cgrp) { return false; }
 
  #endif /* CONFIG_CGROUP_BPF */
 

include/linux/sched.h +4 -0

index 8b3d47a325cc..4b20a346aaa8 100644
@@ -1031,6 +1031,10 @@ struct task_struct {
  #ifdef CONFIG_MEMCG_V1
  	unsigned			in_user_fault:1;
  #endif
+#ifdef CONFIG_MEMCG
+	/* A bpf_memcg_ops program is running; do not recurse into policy */
+	unsigned			in_bpf_memcg:1;
+#endif
  #ifdef CONFIG_LRU_GEN
  	/* whether the LRU algorithm may apply to this access */
  	unsigned			in_lru_fault:1;

mm/bpf_memcontrol.c +47 -1

index fd6dff150f01..cfd0f1d443c9 100644
@@ -246,8 +246,17 @@ static const struct btf_kfunc_id_set bpf_memcontrol_reclaim_kfunc_set = {
   * request and the kernel acts on it.  Nothing here reclaims or sleeps.
   */
 
-/* CFI stubs.  A slot points at these while its policy is being detached. */
+/*
+ * CFI stubs.  These really run: a slot points at them while its policy is
+ * being detached.  Return 0, the identity for the kernel's OR.
+ */
+static u32 high_policy_stub(const struct bpf_memcg_ctx *ctx)
+{
+	return BPF_MEMCG_HIGH_NO_OPINION;
+}
+
  static struct bpf_memcg_ops __bpf_memcg_ops = {
+	.high_policy = high_policy_stub,
  };
 
  static const struct bpf_func_proto *

@@ -326,6 +335,77 @@ static struct bpf_struct_ops bpf_memcg_ops_desc = {
  	 */
  };
 
+static void bpf_memcg_ctx_init(struct bpf_memcg_ctx *ctx,
+			       struct mem_cgroup *memcg,
+			       struct mem_cgroup *over_limit, gfp_t gfp_mask)
+{
+	ctx->memcg = memcg;
+	ctx->memcg_over_limit = over_limit;
+	ctx->task = current;
+	ctx->cgroup_id = cgroup_id(memcg->css.cgroup);
+	ctx->over_limit_cgroup_id = over_limit ?
+		cgroup_id(over_limit->css.cgroup) : 0;
+	ctx->nr_pages_over_high = current->memcg_nr_pages_over_high;
+	ctx->gfp_flags = (__force u32)gfp_mask;
+}
+
+u32 bpf_memcg_high_policy(struct mem_cgroup *memcg,
+			  struct mem_cgroup *over_limit, gfp_t gfp_mask)
+{
+	const struct bpf_prog_array_item *item;
+	const struct bpf_memcg_ops *ops;
+	struct bpf_memcg_ctx ctx;
+	u32 acc = BPF_MEMCG_HIGH_NO_OPINION;
+	struct cgroup *cgrp;
+
+	if (!cgroup_bpf_enabled(CGROUP_MEMCG_OPS))
+		return acc;
+
+	/*
+	 * Only the default hierarchy has a cgroup_bpf, and the static key is
+	 * global, so one policy anywhere turns this on for v1 memcgs too.  A
+	 * v1 memcg still cannot get here, because memory.high and swap.high
+	 * are both v2-only and so it never builds the debt that leads to this
+	 * call.  A hook on a path v1 can reach needs its own cgroup_on_dfl()
+	 * test: a v1 cgroup has no effective array and an uninitialised
+	 * cgrp->bpf.refcnt.
+	 */
+	cgrp = memcg->css.cgroup;
+

mm/bpf_memcontrol.c +14 -0

+		if (ops->high_policy)
+			acc |= ops->high_policy(&ctx) &
+			       BPF_MEMCG_HIGH_VALID_MASK;
+	}
+
+	cgroup_bpf_put(cgrp);
+out:
+	rcu_read_unlock_migrate();
+
+	current->in_bpf_memcg = 0;
+
+	return acc;
+}
+
  static int __init bpf_memcg_ops_register(void)
  {
  	/*

mm/memcontrol.c +29 -2

index 1271d390b617..bc283680640b 100644
@@ -28,6 +28,7 @@
  #include <linux/cgroup-defs.h>
  #include <linux/page_counter.h>
  #include <linux/memcontrol.h>
+#include <linux/bpf_memcontrol.h>
  #include <linux/cgroup.h>
  #include <linux/cpuset.h>
  #include <linux/sched/mm.h>

@@ -2642,9 +2643,26 @@ void __mem_cgroup_handle_over_high(gfp_t gfp_mask)
  	css_put(&memcg->css);
  }
 
+/*
+ * Ask the attached bpf_memcg_ops whether to skip the inline memory.high
+ * reclaim and throttle.
+ *
+ * @memcg:	the memcg being charged
+ * @over_limit:	first memcg found over memory.high or swap.high, starting at
+ *		the charged one, or NULL if the walk found none
+ */
+static bool bpf_memcg_high_defer(struct mem_cgroup *memcg,
+				 struct mem_cgroup *over_limit, gfp_t gfp_mask)
+{
+	u32 req = bpf_memcg_high_policy(memcg, over_limit, gfp_mask);
+
+	return req & BPF_MEMCG_HIGH_DEFER_INLINE;
+}
+
  static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
  			    unsigned int nr_pages)
  {
+	struct mem_cgroup *leaf_memcg = memcg;
  	unsigned int batch = max(MEMCG_CHARGE_BATCH, nr_pages);
  	int nr_retries = MAX_RECLAIM_RETRIES;
  	struct mem_cgroup *mem_over_limit;

@@ -2846,8 +2864,17 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
  	 */
  	if (current->memcg_nr_pages_over_high > MEMCG_CHARGE_BATCH &&
  	    !(current->flags & PF_MEMALLOC) &&
-	    gfpflags_allow_blocking(gfp_mask))
-		__mem_cgroup_handle_over_high(gfp_mask);
+	    gfpflags_allow_blocking(gfp_mask)) {
+		/*
+		 * The loop above left @memcg as the first memcg it found over
+		 * memory.high or swap.high -- possibly the charged one itself
+		 * -- or NULL if it found none.  Note the debt can be left over
+		 * from an earlier charge, so NULL does not mean no pressure.
+		 * The policy wants the memcg we charged.
+		 */
+		if (!bpf_memcg_high_defer(leaf_memcg, memcg, gfp_mask))
+			__mem_cgroup_handle_over_high(gfp_mask);
+	}
  	return 0;
  }
 

This is the first series of memcg_ext, proposed at [1]. It adds bpf_memcg_ops, a struct_ops type through which memory controller policy is attached to a cgroup. A charge runs the policies of its cgroup and of every ancestor, and the kernel combines what they return. BPF picks between things the kernel already does; it never does the work itself and never touches a page counter.

The plan is to grow this one member at a time, and to add a member only when there is a concrete problem it solves and a measurement showing it does. Nothing is added because it might be useful later. So this first series adds one member, for one problem.

The problem
===========

try_charge_memcg() calls __mem_cgroup_handle_over_high() before it returns, which reclaims and can throttle the task. That happens wherever the charge happens, so a task holding a kernel lock can be stuck there, and everything waiting on that lock is stuck behind it.

One concrete scenario which can be resolved by this new feature is the kernfs notify worker. It delivers notifications with the cgroup2 kernfs_rwsem held for read, and the charge for the delivery allocation goes to the cgroup that set the watch, usually one already under pressure. So the worker reclaims while holding the lock, a waiting writer blocks every later reader, and anything touching cgroupfs stalls for seconds. The patch proposed in [2] fixes that one path in the kernel only for kernfs_rwsem. The same path still takes kernfs_supers_rwsem.

One can make an argument that if we know the source of the issue in the kernel, why not fix it similarly to [2] instead of having a generic solution? The reason is that it will be an uphill and continuous battle as the kernel keeps evolving and new sources of lock holders doing allocations keep coming up. In addition, there will be cases where it might not be possible to move the allocations out of locks, or where doing so would make the code really ugly [3].

high_policy() lets a policy say where memory.high should be enforced instead. Its one request, BPF_MEMCG_HIGH_DEFER_INLINE, skips the inline call, leaving the debt to be paid at the return to userspace.

Results
=======

Measured on a kernel without [2], with a policy that marks cgroupfs kernfs lock holders. Reproducer at [4].

                                      baseline    policy    upstream fix
  max kernfs_rwsem hold, worker        2.049 s     199 us   not taken
  max kernfs_supers_rwsem hold         2.049 s     4.8 ms   2.056 s
  max kernfs_rwsem write wait          4.096 s     6.2 ms   8.7 ms
  walker passes over /sys/fs/cgroup        352       5711   6700
  churn mkdir+rmdir ops                 13 059    364 832   415 488
  churn max latency                     30.7 s    15.1 ms   12.7 ms

The policy matches the fix on the bystander numbers, and does better on kernfs_supers_rwsem, which the fix does not help: it drops kernfs_rwsem from the delivery loop, while the policy stops the worker stalling at all, so every lock it holds benefits.

The patches
===========

Patch 1 is a build fix. __cgroup_bpf_query() only works because CGROUP_TCP_SOCK_OPS is the one struct_ops attach type; a second one breaks the build and makes a query for type 0 return the wrong thing.

Patch 2 adds the type and its registration, with no members, so nothing is dispatched and behaviour does not change. It also moves the attach type enum out of CONFIG_CGROUP_BPF and fixes the register_bpf_struct_ops() no-op stub, which never compiled because no caller reached it.

Patch 3 adds high_policy() and wires it into try_charge_memcg().

Patch 4 is the sample. It tracks the two cgroupfs kernfs locks by hooking the rwsem primitives rather than the forty-odd functions that take them.

Known open questions
====================

The semantics of memory.high for remote chargers or kernel threads is a grey area and this series does not aim to resolve that.

Another open question is whether a bound on debt deferral is needed. At the moment, we think that rather than putting a limit on deferral for memory.high, it will be better to handle that through an async worker like memcg->high_work. We aim to introduce that later, along with the right CPU accounting for that async work.

There is no prog_tests harness, so the numbers above come from the reproducer and not from the test suite.

The sample's lock tracking costs about 8% throughput, because it instruments every rwsem operation on the system. That is fine for a sample that has to cover every path, but it argues for a cheaper way to track locks that cause isolation problems between unrelated workloads.

Future work
===========

Asking the kernel to reclaim on the policy's behalf, so a task that cannot be throttled still pays. More members, each with a use case: hard-limit policy, reclaim shaping, dynamic protection.

This builds on "bpf: A common way to attach struct_ops to a cgroup", which supplies attach, detach, ordering, update, query and the RCU rules. bpf_memcg_ops is its second user.

[1] https://lore.kernel.org/[email protected]/
[2] https://lore.kernel.org/[email protected]/
[3] https://lore.kernel.org/20260917-wehten-achtfach-getarnt-c85a4812337d@brauner/
[4] https://github.com/shakeelb/mempressure-repros/tree/main/kernfs-notify-memcg

Shakeel Butt (4):
bpf, cgroup: fix cgroup struct_ops query for a second attach type
memcg_ext: add cgroup-attached bpf_memcg_ops
memcg_ext: allow BPF to defer memory.high enforcement selftests/bpf: add a cgroupfs lock-holder bpf_memcg_ops sample

 MAINTAINERS                                   |   1 +
 include/linux/bpf-cgroup-defs.h               |  22 +-
 include/linux/bpf-cgroup.h                    |  14 +-
 include/linux/bpf.h                           |   2 +-
 include/linux/bpf_memcontrol.h                |  66 ++++++
 include/linux/cgroup.h                        |   7 +
 include/linux/sched.h                         |   4 +
 kernel/bpf/cgroup.c                           |   3 +-
 mm/bpf_memcontrol.c                           | 188 ++++++++++++++-
 mm/memcontrol.c                               |  31 ++-
 .../bpf/progs/memcg_ops_lockholder.c          | 222 ++++++++++++++++++
 11 files changed, 546 insertions(+), 14 deletions(-)
 create mode 100644 include/linux/bpf_memcontrol.h
 create mode 100644 tools/testing/selftests/bpf/progs/memcg_ops_lockholder.c

base-commit: 6e36e099b15bed1e8e5b3e3136c5e3eb56a15aa7 --
2.53.0-Meta

Quoted 1 line · Shakeel Butt

Quoted 4 lines

Known open questions
====================

Quoted 8 lines

The semantics of memory.high for remote chargers or kernel threads is a grey area and this series does not aim to resolve that.

Another open question is whether a bound on debt deferral is needed. At the moment, we think that rather than putting a limit on deferral for memory.high, it will be better to handle that through an async worker like memcg->high_work. We aim to introduce that later, along with the right CPU accounting for that async work.

Hello Shakeel,

On the open question of how deferred debt eventually gets paid: would it make sense for the policy to also notify userspace (e.g. via ringbuf) when it defers,

and have a userspace reclaimer do the reclaim
through memory.reclaim?

I understand one of the concerns for the async worker is CPU accounting. If the concern is that the kworker's CPU usage is not charged to the target cgroup, the userspace reclaimer could instead be spawned with clone3(CLONE_INTO_CGROUP) so it runs inside the target cgroup, and both its CPU and memory usage get charged there.

One caveat: intermediate cgroups with the no-internal-process constraint cannot take processes, so this would only work for leaf cgroups.

What do you think?

Hi Yafang,

Quoted 2 lines · Yafang Shao

Quoted 1 line · Shakeel Butt

[...]

Quoted 13 lines

Quoted 4 lines

Known open questions
====================

Quoted 8 lines

The semantics of memory.high for remote chargers or kernel threads is a grey area and this series does not aim to resolve that.

Another open question is whether a bound on debt deferral is needed. At the moment, we think that rather than putting a limit on deferral for memory.high, it will be better to handle that through an async worker like memcg->high_work. We aim to introduce that later, along with the right CPU accounting for that async work.

Quoted 5 lines

Hello Shakeel,

On the open question of how deferred debt eventually gets paid: would it make sense for the policy to also notify userspace (e.g. via ringbuf) when it defers,

I think the notification through bpf programs is already possible and a bpf program deciding to bypass memory.high can already do notification via ringbuf.

Quoted 13 lines

and have a userspace reclaimer do the reclaim
through memory.reclaim?

I understand one of the concerns for the async worker is CPU accounting. If the concern is that the kworker's CPU usage is not charged to the target cgroup, the userspace reclaimer could instead be spawned with clone3(CLONE_INTO_CGROUP) so it runs inside the target cgroup, and both its CPU and memory usage get charged there.

One caveat: intermediate cgroups with the no-internal-process constraint cannot take processes, so this would only work for leaf cgroups.

Quoted 1 line

What do you think?

I think all of this is possible without additional code and with this series. With AI, should be very easy to prototype it. Please take a stab and I will look into it as well (time permitting).

Thanks for taking a look and also please let me know what other ways you think memcg can be customized through BPF in a beneficial way.

Quoted 28 lines · Shakeel Butt

Hi Yafang,

Quoted 7 lines · Yafang Shao

Quoted 1 line · Shakeel Butt

Hello Shakeel,

On the open question of how deferred debt eventually gets paid: would it make sense for the policy to also notify userspace (e.g. via ringbuf) when it defers,

I think the notification through bpf programs is already possible and a bpf program deciding to bypass memory.high can already do notification via ringbuf.

Quoted 13 lines

and have a userspace reclaimer do the reclaim
through memory.reclaim?

I understand one of the concerns for the async worker is CPU accounting. If the concern is that the kworker's CPU usage is not charged to the target cgroup, the userspace reclaimer could instead be spawned with clone3(CLONE_INTO_CGROUP) so it runs inside the target cgroup, and both its CPU and memory usage get charged there.

One caveat: intermediate cgroups with the no-internal-process constraint cannot take processes, so this would only work for leaf cgroups.

Quoted 5 lines

Quoted 1 line

What do you think?

I think all of this is possible without additional code and with this series. With AI, should be very easy to prototype it. Please take a stab and I will look into it as well (time permitting).

An LLM helped me quickly implement a userspace async memcg reclaimer based on your series, and it seems to work quite well.

Quoted 4 lines

Thanks for taking a look and also please let me know what other ways you think memcg can be customized through BPF in a beneficial way.

Sure. On our production servers we have been running a set of BPF programs to tailor kernel behavior for different workloads — all of them global programs so far — and I believe they are all good candidates for per-cgroup BPF policies now that cgroup-attached struct_ops is available. They have been really helpful in our Kubernetes production environment. I have sent some of them upstream, such as:

- BPF-THP

    https://lwn.net/Articles/1039689/
  - BPF-auto-NUMA
    https://lwn.net/Articles/1054030/

Perhaps we can revisit both of them and turn them into per-cgroup policies — what do you think?

We are also running some custom BPF programs that have not been sent upstream yet, such as:

- BPF-async-reclaimer
We don't care about the CPU accounting of the kworker, so we just
wake up a kworker to do the async reclaiming.

- BPF-fault-around

Both are really beneficial to our workloads, and both are global programs today.

We are planning a few more customizations to resolve painful production issues, such as:

- The long-standing inode::lock contention caused by dentries [0]. We have not started implementing it yet, but we might introduce a
memcg->dentry_limit or a memcg->vfs_cache_pressure as BPF policies..
- cgroup-level readahead.

So, to answer your question directly: for memcg itself, the beneficial customizations for us are the reclaim policy (the async reclaimer above), the dentry/vfs cache pressure knobs, and fault-around; the rest are per-cgroup MM policies that would need the struct_ops-to-cgroup mechanism generalized beyond memcg — which is why I hope these use cases can help make the design more generic.

[0] https://lore.kernel.org/linux-fsdevel/[email protected]/

Signature · 2 lines

Regards
Yafang

Quoted 35 lines · Yafang Shao

Quoted 33 lines · Shakeel Butt

Hi Yafang,

Quoted 7 lines · Yafang Shao

Quoted 1 line · Shakeel Butt

Hello Shakeel,

On the open question of how deferred debt eventually gets paid: would it make sense for the policy to also notify userspace (e.g. via ringbuf) when it defers,

I think the notification through bpf programs is already possible and a bpf program deciding to bypass memory.high can already do notification via ringbuf.

Quoted 9 lines

and have a userspace reclaimer do the reclaim
through memory.reclaim?

I understand one of the concerns for the async worker is CPU accounting. If the concern is that the kworker's CPU usage is not charged to the target cgroup, the userspace reclaimer could instead be spawned with clone3(CLONE_INTO_CGROUP) so it runs inside the target cgroup, and both its CPU and memory usage get charged there.

Quoted 5 lines

One caveat: intermediate cgroups with the no-internal-process constraint cannot take processes, so this would only work for leaf cgroups.

What do you think?

I think all of this is possible without additional code and with this series. With AI, should be very easy to prototype it. Please take a stab and I will look into it as well (time permitting).

Quoted 2 lines

An LLM helped me quickly implement a userspace async memcg reclaimer based on your series, and it seems to work quite well.

That's awesome. Please do take a look at the code and provide feedback and if you don't mind, a tested-by tag would be awesome.

try_charge_memcg() calls __mem_cgroup_handle_over_high() before it returns, which reclaims and can throttle the task. That happens wherever the charge happens, so a task holding a kernel lock can be stuck there, and everything waiting on that lock is stuck behind it.

One concrete scenario which can be resolved by this new feature is the kernfs notify worker. It delivers notifications with the cgroup2 kernfs_rwsem held for read, and the charge for the delivery allocation goes to the cgroup that set the watch, usually one already under pressure. So the worker reclaims while holding the lock, a waiting writer blocks every later reader, and anything touching cgroupfs stalls for seconds.

Hello, Shakeel.

Quoted 4 lines · Shakeel Butt

try_charge_memcg() calls __mem_cgroup_handle_over_high() before it returns, which reclaims and can throttle the task. That happens wherever the charge happens, so a task holding a kernel lock can be stuck there, and everything waiting on that lock is stuck behind it.

Why not just raise the lazy bound high enough that most charges never enforce inline, and maybe annotate the specific paths that can allocate a lot so that they do? Inline enforcement should be the exception, not the rule. Flipping that and then trying to reverse it with custom BPF policies doesn't make a lot of sense.

Quoted 6 lines

One concrete scenario which can be resolved by this new feature is the kernfs notify worker. It delivers notifications with the cgroup2 kernfs_rwsem held for read, and the charge for the delivery allocation goes to the cgroup that set the watch, usually one already under pressure. So the worker reclaims while holding the lock, a waiting writer blocks every later reader, and anything touching cgroupfs stalls for seconds.

Slowing down the kworker inline doesn't make sense. It's charging on behalf of the watcher through set_active_memcg(), which already tells us whose debt it is. Wouldn't it make more sense to defer the debt to that cgroup instead of slowing down the kernel thread?

I am fine with changing the default behavior. Actually, I have been contemplating whether I should propose a revert of commit c9afe31ec443e ("memcg: synchronously enforce memory.high for large overcharges") because it has introduced more problems than it has solved, but that is a separate topic. The initial commit already mentioned that MEMCG_CHARGE_BATCH was used arbitrarily, so replacing it with something big might be acceptable. I want to keep that decision separate.

Returning to the actual proposal, my plan was to start small with a narrow, specific use case. However, my long-term plan is to provide a mechanism to change the default behavior for custom use cases. For example, for memory.high, I will provide a way for users to specify what behavior they want, i.e., whether or not they want more synchronous throttling.

Hello, Shakeel.

Quoted 7 lines · Shakeel Butt

I am fine with changing the default behavior. Actually, I have been contemplating whether I should propose a revert of commit c9afe31ec443e ("memcg: synchronously enforce memory.high for large overcharges") because it has introduced more problems than it has solved, but that is a separate topic. The initial commit already mentioned that MEMCG_CHARGE_BATCH was used arbitrarily, so replacing it with something big might be acceptable. I want to keep that decision separate.

Which kernel paths allocate enough for this to matter? Can you give specific examples where the in-kernel synchronous enforcement helps?

Quoted 6 lines

Returning to the actual proposal, my plan was to start small with a narrow, specific use case. However, my long-term plan is to provide a mechanism to change the default behavior for custom use cases. For example, for memory.high, I will provide a way for users to specify what behavior they want, i.e., whether or not they want more synchronous throttling.

This doesn't seem like a policy problem.

It feels like a problem that should
be and can reasonably be solved for everybody, so I'm not sure whether BPF is the right call for this specific purpose.

mlock(), madvise(POPULATE), fadvise(WILLNEED) are the obvious ones where large amount of memory can be allocated before returning to userspace.

Quoted 1 line

This doesn't seem like a policy problem.

What is "This" in the above statement?

Here if you meant that default behavior of memory.high should work for most (if not all) users then we are on same page. If some user want memory.high reclaim to happen in a separate thread instead of return-to-userspace or synchronously, this proposal provides mechanism through BPF to such users to achieve their goals.

Hello, Shakeel.

Quoted 6 lines

Here if you meant that default behavior of memory.high should work for most (if not all) users then we are on same page. If some user want memory.high reclaim to happen in a separate thread instead of return-to-userspace or synchronously, this proposal provides mechanism through BPF to such users to achieve their goals.

I think there's a common reasonable solution here, which is deciding by who the charge is for. A task charging for itself gets return-to-userspace enforcement plus the checks in the explicit bulk operations above. A kthread or anything else charging on behalf of a cgroup shouldn't be throttled for the cgroup's overrun. The overrun is attributed to the cgroup, async reclaim is kicked, and the cgroup's own tasks absorb the throttling on their next return to userspace. The in-charge synchronous fallback goes away. Each case has one reasonable answer, so I don't see a policy choice to expose here.

We are in agreement on (1) and (2) completely. For (3), I am fine with removing the sync enforcement, but for throttling points for bulk operation sites, I think we should only add them when there is an actual use case for that or someone complains about overrun from those sites.

Now, setting aside the default behavior of memory.high, I want to provide additional flexibility to users for (3) specifically. One specific case is letting users opt in to async reclaim instead of the other forms of memory.high enforcement. Basically, users can specify that instead of having their application threads throttled, they would prefer async reclaim to bring their usage back below memory.high.

Hello, Shakeel.

Quoted 4 lines · Shakeel Butt

We are in agreement on (1) and (2) completely. For (3), I am fine with removing the sync enforcement, but for throttling points for bulk operation sites, I think we should only add them when there is an actual use case for that or someone complains about overrun from those sites.

On (3), if removing synchronous enforcement wouldn't regress anything, that's fine, but why was it added in the first place?

Quoted 6 lines

Now, setting aside the default behavior of memory.high, I want to provide additional flexibility to users for (3) specifically. One specific case is letting users opt in to async reclaim instead of the other forms of memory.high enforcement. Basically, users can specify that instead of having their application threads throttled, they would prefer async reclaim to bring their usage back below memory.high.

As for flexibility, we already have a gradient of enforcement around memory.high. Is the need here to make the shape of that gradient configurable? Can you give specific examples where this is needed?

Quoted 3 lines

On (3), if removing synchronous enforcement wouldn't regress anything, that's fine, but why was it added in the first place?

I added the sync enforcement to replace a Google internal feature which, on memcg OOM, allows node controller couple of seconds to either increase the max limit or let the memcg die. With sync enforcement, memory.high helped in simple benchmarks. However later testing on some realistic Google workloads, I found out that several thousand threads are very normal of typical Google workload and memory.high sync enforcement is not effective on applications with large amount of threads. In addition, there were workloads which on noticing blocked threads, keep forking more threads. At the end implementing that feature using memory.high didn't pan out.

Quoted 3 lines

As for flexibility, we already have a gradient of enforcement around memory.high. Is the need here to make the shape of that gradient configurable? Can you give specific examples where this is needed?

The concrete example I have is the kswapd like async reclaimers (plural) per memcg. Kswapd is woken up on free pages falling below low watermark and then when free pages fall below min watermark, allocators get throttled (enter direct reclaim). I want to apply similar concept to memcg (but with right cpu accounting and more concurrency).