Re: [PATCH v13] mm/bpf: Add bpf_proactive_reclaim kfunc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/bpf: anonymous swappiness enum is omitted from BTF
--

commit 1eb287ed1b56fa630faef31e3bfc3e5da668a9da
Author: Hui Zhu <zhuhui@kylinos.cn>

mm/bpf: Add bpf_proactive_reclaim kfunc

This commit adds the bpf_proactive_reclaim() kfunc to allow BPF programs
to proactively reclaim memory from a memory cgroup. It also converts
swappiness macros into an enum with the stated goal of emitting them into
BTF for use by BPF programs via vmlinux.h.

> diff --git a/mm/bpf_memcontrol.c b/mm/bpf_memcontrol.c
> index 716df49d76477..d8f579c285600 100644
> --- a/mm/bpf_memcontrol.c
> +++ b/mm/bpf_memcontrol.c

[ ... ]

> +/**
> + * bpf_proactive_reclaim - proactively reclaim memory from a memory cgroup
> + * @memcg: the target memory cgroup to reclaim from.
> + * @size: the amount of memory to reclaim, in bytes, clamped to
> + *        MEMCG_CHARGE_BATCH.
> + * @swappiness: the reclaim swappiness, in the range [MIN_SWAPPINESS,
> + *              MAX_SWAPPINESS], or one of the special modes: -1 to use
> + *              the memcg's own swappiness, or SWAPPINESS_ANON_ONLY to
> + *              reclaim only anon folios.
> + *
> + * Performs one proactive reclaim pass on @memcg, like a write to
> + * memory.reclaim but without retrying until @size is reached. Call it
> + * repeatedly to reclaim more than one batch.
> + *
> + * Only available to BPF_PROG_TYPE_SYSCALL, because other sleepable programs
> + * may run with filesystem locks held, which the reclaim path can deadlock
> + * on via filesystem shrinkers.
> + *
> + * Return: The amount of memory reclaimed, in bytes, or a negative error.
> + */
> +__bpf_kfunc long bpf_proactive_reclaim(struct mem_cgroup *memcg,
> +				       unsigned long size,
> +				       int swappiness)

[Severity: Medium]
Since the function signature uses int swappiness instead of an enum type,
will the compiler ever reference the newly added enum?

Without a reference in the signature, the enum is treated as purely internal
to the compilation unit and disconnected from the kfunc.

> +{
> +	unsigned long nr_reclaimed;
> +	unsigned long nr_pages;
> +
> +	if (swappiness != -1 && swappiness != SWAPPINESS_ANON_ONLY) {
> +		if (swappiness < MIN_SWAPPINESS || swappiness > MAX_SWAPPINESS)
> +			return -EINVAL;
> +	}

[ ... ]

> diff --git a/mm/internal.h b/mm/internal.h
> index 0434dfcfc36f1..4d426663ddd81 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h

[ ... ]

>  #define MEMCG_RECLAIM_MAY_SWAP (1 << 1)
>  #define MEMCG_RECLAIM_PROACTIVE (1 << 2)
> -#define MIN_SWAPPINESS 0
> -#define MAX_SWAPPINESS 200
> +enum {
> +	MIN_SWAPPINESS = 0,
> +	MAX_SWAPPINESS = 200,
>  
> -/* Just reclaim from anon folios in proactive memory reclaim */
> -#define SWAPPINESS_ANON_ONLY (MAX_SWAPPINESS + 1)
> +	/* Just reclaim from anon folios in proactive memory reclaim */
> +	SWAPPINESS_ANON_ONLY = MAX_SWAPPINESS + 1,
> +};

[Severity: Medium]
The commit message states these are converted to an enum so that they are
emitted into BTF. Because this is an anonymous enum that isn't used to type
any variable or function argument, will it actually be preserved in DWARF/BTF?

It appears pahole omits completely unreferenced anonymous enums, meaning
these constants will not be emitted into vmlinux.h as intended.

Could we give this enum a name and use it in the bpf_proactive_reclaim()
signature to ensure it gets generated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921030854.17858-1-hui.zhu@linux.dev?part=1