Re: [PATCH] memcg: allow exiting tasks to write back data to swap

[Balbir Singh]

On 12/12/24 12:21, Rik van Riel wrote:
> On Thu, 2024-12-12 at 10:15 +1100, Balbir Singh wrote:
>> On 12/12/24 02:53, Rik van Riel wrote:
>>>
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index 7b3503d12aaf..03d77e93087e 100644
>>> --- a/mm/memcontrol.c
>>> +++ b/mm/memcontrol.c
>>> @@ -5371,6 +5371,15 @@ bool
>>> mem_cgroup_zswap_writeback_enabled(struct mem_cgroup *memcg)
>>>  	if (!zswap_is_enabled())
>>>  		return true;
>>>  
>>> +	/*
>>> +	 * Always allow exiting tasks to push data to swap. A
>>> process in
>>> +	 * the middle of exit cannot get OOM killed, but may need
>>> to push
>>> +	 * uncompressible data to swap in order to get the cgroup
>>> memory
>>> +	 * use below the limit, and make progress with the exit.
>>> +	 */
>>> +	if ((current->flags & PF_EXITING) && memcg ==
>>> mem_cgroup_from_task(current))
>>> +		return true;
>>> +
>>>  	for (; memcg; memcg = parent_mem_cgroup(memcg))
>>>  		if (!READ_ONCE(memcg->zswap_writeback))
>>>  			return false;
>>
>> Rik,
>>
>> I am unable to understand the motivation here, so we want 
>> mem_cgroup_zswap_writeback_enabled() to return true, it only
>> returns false if a memcg in the hierarchy has zswap_writeback
>> set to 0 (false). In my git-grep I can't seem to find how/why
>> that may be the case. I can see memcg starts of with the value
>> set to true, if CONFIG_ZSWAP is enabled.
>>
>> Your changelog above makes sense, but I am unable to map it to
>> the code changes.
>>
> 
> Wait, are you asking about the code that I'm
> adding, or about the code that was already
> there?
> 
> I want to add the code that allows zswap
> writeback if the reclaiming task is exiting,
> and in the same cgroup as the to be written
> back memory.
>

I was asking about this change (this patch), I know that the return
true will help avoid the PAGE_ACTIVATE path, but I am not sure why
this function will return false if CONFIG_ZSWAP is enabled (unless
zswap_writeback is turned off in one of the groups)

Balbir