[RFC v1] memcg: add memcg lru for page reclaiming

[RFC v1] memcg: add memcg lru for page reclaiming

[Hillf Danton]

Currently soft limit reclaim is frozen, see
Documentation/admin-guide/cgroup-v2.rst for reasons.

Copying the page lru idea, memcg lru is added for selecting victim
memcg to reclaim pages from under memory pressure. It now works in
parallel to slr not only because the latter needs some time to reap
but the coexistence facilitates it a lot to add the lru in a straight
forward manner.

A lru list paired with a spin lock is added, thanks to the current
memcg high_work that provides other things it needs, and a couple of
helpers to add memcg to and pick victim from lru.

V1 is based on 5.4-rc3.

Changes since v0
- add MEMCG_LRU in init/Kconfig
- drop changes in mm/vmscan.c
- make memcg lru work in parallel to slr

Cc: Chris Down <chris@chrisdown.name>
Cc: Tejun Heo <tj@kernel.org>
Cc: Roman Gushchin <guro@fb.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Mel Gorman <mgorman@suse.de>
Signed-off-by: Hillf Danton <hdanton@sina.com>
---

--- a/init/Kconfig
+++ b/init/Kconfig
@@ -843,6 +843,14 @@ config MEMCG
 	help
 	  Provides control over the memory footprint of tasks in a cgroup.
 
+config MEMCG_LRU
+	bool
+	depends on MEMCG
+	help
+	  Select victim memcg on lru for page reclaiming.
+
+	  Say N if unsure.
+
 config MEMCG_SWAP
 	bool "Swap controller"
 	depends on MEMCG && SWAP
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -223,6 +223,10 @@ struct mem_cgroup {
 	/* Upper bound of normal memory consumption range */
 	unsigned long high;
 
+#ifdef CONFIG_MEMCG_LRU
+	struct list_head lru_node;
+#endif
+
 	/* Range enforcement for interrupt charges */
 	struct work_struct high_work;
 
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2338,14 +2338,54 @@ static int memcg_hotplug_cpu_dead(unsign
 	return 0;
 }
 
+#ifdef CONFIG_MEMCG_LRU
+static DEFINE_SPINLOCK(memcg_lru_lock);
+static LIST_HEAD(memcg_lru);	/* a copy of page lru */
+
+static void memcg_add_lru(struct mem_cgroup *memcg)
+{
+	spin_lock_irq(&memcg_lru_lock);
+	if (list_empty(&memcg->lru_node))
+		list_add_tail(&memcg->lru_node, &memcg_lru);
+	spin_unlock_irq(&memcg_lru_lock);
+}
+
+static struct mem_cgroup *memcg_pick_lru(void)
+{
+	struct mem_cgroup *memcg, *next;
+
+	spin_lock_irq(&memcg_lru_lock);
+
+	list_for_each_entry_safe(memcg, next, &memcg_lru, lru_node) {
+		list_del_init(&memcg->lru_node);
+
+		if (page_counter_read(&memcg->memory) > memcg->high) {
+			spin_unlock_irq(&memcg_lru_lock);
+			return memcg;
+		}
+	}
+	spin_unlock_irq(&memcg_lru_lock);
+
+	return NULL;
+}
+#endif
+
 static void reclaim_high(struct mem_cgroup *memcg,
 			 unsigned int nr_pages,
 			 gfp_t gfp_mask)
 {
+#ifdef CONFIG_MEMCG_LRU
+	struct mem_cgroup *start = memcg;
+#endif
 	do {
 		if (page_counter_read(&memcg->memory) <= memcg->high)
 			continue;
 		memcg_memory_event(memcg, MEMCG_HIGH);
+		if (IS_ENABLED(CONFIG_MEMCG_LRU))
+			if (start != memcg) {
+				memcg_add_lru(memcg);
+				return;
+			}
 		try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask, true);
 	} while ((memcg = parent_mem_cgroup(memcg)));
 }
@@ -3158,6 +3198,13 @@ unsigned long mem_cgroup_soft_limit_recl
 	unsigned long excess;
 	unsigned long nr_scanned;
 
+	if (IS_ENABLED(CONFIG_MEMCG_LRU)) {
+		struct mem_cgroup *memcg = memcg_pick_lru();
+		if (memcg)
+			schedule_work(&memcg->high_work);
+		return 0;
+	}
+
 	if (order > 0)
 		return 0;
 
@@ -5068,6 +5115,8 @@ static struct mem_cgroup *mem_cgroup_all
 	if (memcg_wb_domain_init(memcg, GFP_KERNEL))
 		goto fail;
 
+	if (IS_ENABLED(CONFIG_MEMCG_LRU))
+		INIT_LIST_HEAD(&memcg->lru_node);
 	INIT_WORK(&memcg->high_work, high_work_func);
 	memcg->last_scanned_node = MAX_NUMNODES;
 	INIT_LIST_HEAD(&memcg->oom_notify);
--

Re: [RFC v1] memcg: add memcg lru for page reclaiming

[Michal Hocko]

On Mon 21-10-19 19:56:54, Hillf Danton wrote:
> 
> Currently soft limit reclaim is frozen, see
> Documentation/admin-guide/cgroup-v2.rst for reasons.
> 
> Copying the page lru idea, memcg lru is added for selecting victim
> memcg to reclaim pages from under memory pressure. It now works in
> parallel to slr not only because the latter needs some time to reap
> but the coexistence facilitates it a lot to add the lru in a straight
> forward manner.

This doesn't explain what is the problem/feature you would like to
fix/achieve. It also doesn't explain the overall design. 

> A lru list paired with a spin lock is added, thanks to the current
> memcg high_work that provides other things it needs, and a couple of
> helpers to add memcg to and pick victim from lru.
> 
> V1 is based on 5.4-rc3.
> 
> Changes since v0
> - add MEMCG_LRU in init/Kconfig
> - drop changes in mm/vmscan.c
> - make memcg lru work in parallel to slr
> 
> Cc: Chris Down <chris@chrisdown.name>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Roman Gushchin <guro@fb.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Shakeel Butt <shakeelb@google.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Mel Gorman <mgorman@suse.de>
> Signed-off-by: Hillf Danton <hdanton@sina.com>
> ---
> 
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -843,6 +843,14 @@ config MEMCG
>  	help
>  	  Provides control over the memory footprint of tasks in a cgroup.
>  
> +config MEMCG_LRU
> +	bool
> +	depends on MEMCG
> +	help
> +	  Select victim memcg on lru for page reclaiming.
> +
> +	  Say N if unsure.
> +
>  config MEMCG_SWAP
>  	bool "Swap controller"
>  	depends on MEMCG && SWAP
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -223,6 +223,10 @@ struct mem_cgroup {
>  	/* Upper bound of normal memory consumption range */
>  	unsigned long high;
>  
> +#ifdef CONFIG_MEMCG_LRU
> +	struct list_head lru_node;
> +#endif
> +
>  	/* Range enforcement for interrupt charges */
>  	struct work_struct high_work;
>  
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2338,14 +2338,54 @@ static int memcg_hotplug_cpu_dead(unsign
>  	return 0;
>  }
>  
> +#ifdef CONFIG_MEMCG_LRU
> +static DEFINE_SPINLOCK(memcg_lru_lock);
> +static LIST_HEAD(memcg_lru);	/* a copy of page lru */
> +
> +static void memcg_add_lru(struct mem_cgroup *memcg)
> +{
> +	spin_lock_irq(&memcg_lru_lock);
> +	if (list_empty(&memcg->lru_node))
> +		list_add_tail(&memcg->lru_node, &memcg_lru);
> +	spin_unlock_irq(&memcg_lru_lock);
> +}
> +
> +static struct mem_cgroup *memcg_pick_lru(void)
> +{
> +	struct mem_cgroup *memcg, *next;
> +
> +	spin_lock_irq(&memcg_lru_lock);
> +
> +	list_for_each_entry_safe(memcg, next, &memcg_lru, lru_node) {
> +		list_del_init(&memcg->lru_node);
> +
> +		if (page_counter_read(&memcg->memory) > memcg->high) {
> +			spin_unlock_irq(&memcg_lru_lock);
> +			return memcg;
> +		}
> +	}
> +	spin_unlock_irq(&memcg_lru_lock);
> +
> +	return NULL;
> +}
> +#endif
> +
>  static void reclaim_high(struct mem_cgroup *memcg,
>  			 unsigned int nr_pages,
>  			 gfp_t gfp_mask)
>  {
> +#ifdef CONFIG_MEMCG_LRU
> +	struct mem_cgroup *start = memcg;
> +#endif
>  	do {
>  		if (page_counter_read(&memcg->memory) <= memcg->high)
>  			continue;
>  		memcg_memory_event(memcg, MEMCG_HIGH);
> +		if (IS_ENABLED(CONFIG_MEMCG_LRU))
> +			if (start != memcg) {
> +				memcg_add_lru(memcg);
> +				return;
> +			}
>  		try_to_free_mem_cgroup_pages(memcg, nr_pages, gfp_mask, true);
>  	} while ((memcg = parent_mem_cgroup(memcg)));
>  }
> @@ -3158,6 +3198,13 @@ unsigned long mem_cgroup_soft_limit_recl
>  	unsigned long excess;
>  	unsigned long nr_scanned;
>  
> +	if (IS_ENABLED(CONFIG_MEMCG_LRU)) {
> +		struct mem_cgroup *memcg = memcg_pick_lru();
> +		if (memcg)
> +			schedule_work(&memcg->high_work);
> +		return 0;
> +	}
> +
>  	if (order > 0)
>  		return 0;
>  
> @@ -5068,6 +5115,8 @@ static struct mem_cgroup *mem_cgroup_all
>  	if (memcg_wb_domain_init(memcg, GFP_KERNEL))
>  		goto fail;
>  
> +	if (IS_ENABLED(CONFIG_MEMCG_LRU))
> +		INIT_LIST_HEAD(&memcg->lru_node);
>  	INIT_WORK(&memcg->high_work, high_work_func);
>  	memcg->last_scanned_node = MAX_NUMNODES;
>  	INIT_LIST_HEAD(&memcg->oom_notify);
> --
> 

-- 
Michal Hocko
SUSE Labs

Re: [RFC v1] memcg: add memcg lru for page reclaiming

[Hillf Danton]

On Mon, 21 Oct 2019 14:14:53 +0200 Michal Hocko wrote:
> 
> On Mon 21-10-19 19:56:54, Hillf Danton wrote:
> > 
> > Currently soft limit reclaim is frozen, see
> > Documentation/admin-guide/cgroup-v2.rst for reasons.
> > 
> > Copying the page lru idea, memcg lru is added for selecting victim
> > memcg to reclaim pages from under memory pressure. It now works in
> > parallel to slr not only because the latter needs some time to reap
> > but the coexistence facilitates it a lot to add the lru in a straight
> > forward manner.
> 
> This doesn't explain what is the problem/feature you would like to
> fix/achieve. It also doesn't explain the overall design. 

1, memcg lru makes page reclaiming hierarchy aware

While doing the high work, memcgs are currently reclaimed one after
another up through the hierarchy; in this RFC after ripping pages off
the first victim, the work finishes with the first ancestor of the victim
added to lru.

Recaliming is defered until kswapd becomes active.

2, memcg lru tries much to avoid overreclaim

Only one memcg is picked off lru in FIFO mode under memory pressure,
and MEMCG_CHARGE_BATCH pages are reclaimed one memcg at a time.

In next version, a new function will be added for kswapd to call,

	void memcg_try_to_free_pages(void)

with CONFIG_MEMCG_LRU dropped and mem_cgroup_soft_limit_reclaim()
untouched.

Thanks
Hillf

Re: [RFC v1] memcg: add memcg lru for page reclaiming

[Michal Hocko]

On Tue 22-10-19 21:30:50, Hillf Danton wrote:
> 
> On Mon, 21 Oct 2019 14:14:53 +0200 Michal Hocko wrote:
> > 
> > On Mon 21-10-19 19:56:54, Hillf Danton wrote:
> > > 
> > > Currently soft limit reclaim is frozen, see
> > > Documentation/admin-guide/cgroup-v2.rst for reasons.
> > > 
> > > Copying the page lru idea, memcg lru is added for selecting victim
> > > memcg to reclaim pages from under memory pressure. It now works in
> > > parallel to slr not only because the latter needs some time to reap
> > > but the coexistence facilitates it a lot to add the lru in a straight
> > > forward manner.
> > 
> > This doesn't explain what is the problem/feature you would like to
> > fix/achieve. It also doesn't explain the overall design. 
> 
> 1, memcg lru makes page reclaiming hierarchy aware

Is that a problem statement or a design goal?

> While doing the high work, memcgs are currently reclaimed one after
> another up through the hierarchy;

Which is the design because it is the the memcg where the high limit got
hit. The hierarchical behavior ensures that the subtree of that memcg is
reclaimed and we try to spread the reclaim fairly over the tree.

> in this RFC after ripping pages off
> the first victim, the work finishes with the first ancestor of the victim
> added to lru.
> 
> Recaliming is defered until kswapd becomes active.

This is a wrong assumption because high limit might be configured way
before kswapd is woken up.

> 2, memcg lru tries much to avoid overreclaim

Again, is this a problem statement or a design goal?
 
> Only one memcg is picked off lru in FIFO mode under memory pressure,
> and MEMCG_CHARGE_BATCH pages are reclaimed one memcg at a time.

And why is this preferred over SWAP_CLUSTER_MAX and whole subtree
reclaim that we do currently? 

Please do not set another version until it is actually clear what you
want to achieve and why.
-- 
Michal Hocko
SUSE Labs

Re: [RFC v1] memcg: add memcg lru for page reclaiming

[Hillf Danton]

On Tue, 22 Oct 2019 15:58:32 +0200 Michal Hocko wrote:
> 
> On Tue 22-10-19 21:30:50, Hillf Danton wrote:
> > 
> > On Mon, 21 Oct 2019 14:14:53 +0200 Michal Hocko wrote:
> > > 
> > > On Mon 21-10-19 19:56:54, Hillf Danton wrote:
> > > > 
> > > > Currently soft limit reclaim is frozen, see
> > > > Documentation/admin-guide/cgroup-v2.rst for reasons.
> > > > 
> > > > Copying the page lru idea, memcg lru is added for selecting victim
> > > > memcg to reclaim pages from under memory pressure. It now works in
> > > > parallel to slr not only because the latter needs some time to reap
> > > > but the coexistence facilitates it a lot to add the lru in a straight
> > > > forward manner.
> > > 
> > > This doesn't explain what is the problem/feature you would like to
> > > fix/achieve. It also doesn't explain the overall design. 
> > 
> > 1, memcg lru makes page reclaiming hierarchy aware
> 
> Is that a problem statement or a design goal?

A problem in soft limit reclaim as per cgroup-v2.rst that is addressed
in the RFC.

> > While doing the high work, memcgs are currently reclaimed one after
> > another up through the hierarchy;
> 
> Which is the design because it is the the memcg where the high limit got
> hit. The hierarchical behavior ensures that the subtree of that memcg is
> reclaimed and we try to spread the reclaim fairly over the tree.

Yeah, that coding is scarcely able to escape standing ovation. No one of
its merits yet is missed in the RFC except for breaking spiraling up the
memcg hierarchy into two parts, the up half that rips pages off the first
victim, and the bottom half that queues the victim's first ancestor on the
lru(the ice box storing the cakes baked for kswapd), see below for reasons.

> > in this RFC after ripping pages off
> > the first victim, the work finishes with the first ancestor of the victim
> > added to lru.
> > 
> > Recaliming is defered until kswapd becomes active.
> 
> This is a wrong assumption because high limit might be configured way
> before kswapd is woken up.

This change was introduced because high limit breach looks not like a
serious problem in the absence of memory pressure. Lets do the hard work,
reclaiming one memcg a time up through the hierarchy, when kswapd becomes
active. It also explains the BH introduced.

> > 2, memcg lru tries much to avoid overreclaim
> 
> Again, is this a problem statement or a design goal?

Another problem in SLR as per cgroup-v2.rst that is addressed in the RFC.

> > Only one memcg is picked off lru in FIFO mode under memory pressure,
> > and MEMCG_CHARGE_BATCH pages are reclaimed one memcg at a time.
> 
> And why is this preferred over SWAP_CLUSTER_MAX

No change is added in the current high work behavior in terms of
MEMCG_CHARGE_BATCH; try_to_free_mem_cgroup_pages() takes care of both.

> and whole subtree reclaim that we do currently? 

We terminate climbing up the hierarchy once kswapd finger snaps "Cut. Work done."

Thanks
Hillf

Re: [RFC v1] memcg: add memcg lru for page reclaiming

[Michal Hocko]

On Wed 23-10-19 12:44:48, Hillf Danton wrote:
> 
> On Tue, 22 Oct 2019 15:58:32 +0200 Michal Hocko wrote:
> > 
> > On Tue 22-10-19 21:30:50, Hillf Danton wrote:
[...]
> > > in this RFC after ripping pages off
> > > the first victim, the work finishes with the first ancestor of the victim
> > > added to lru.
> > > 
> > > Recaliming is defered until kswapd becomes active.
> > 
> > This is a wrong assumption because high limit might be configured way
> > before kswapd is woken up.
> 
> This change was introduced because high limit breach looks not like a
> serious problem in the absence of memory pressure. Lets do the hard work,
> reclaiming one memcg a time up through the hierarchy, when kswapd becomes
> active. It also explains the BH introduced.

But this goes against the main motivation for the high limit - to
throttle. It is not all that important that there is not global memory
pressure. The preventive high limit reclaim is there to make sure that
the specific memcg is kept in a reasonable containment.
-- 
Michal Hocko
SUSE Labs