Re: [PATCH] mm: Do not deactivate when the cgroup has plenty inactive page

[zhong jiang <zhongjiang-ali@linux.alibaba.com>]

On 2020/9/25 8:07 下午, Michal Hocko wrote:
> On Fri 25-09-20 19:49:12, zhongjiang-ali wrote:
>> After appling the series patches(mm: fix page aging across multiple cgroups),
>> cgroup memory reclaim strategy is based on reclaim root's inactive:active
>> ratio. if the target lruvec need to deactivate, its children cgroup also will
>> deactivate. That will result in hot page to be reclaimed and other cgroup's
>> cold page will be left, which is not expected.
>>
>> The patch will not force deactivate when inactive_is_low is not true unless
>> we has scanned the inactive list and memory is unable to reclaim.
> Do you have any data to present?

I write an testcase that cgroup B has a lot of hot pagecache and cgroup 
C  is full of cold pagecache.  and

their parent cgroup A will trigger the reclaim due of it's limit has 
been breached.


The testcase should assume that we should not reclaim the  hot pagecache 
in cgroup B because C has

plenty cold pagecache.   Unfortunately,  I can see cgroup B hot 
pagecache has been decreased when

cgroup A trigger the reclaim.


Thanks,

>
>> Signed-off-by: zhongjiang-ali <zhongjiang-ali@linux.alibaba.com>
>> ---
>>   mm/vmscan.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 466fc31..77d395f 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -2407,8 +2407,21 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
>>   		case SCAN_FILE:
>>   		case SCAN_ANON:
>>   			/* Scan one type exclusively */
>> -			if ((scan_balance == SCAN_FILE) != file)
>> +			if ((scan_balance == SCAN_FILE) != file) {
>>   				scan = 0;
>> +			} else {
>> +				/*
>> +				 * Reclaim memory is based on the root's inactive: active
>> +				 * ratio, but it is possible that silbing cgroup has a lot
>> +				 * of cold memory to reclaim rather than reclaim the hot
>> +				 * cache in the current cgroup.
>> +				 */
>> +				if (!sc->force_deactivate && is_active_lru(lru) &&
>> +						!inactive_is_low(lruvec, lru - LRU_ACTIVE)) {
>> +					if (sc->may_deactivate & (1 << file))
>> +						scan = 0;
>> +				}
>> +			}
>>   			break;
>>   		default:
>>   			/* Look ma, no brain */
>> -- 
>> 1.8.3.1
>>