Re: [PATCH 1/2] mm/memcg: try harder to decrease [memory,memsw].limit_in_bytes

[Andrey Ryabinin]

On 12/20/2017 01:33 PM, Michal Hocko wrote:
> On Wed 20-12-17 13:24:28, Andrey Ryabinin wrote:
>> mem_cgroup_resize_[memsw]_limit() tries to free only 32 (SWAP_CLUSTER_MAX)
>> pages on each iteration. This makes practically impossible to decrease
>> limit of memory cgroup. Tasks could easily allocate back 32 pages,
>> so we can't reduce memory usage, and once retry_count reaches zero we return
>> -EBUSY.
>>
>> It's easy to reproduce the problem by running the following commands:
>>
>>   mkdir /sys/fs/cgroup/memory/test
>>   echo $$ >> /sys/fs/cgroup/memory/test/tasks
>>   cat big_file > /dev/null &
>>   sleep 1 && echo $((100*1024*1024)) > /sys/fs/cgroup/memory/test/memory.limit_in_bytes
>>   -bash: echo: write error: Device or resource busy
>>
>> Instead of trying to free small amount of pages, it's much more
>> reasonable to free 'usage - limit' pages.
> 
> But that only makes the issue less probable. It doesn't fix it because 
> 		if (curusage >= oldusage)
> 			retry_count--;
> can still be true because allocator might be faster than the reclaimer.
> Wouldn't it be more reasonable to simply remove the retry count and keep
> trying until interrupted or we manage to update the limit.

But does it makes sense to continue reclaiming even if reclaimer can't make any
progress? I'd say no. "Allocator is faster than reclaimer" may be not the only reason
for failed reclaim. E.g. we could try to set limit lower than amount of mlock()ed memory
in cgroup, retrying reclaim would be just a waste of machine's resources.
Or we simply don't have any swap, and anon > new_limit. Should be burn the cpu in that case?


> Another option would be to commit the new limit and allow temporal overcommit
> of the hard limit. New allocations and the limit update paths would
> reclaim to the hard limit.
> 

It sounds a bit fragile and tricky to me. I wouldn't go that way without unless we have a very good reason for this.