Re: [PATCH 1/2] memcg: move mem_cgroup_swappiness to memcontrol.h
Ridong Chen <[email protected]>
| Newsgroups | org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
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. -- Best regards Ridong