[RH7] mm: Fix race between reparenting memcg and list_lru_del()

Submitted by Kirill Tkhai on Dec. 1, 2020, 10:34 a.m.

Details

Message ID 160681879697.785824.8485370656006116337.stgit@localhost.localdomain
State New
Series "mm: Fix race between reparenting memcg and list_lru_del()"
Headers show

Commit Message

Kirill Tkhai Dec. 1, 2020, 10:34 a.m. 
From: Roman Gushchin <guro@fb.com>

On reparenting struct list_lru_one::nr_items may become
negative, so all the shrinker bits logic works as not expected.

This leads to cleared shrinker bit while LRU is not
actually empty.

(We will pull description from ms git later, when it's available).

https://lkml.org/lkml/2020/11/30/1093

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 mm/list_lru.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Patch hide | download patch | download mbox 

diff --git a/mm/list_lru.c b/mm/list_lru.c
index 21e12a8364ff..05d517197fbe 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -511,7 +511,6 @@  static void memcg_drain_list_lru_node(struct list_lru *lru, int nid,
 	struct list_lru_node *nlru = &lru->node[nid];
 	int dst_idx = memcg_cache_id(dst_memcg);
 	struct list_lru_one *src, *dst;
-	bool set;
 
 	/*
 	 * Since list_lru_{add,del} may be called under an IRQ-safe lock,
@@ -523,9 +522,8 @@  static void memcg_drain_list_lru_node(struct list_lru *lru, int nid,
 	dst = list_lru_from_memcg_idx(nlru, dst_idx);
 
 	list_splice_init(&src->list, &dst->list);
-	set = (!dst->nr_items && src->nr_items);
 	dst->nr_items += src->nr_items;
-	if (set)
+	if (src->nr_items)
 		memcg_set_shrinker_bit(dst_memcg, nid, lru_shrinker_id(lru));
 	src->nr_items = 0;

Comments

Vasily Averin Dec. 3, 2020, 9:19 a.m. 
I'm going to wait till final patch version will be committed upstream

Kirill,
I belive you will be notified, so please let me know when this happen.

Thank you,
	Vasily Averin

On 12/1/20 1:34 PM, Kirill Tkhai wrote:
> From: Roman Gushchin <guro@fb.com>
> 
> On reparenting struct list_lru_one::nr_items may become
> negative, so all the shrinker bits logic works as not expected.
> 
> This leads to cleared shrinker bit while LRU is not
> actually empty.
> 
> (We will pull description from ms git later, when it's available).
> 
> https://lkml.org/lkml/2020/11/30/1093
> 
> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  mm/list_lru.c |    4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index 21e12a8364ff..05d517197fbe 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -511,7 +511,6 @@ static void memcg_drain_list_lru_node(struct list_lru *lru, int nid,
>  	struct list_lru_node *nlru = &lru->node[nid];
>  	int dst_idx = memcg_cache_id(dst_memcg);
>  	struct list_lru_one *src, *dst;
> -	bool set;
>  
>  	/*
>  	 * Since list_lru_{add,del} may be called under an IRQ-safe lock,
> @@ -523,9 +522,8 @@ static void memcg_drain_list_lru_node(struct list_lru *lru, int nid,
>  	dst = list_lru_from_memcg_idx(nlru, dst_idx);
>  
>  	list_splice_init(&src->list, &dst->list);
> -	set = (!dst->nr_items && src->nr_items);
>  	dst->nr_items += src->nr_items;
> -	if (set)
> +	if (src->nr_items)
>  		memcg_set_shrinker_bit(dst_memcg, nid, lru_shrinker_id(lru));
>  	src->nr_items = 0;
>  
> 
>
Kirill Tkhai Dec. 3, 2020, 9:40 a.m. 
Pull from here: https://ozlabs.org/~akpm/mmotm/broken-out/mm-list_lru-set-shrinker-map-bit-when-child-nr_items-is-not-zero.patch

On 03.12.2020 12:19, Vasily Averin wrote:
> I'm going to wait till final patch version will be committed upstream
> 
> Kirill,
> I belive you will be notified, so please let me know when this happen.
> 
> Thank you,
> 	Vasily Averin
> 
> On 12/1/20 1:34 PM, Kirill Tkhai wrote:
>> From: Roman Gushchin <guro@fb.com>
>>
>> On reparenting struct list_lru_one::nr_items may become
>> negative, so all the shrinker bits logic works as not expected.
>>
>> This leads to cleared shrinker bit while LRU is not
>> actually empty.
>>
>> (We will pull description from ms git later, when it's available).
>>
>> https://lkml.org/lkml/2020/11/30/1093
>>
>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
>> ---
>>  mm/list_lru.c |    4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/mm/list_lru.c b/mm/list_lru.c
>> index 21e12a8364ff..05d517197fbe 100644
>> --- a/mm/list_lru.c
>> +++ b/mm/list_lru.c
>> @@ -511,7 +511,6 @@ static void memcg_drain_list_lru_node(struct list_lru *lru, int nid,
>>  	struct list_lru_node *nlru = &lru->node[nid];
>>  	int dst_idx = memcg_cache_id(dst_memcg);
>>  	struct list_lru_one *src, *dst;
>> -	bool set;
>>  
>>  	/*
>>  	 * Since list_lru_{add,del} may be called under an IRQ-safe lock,
>> @@ -523,9 +522,8 @@ static void memcg_drain_list_lru_node(struct list_lru *lru, int nid,
>>  	dst = list_lru_from_memcg_idx(nlru, dst_idx);
>>  
>>  	list_splice_init(&src->list, &dst->list);
>> -	set = (!dst->nr_items && src->nr_items);
>>  	dst->nr_items += src->nr_items;
>> -	if (set)
>> +	if (src->nr_items)
>>  		memcg_set_shrinker_bit(dst_memcg, nid, lru_shrinker_id(lru));
>>  	src->nr_items = 0;
>>  
>>
>>