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

On 7/14/2026 6:21 PM, Barry Song wrote:
> 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?
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?

```
...
#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
}
...
```


-- 
Best regards
Ridong