Re: [PATCH 1/2] memcg: move mem_cgroup_swappiness to memcontrol.h

[Barry Song]

On Tue, Jul 14, 2026 at 7:31 PM Ridong Chen <ridong.chen@xxxxxxxxx> wrote:

[...]
> >>>>
> >>>> If this is the case, it still seems better to keep
> >>>> extern int vm_swappiness in include/linux/swap.h.
> >>>>
> >>>> Then we don't need the comment:
> >>>> /* Defined in mm/vmscan.c; used by mem_cgroup_swappiness(). */
> >>>>
> >>>> It also makes it clearer that vm_swappiness is an extern variable
> >>>> belonging to the swap module, rather than the memcontrol module.
> >>>
> >>> BTW, if mem_c_group_swappiness() and vm_swappiness are only used
> >>> within mm/, could all of these be moved to mm/swap.h and
> >>> mm/internal.h instead?
> >>>
> >>> We are making a big effort to move many unrelated things out of
> >>> include/linux/swap.h recently. Could you check?
> >>>
> >>> https://lore.kernel.org/linux-mm/20260708-ch-swap-series-plus-folio-lru-cleanup-v9-0-2bc72b4f8730@xxxxxxxxx/
> >>
> >> Good suggestion. Moving them to mm/internal.h makes sense. Will update
> >> in the next version.
> >
> > Either mm/swap.h or mm/internal.h.
> > vm_swappiness probably belongs in mm/swap.h rather than
> > mm/internal.h, right?
> >
> > BTW, I am not particularly eager about this cleanup;
> > it could be done later as a separate patch.
> >
> > If you decide not to do the cleanup, I think it would be better to
> > leave "extern int vm_swappiness" in include/linux/swap.h rather than
> > declaring it in include/linux/memcontrol.h?
> I noticed that some swap-related macros are already defined in
> mm/internal.h, so I'm planning to place the code right after them for
> better grouping. Does that work for you?

We also have mm/swap.h, which is dedicated to exporting
swap-related things. Is it a better place than mm/internal.h?

>
> ```
> ...
> #define MEMCG_RECLAIM_MAY_SWAP (1 << 1)
> #define MEMCG_RECLAIM_PROACTIVE (1 << 2)
> #define MIN_SWAPPINESS 0
> #define MAX_SWAPPINESS 200
>
> /* Just reclaim from anon folios in proactive memory reclaim */
> #define SWAPPINESS_ANON_ONLY (MAX_SWAPPINESS + 1)
>
> extern int vm_swappiness;
>
> static inline int mem_cgroup_swappiness(struct mem_cgroup *memcg)
> {
> #ifdef CONFIG_MEMCG_V1
>         /* Cgroup2 doesn't have per-cgroup swappiness */
>         if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
>                 return READ_ONCE(vm_swappiness);
>
>         /* root ? */
>         if (mem_cgroup_disabled() || mem_cgroup_is_root(memcg))
>                 return READ_ONCE(vm_swappiness);
>
>         return READ_ONCE(memcg->swappiness);
> #else
>         return READ_ONCE(vm_swappiness);
> #endif
> }
> ...
> ```