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

On Tue, Jul 14, 2026 at 3:43 PM Ridong Chen <[email protected]> wrote:
>
>
>
> On 7/14/2026 9:48 AM, Barry Song wrote:
> > On Tue, Jul 14, 2026 at 9:43 AM Barry Song <[email protected]> wrote:
> >>
> >> On Tue, Jul 14, 2026 at 9:20 AM Ridong Chen <[email protected]> wrote:
> >>>
> >>>
> >>>
> >>> On 7/13/2026 11:08 PM, Barry Song wrote:
> >>>> On Sat, Jul 11, 2026 at 5:12 PM Ridong Chen <[email protected]> wrote:
> >>>>>
> >>>>> From: Ridong Chen <[email protected]>
> >>>>>
> >>>>> The per-memcg swappiness knob is v1-only; v2 always uses global
> >>>>> vm_swappiness and ignores the per-cgroup field.
> >>>>>
> >>>>> Guard memcg->swappiness with CONFIG_MEMCG_V1, and move the helper
> >>>>> to memcontrol.h where it belongs.
> >>>>>
> >>>>> No functional change for v1; v2-only kernels drop the unused field.
> >>>>>
> >>>>> Signed-off-by: Ridong Chen <[email protected]>
> >>>>> Acked-by: Johannes Weiner <[email protected]>
> >>>>
> >>>> Reviewed-by: Barry Song <[email protected]>
> >>>>
> >>>> With some nits.
> >>>>
> >>>>> ---
> >>>> [...]
> >>>>>           struct mem_cgroup_per_node *nodeinfo[];
> >>>>> @@ -365,6 +366,9 @@ enum objext_flags {
> >>>>>
> >>>>>    #define OBJEXTS_FLAGS_MASK (__NR_OBJEXTS_FLAGS - 1)
> >>>>>
> >>>>> +/* Defined in mm/vmscan.c; used by mem_cgroup_swappiness(). */
> >>>>> +extern int vm_swappiness;
> >>>>
> >>>> This is a bit unusual. I'm not sure whether mm/swap.h would be
> >>>> a more appropriate place for this.
> >>>>
> >>> Thank you for your reply.
> >>>
> >>> The vm_swappiness variable is not utilized within mm/swap.c.
> >>> Furthermore, since memcontrol.h does not include swap.h, retaining the
> >>> extern int vm_swappiness declaration in mm/swap.h will result in a
> >>> compilation failure.
> >>
> >> 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@gmail.com/
>
> 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?

Best Regards
Barry