linux-kernel - Re: [PATCH v2 1/3] mm: Fix dropped memcg from mem cgroup soft limit tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 4 Mar 2021 09:35:08 -0800
From:   Tim Chen <tim.c.chen@...ux.intel.com>
To:     Michal Hocko <mhocko@...e.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Johannes Weiner <hannes@...xchg.org>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Ying Huang <ying.huang@...el.com>, linux-mm@...ck.org,
        cgroups@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] mm: Fix dropped memcg from mem cgroup soft limit
 tree



On 2/18/21 11:13 AM, Michal Hocko wrote:

> 
> Fixes: 4e41695356fb ("memory controller: soft limit reclaim on contention")
> Acked-by: Michal Hocko <mhocko@...e.com>
> 
> Thanks!
>> ---
>>  mm/memcontrol.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index ed5cc78a8dbf..a51bf90732cb 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -3505,8 +3505,12 @@ unsigned long mem_cgroup_soft_limit_reclaim(pg_data_t *pgdat, int order,
>>  			loop > MEM_CGROUP_MAX_SOFT_LIMIT_RECLAIM_LOOPS))
>>  			break;
>>  	} while (!nr_reclaimed);
>> -	if (next_mz)
>> +	if (next_mz) {
>> +		spin_lock_irq(&mctz->lock);
>> +		__mem_cgroup_insert_exceeded(next_mz, mctz, excess);
>> +		spin_unlock_irq(&mctz->lock);
>>  		css_put(&next_mz->memcg->css);
>> +	}
>>  	return nr_reclaimed;
>>  }
>>  
>> -- 
>> 2.20.1
> 

Mel,

Reviewing this patch a bit more, I realize that there is a chance that the removed
next_mz could be inserted back to the tree from a memcg_check_events
that happen in between.  So we need to make sure that the next_mz
is indeed off the tree and update the excess value before adding it
back.  Update the patch to the patch below.

Thanks.

Tim
---

>From 412764d1fad219b04c77bcb1cc8161067c8424f2 Mon Sep 17 00:00:00 2001
From: Tim Chen <tim.c.chen@...ux.intel.com>
Date: Tue, 2 Feb 2021 15:53:21 -0800
Subject: [PATCH v3] mm: Fix dropped memcg from mem cgroup soft limit tree
To: Andrew Morton <akpm@...ux-foundation.org>, Johannes Weiner <hannes@...xchg.org>, Michal Hocko <mhocko@...e.cz>,Vladimir Davydov <vdavydov.dev@...il.com>
Cc: Dave Hansen <dave.hansen@...el.com>, Ying Huang <ying.huang@...el.com>, linux-mm@...ck.org, cgroups@...r.kernel.org, linux-kernel@...r.kernel.org

During soft limit memory reclaim, we will temporarily remove the target
mem cgroup from the cgroup soft limit tree.  We then perform memory
reclaim, update the memory usage excess count and re-insert the mem
cgroup back into the mem cgroup soft limit tree according to the new
memory usage excess count.

However, when memory reclaim failed for a maximum number of attempts
and we bail out of the reclaim loop, we forgot to put the target mem
cgroup chosen for next reclaim back to the soft limit tree. This prevented
pages in the mem cgroup from being reclaimed in the future even though
the mem cgroup exceeded its soft limit.  Fix the logic and put the mem
cgroup back on the tree when page reclaim failed for the mem cgroup.

Fixes: 4e41695356fb ("memory controller: soft limit reclaim on contention")
---
 mm/memcontrol.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index ed5cc78a8dbf..bc9cc73ff66b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3505,8 +3505,18 @@ unsigned long mem_cgroup_soft_limit_reclaim(pg_data_t *pgdat, int order,
 			loop > MEM_CGROUP_MAX_SOFT_LIMIT_RECLAIM_LOOPS))
 			break;
 	} while (!nr_reclaimed);
-	if (next_mz)
+	if (next_mz) {
+		/*
+		 * next_mz was removed in __mem_cgroup_largest_soft_limit_node.
+		 * Put it back in tree with latest excess value.
+		 */
+		spin_lock_irq(&mctz->lock);
+		__mem_cgroup_remove_exceeded(next_mz, mctz);
+		excess = soft_limit_excess(next_mz->memcg);
+		__mem_cgroup_insert_exceeded(next_mz, mctz, excess);
+		spin_unlock_irq(&mctz->lock);
 		css_put(&next_mz->memcg->css);
+	}
 	return nr_reclaimed;
 }
 
-- 
2.20.1