Re: [RFC PATCH v3 bpf-next 4/5] sched: add BPF-based per-cgroup NUMA balancing mode selection

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2148 bytes --]

> sched: add BPF-based per-cgroup NUMA balancing mode selection

TL;DR: Comment only: the comment on task_numab_in_tiering_mode() may describe
the return value backwards, since task_numa_fault() skips non-top-tier
statistics when it returns true.

> diff --git a/include/linux/sched/numa_balancing.h b/include/linux/sched/numa_balancing.h
> index 36de57f7fb06..d63891a8be01 100644
> --- a/include/linux/sched/numa_balancing.h
> +++ b/include/linux/sched/numa_balancing.h

[ ... ]

> @@ -53,6 +95,68 @@ static inline bool numab_in_normal_mode(void)
>  {
>  	return sysctl_numa_balancing_mode & NUMA_BALANCING_NORMAL;
>  }
> +
> +/*
> + * Whether @p runs in tiering mode, i.e. whether faults on non-top-tier
> + * memory still feed its NUMA statistics. The requests of the task's
> + * cgroup and of every ancestor are combined; an opt-out wins, otherwise
> + * the bits are read independently, as the sysctl reads them. Without
> + * any request, the global sysctl decides.
> + */
> +static inline bool task_numab_in_tiering_mode(struct task_struct *p)
> +{
> +#ifdef CONFIG_BPF_NUMAB
> +	if (static_branch_unlikely(&numab_cgroup_mode_set)) {
> +		int mode = task_numab_mode(p);
> +
> +		if (mode != NUMAB_FOLLOW_SYSTEM)
> +			return mode & NUMA_BALANCING_MEMORY_TIERING;
> +	}
> +#endif
> +	return numab_in_tiering_mode();
> +}

The comment above task_numab_in_tiering_mode() says it reports "whether
faults on non-top-tier memory still feed its NUMA statistics".  Is that
the right way around?

The only caller, task_numa_fault() in kernel/sched/fair.c, uses it like
this:

	if (!node_is_toptier(mem_node) &&
	    (task_numab_in_tiering_mode(p) || !cpupid_valid(last_cpupid)))
		return;

so a true return makes task_numa_fault() bail out before recording any
statistics for non-top-tier memory.  Should the comment say that faults on
non-top-tier memory are kept out of the task's NUMA statistics instead?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/39213018427