Re: [PATCH 1/2] memcg: move mem_cgroup_swappiness to memcontrol.h
Barry Song <[email protected]>
| Newsgroups | org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <CAGsJ_4w8P8ERoXJY70wXf71MYEA16vX+JMAaE4sfB4=OqzN_Pg@mail.gmail.com> |
On Tue, Jul 14, 2026 at 7:31 PM Ridong Chen <[email protected]> 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@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? > 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 > } > ... > ```