Skip to contentSkip to conversation

mm: allow to decrease swap.max below actual swap usage

PATCH In review Roman Gushchin 3 participants 4 messages Apr 12, 2018 – Apr 17, 2018

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mm: allow to decrease swap.max below actual swap usage

Currently an attempt to set swap.max into a value lower than the actual swap usage fails. And a user can't do much with it, except turning off swap globally (using swapoff).

This is an actual problem we met in the production. The default value for swap.max is "max", so turning swap on globally may lead to swapping out in some cgroups. As it's not possible to lower the allowed maximum, it creates additional configuration issues.

For example, to disable swapping for a particular cgroup it has to be re-created with memory.max set to 0.

This patch aims to fix this issue by allowing setting swap.max into any value (which corresponds to cgroup v2 API design), and schedule a background job to fit swap size into the new limit.

The following script can be used to test the memory.swap behavior:
#!/bin/bash

mkdir -p /sys/fs/cgroup/test_swap
echo 100M > /sys/fs/cgroup/test_swap/memory.max echo max > /sys/fs/cgroup/test_swap/memory.swap.max

mkdir -p /sys/fs/cgroup/test_swap_2
echo 100M > /sys/fs/cgroup/test_swap_2/memory.max echo max > /sys/fs/cgroup/test_swap_2/memory.swap.max

echo $$ > /sys/fs/cgroup/test_swap/cgroup.procs
allocate 200M &

echo $$ > /sys/fs/cgroup/test_swap_2/cgroup.procs allocate 200M &

sleep 2

cat /sys/fs/cgroup/test_swap/memory.swap.current cat /sys/fs/cgroup/test_swap_2/memory.swap.current

echo max > /sys/fs/cgroup/test_swap/memory.max echo 50M > /sys/fs/cgroup/test_swap/memory.swap.max

sleep 10

cat /sys/fs/cgroup/test_swap/memory.swap.current cat /sys/fs/cgroup/test_swap_2/memory.swap.current

pkill allocate

Original test results:
106024960
106348544
./swap.sh: line 23: echo: write error: Device or resource busy 106024960
106348544

With this patch applied:
106045440
106352640
52428800
106201088

---
 include/linux/memcontrol.h |  1 +
 include/linux/swap.h       |  9 +++++++
 include/linux/swapfile.h   |  3 ++-
 mm/frontswap.c             |  2 +-
 mm/memcontrol.c            | 27 +++++++++++++++----
 mm/swapfile.c              | 64 ++++++++++++++++++++++++++++++++++++++++++----
 6 files changed, 94 insertions(+), 12 deletions(-)
include/linux/memcontrol.h +1 -0
index af9eed2e3e04..a0825cc61ee7 100644
@@ -186,6 +186,7 @@ struct mem_cgroup {
 
 	/* Range enforcement for interrupt charges */
 	struct work_struct high_work;
+	struct work_struct swap_work;
 
 	unsigned long soft_limit;
 
include/linux/swap.h +9 -0
index 1985940af479..878f111d0603 100644
@@ -650,6 +650,8 @@ extern int mem_cgroup_try_charge_swap(struct page *page, swp_entry_t entry);
 extern void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages);
 extern long mem_cgroup_get_nr_swap_pages(struct mem_cgroup *memcg);
 extern bool mem_cgroup_swap_full(struct page *page);
+extern int mem_cgroup_shrink_swap(struct mem_cgroup *memcg,
+				  unsigned long nr_pages);
 #else
 static inline void mem_cgroup_swapout(struct page *page, swp_entry_t entry)
 {
@@ -675,6 +677,13 @@ static inline bool mem_cgroup_swap_full(struct page *page)
 {
 	return vm_swap_full();
 }
+
+static inline int mem_cgroup_shrink_swap(struct mem_cgroup *memcg,
+					 unsigned long nr_pages)
+{
+	return 0;
+}
+
 #endif
 
 #endif /* __KERNEL__*/
include/linux/swapfile.h +2 -1
index 06bd7b096167..16844259e802 100644
@@ -9,6 +9,7 @@
 extern spinlock_t swap_lock;
 extern struct plist_head swap_active_head;
 extern struct swap_info_struct *swap_info[];
-extern int try_to_unuse(unsigned int, bool, unsigned long);
+extern int try_to_unuse(unsigned int type, bool fronstswap,
+			unsigned long pages_to_unuse, struct mem_cgroup *memcg);
 
 #endif /* _LINUX_SWAPFILE_H */
mm/frontswap.c +1 -1
index fec8b5044040..f7cb2e802fce 100644
@@ -458,7 +458,7 @@ void frontswap_shrink(unsigned long target_pages)
 	ret = __frontswap_shrink(target_pages, &pages_to_unuse, &type);
 	spin_unlock(&swap_lock);
 	if (ret == 0)
-		try_to_unuse(type, true, pages_to_unuse);
+		try_to_unuse(type, true, pages_to_unuse, NULL);
 	return;
 }
 EXPORT_SYMBOL(frontswap_shrink);
mm/memcontrol.c +22 -5
index 3e7942c301a8..bacb1fd17100 100644
@@ -60,6 +60,7 @@
 #include <linux/vmpressure.h>
 #include <linux/mm_inline.h>
 #include <linux/swap_cgroup.h>
+#include <linux/swapfile.h>
 #include <linux/cpu.h>
 #include <linux/oom.h>
 #include <linux/lockdep.h>
@@ -1870,6 +1871,23 @@ static void high_work_func(struct work_struct *work)
 	reclaim_high(memcg, MEMCG_CHARGE_BATCH, GFP_KERNEL);
 }
 
+static void swap_work_func(struct work_struct *work)
+{
+	struct mem_cgroup *memcg;
+
+	memcg = container_of(work, struct mem_cgroup, swap_work);
+
+	for (;;) {
+		unsigned long usage = page_counter_read(&memcg->swap);
+
+		if (usage <= memcg->swap.limit)
+			break;
+
+		if (mem_cgroup_shrink_swap(memcg, usage - memcg->swap.limit))
+			break;
+	}
+}
+
 /*
  * Scheduled by try_charge() to be executed from the userland return path
  * and reclaims memory over the high limit.
@@ -4394,6 +4412,7 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
 		goto fail;
 
 	INIT_WORK(&memcg->high_work, high_work_func);
+	INIT_WORK(&memcg->swap_work, swap_work_func);
 	memcg->last_scanned_node = MAX_NUMNODES;
 	INIT_LIST_HEAD(&memcg->oom_notify);
 	mutex_init(&memcg->thresholds_lock);
@@ -4529,6 +4548,7 @@ static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
 
 	vmpressure_cleanup(&memcg->vmpressure);
 	cancel_work_sync(&memcg->high_work);
+	cancel_work_sync(&memcg->swap_work);
 	mem_cgroup_remove_from_trees(memcg);
 	memcg_free_kmem(memcg);
 	mem_cgroup_free(memcg);
@@ -6415,11 +6435,8 @@ static ssize_t swap_max_write(struct kernfs_open_file *of,
 	if (err)
 		return err;
 
-	mutex_lock(&memcg_limit_mutex);
-	err = page_counter_limit(&memcg->swap, max);
-	mutex_unlock(&memcg_limit_mutex);
-	if (err)
-		return err;
+	xchg(&memcg->swap.limit, max);
+	schedule_work(&memcg->swap_work);
 
 	return nbytes;
 }
mm/swapfile.c +59 -5

96 lines not on this page · raw message

Signature · 1 line
2.14.3
maintainer
#1 lore raw

Hello, Roman.

The reclaim behavior is a bit worrisome.

* It disables an entire swap area while reclaim is in progress. Most systems only have one swap area, so this would disable allocating new swap area for everyone.

* The reclaim seems very inefficient. IIUC, it has to read every swap page to see whether the page belongs to the target memcg and for each matching page, which involves walking page mm's and page tables.

An easy optimization would be walking swap_cgroup_ctrl so that it only reads swap entries which belong to the target cgroup and avoid disabling swap for others, but looking at the code, I wonder whether we need active reclaim at all.

Swap already tries to aggressively reclaim swap entries when swap usage > 50% of the limit, so simply reducing the limit already triggers aggressive reclaim, and given that it's swap, just waiting it out could be the better behavior anyway, so how about something like the following?

------ 8< ------
From: Tejun Heo <[email protected]>
Subject: mm: memcg: allow lowering memory.swap.max below the current usage

Currently an attempt to set swap.max into a value lower than the actual swap usage fails, which causes configuration problems as there's no way of lowering the configuration below the current usage short of turning off swap entirely. This makes swap.max difficult to use and allows delegatees to lock the delegator out of reducing swap allocation.

This patch updates swap_max_write() so that the limit can be lowered below the current usage. It doesn't implement active reclaiming of swap entries for the following reasons.

* mem_cgroup_swap_full() already tells the swap machinary to aggressively reclaim swap entries if the usage is above 50% of limit, so simply lowering the limit automatically triggers gradual reclaim.

* Forcing back swapped out pages is likely to heavily impact the workload and mess up the working set. Given that swap usually is a lot less valuable and less scarce, letting the existing usage dissipate over time through the above gradual reclaim and as they're falted back in is likely the better behavior.

Cc Roman Gushchin <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Shaohua Li <[email protected]>
Cc Rik van Riel <[email protected]>
---
 Documentation/cgroup-v2.txt |    5 +++++
 mm/memcontrol.c             |    6 +-----
 2 files changed, 6 insertions(+), 5 deletions(-)
Documentation/cgroup-v2.txt +5 -0
@@ -1199,6 +1199,11 @@ PAGE_SIZE multiple when read back.
 	Swap usage hard limit.  If a cgroup's swap usage reaches this
 	limit, anonymous memory of the cgroup will not be swapped out.
 
+	When reduced under the current usage, the existing swap
+	entries are reclaimed gradually and the swap usage may stay
+	higher than the limit for an extended period of time.  This
+	reduces the impact on the workload and memory management.
+
 
 Usage Guidelines
 ~~~~~~~~~~~~~~~~
mm/memcontrol.c +1 -5
@@ -6144,11 +6144,7 @@ static ssize_t swap_max_write(struct ker
 	if (err)
 		return err;
 
-	mutex_lock(&memcg_limit_mutex);
-	err = page_counter_limit(&memcg->swap, max);
-	mutex_unlock(&memcg_limit_mutex);
-	if (err)
-		return err;
+	xchg(&memcg->swap.limit, max);
 
 	return nbytes;
 }
Acked-by
#2 lore raw
Quoted 7 lines · Tejun Heo

Hello, Roman.

The reclaim behavior is a bit worrisome.

* It disables an entire swap area while reclaim is in progress. Most systems only have one swap area, so this would disable allocating new swap area for everyone.

That could easily cause OOM kills on systems.

I prefer Tejun's simple approach, of having the system slowly reduce swap use below the new maximum limit.

Quoted 5 lines

* The reclaim seems very inefficient. IIUC, it has to read every swap
page to see whether the page belongs to the target memcg and for each matching page, which involves walking page mm's and page tables.

One of my Outreachy interns, Kelley Nielsen, worked on making swap reclaim more efficient by scanning the virtual address space of processes.

Unfortunately, we ran into some unforseen issues with that approach that we never managed to sort out :(

Quoted 1 line
Signature · 1 line
All Rights Reversed.
author Acked-by
#3 lore raw
In reply to Tejun Heo

Hi, Tejun!

Quoted 35 lines · Tejun Heo

Hello, Roman.

The reclaim behavior is a bit worrisome.

* It disables an entire swap area while reclaim is in progress. Most systems only have one swap area, so this would disable allocating new swap area for everyone.

* The reclaim seems very inefficient. IIUC, it has to read every swap page to see whether the page belongs to the target memcg and for each matching page, which involves walking page mm's and page tables.

An easy optimization would be walking swap_cgroup_ctrl so that it only reads swap entries which belong to the target cgroup and avoid disabling swap for others, but looking at the code, I wonder whether we need active reclaim at all.

Swap already tries to aggressively reclaim swap entries when swap usage > 50% of the limit, so simply reducing the limit already triggers aggressive reclaim, and given that it's swap, just waiting it out could be the better behavior anyway, so how about something like the following?

------ 8< ------
From: Tejun Heo <[email protected]>
Subject: mm: memcg: allow lowering memory.swap.max below the current usage

Currently an attempt to set swap.max into a value lower than the actual swap usage fails, which causes configuration problems as there's no way of lowering the configuration below the current usage short of turning off swap entirely. This makes swap.max difficult to use and allows delegatees to lock the delegator out of reducing swap allocation.

Quoted 3 lines

This patch updates swap_max_write() so that the limit can be lowered below the current usage. It doesn't implement active reclaiming of swap entries for the following reasons.

This is definitely better than existing state of things, and it's also safe.

I assume, that active swap reclaim can be useful in some cases, but we can return to this question later.

Quoted 29 lines

* mem_cgroup_swap_full() already tells the swap machinary to aggressively reclaim swap entries if the usage is above 50% of limit, so simply lowering the limit automatically triggers gradual reclaim.

* Forcing back swapped out pages is likely to heavily impact the workload and mess up the working set. Given that swap usually is a lot less valuable and less scarce, letting the existing usage dissipate over time through the above gradual reclaim and as they're falted back in is likely the better behavior.

Cc Roman Gushchin <[email protected]>
Cc Johannes Weiner <[email protected]>
Cc Michal Hocko <[email protected]>
Cc Shaohua Li <[email protected]>
Cc Rik van Riel <[email protected]>
---
 Documentation/cgroup-v2.txt |    5 +++++
 mm/memcontrol.c             |    6 +-----
 2 files changed, 6 insertions(+), 5 deletions(-)

Documentation/cgroup-v2.txt PAGE_SIZE multiple when read back. · line 1205

Quoted 7 lines
Documentation/cgroup-v2.txt +4 -0
@@ -1199,6 +1199,11 @@ PAGE_SIZE multiple when read back.
 	Swap usage hard limit.  If a cgroup's swap usage reaches this
 	limit, anonymous memory of the cgroup will not be swapped out.
 
+	When reduced under the current usage, the existing swap
+	entries are reclaimed gradually and the swap usage may stay
+	higher than the limit for an extended period of time.  This
+	reduces the impact on the workload and memory management.

I would probably drop the last sentence: it looks like an excuse for the defined semantics; but it's totally fine.

Thanks!