2020-04-18 13:47:27

by Alex Shi

[permalink] [raw]
Subject: [PATCH v2 0/2] always record memcg id for swapped memcg pages

Hi Johannes,

2 patches fold MEMCG_SWAP feature into kernel as default. I don't know if
they could fit your new solution.

But just in case if they can save you some typing. :)

Thanks
Alex
Cc: Andrew Morton <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Vladimir Davydov <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
Alex Shi (2):
memcg: folding CONFIG_MEMCG_SWAP as default
mm/memcg: remove swapaccount kernel parameter

Documentation/admin-guide/kernel-parameters.txt | 5 --
arch/arm/configs/omap2plus_defconfig | 1 -
arch/arm64/configs/defconfig | 1 -
arch/mips/configs/db1xxx_defconfig | 1 -
arch/mips/configs/generic_defconfig | 1 -
arch/mips/configs/loongson3_defconfig | 1 -
arch/parisc/configs/generic-64bit_defconfig | 1 -
arch/powerpc/configs/powernv_defconfig | 1 -
arch/powerpc/configs/pseries_defconfig | 1 -
arch/s390/configs/debug_defconfig | 1 -
arch/s390/configs/defconfig | 1 -
arch/sh/configs/sdk7786_defconfig | 1 -
arch/sh/configs/urquell_defconfig | 1 -
include/linux/memcontrol.h | 4 --
include/linux/swap.h | 27 ----------
include/linux/swap_cgroup.h | 30 -----------
init/Kconfig | 20 --------
mm/Makefile | 4 +-
mm/memcontrol.c | 68 ++++++-------------------
mm/swap_cgroup.c | 7 ---
20 files changed, 17 insertions(+), 160 deletions(-)

--
1.8.3.1


2020-04-18 13:47:56

by Alex Shi

[permalink] [raw]
Subject: [PATCH v2 2/2] mm/memcg: remove swapaccount kernel parameter

After merge the CONFIG_MEMCG_SWAP into kernel as default feature. This
patch removes the kernel parameter interface swapaccount= to keep this
feature from user disable in boot.

Now, all swapped memcg page record it's id permanently.

Signed-off-by: Alex Shi <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Vladimir Davydov <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
Documentation/admin-guide/kernel-parameters.txt | 5 ---
include/linux/memcontrol.h | 2 -
mm/memcontrol.c | 50 ++++++++-----------------
mm/swap_cgroup.c | 7 ----
4 files changed, 15 insertions(+), 49 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f2a93c8679e8..f8532dfc4f17 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4877,11 +4877,6 @@
This parameter controls use of the Protected
Execution Facility on pSeries.

- swapaccount=[0|1]
- [KNL] Enable accounting of swap in memory resource
- controller if no parameter or 1 is given or disable
- it if 0 is given (See Documentation/admin-guide/cgroup-v1/memory.rst)
-
swiotlb= [ARM,IA-64,PPC,MIPS,X86]
Format: { <int> | force | noforce }
<int> -- Number of I/O TLB slabs
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 45842ed8ba53..9d6330ef5a13 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -569,8 +569,6 @@ struct mem_cgroup *mem_cgroup_get_oom_group(struct task_struct *victim,
struct mem_cgroup *oom_domain);
void mem_cgroup_print_oom_group(struct mem_cgroup *memcg);

-extern int do_swap_account;
-
struct mem_cgroup *lock_page_memcg(struct page *page);
void __unlock_page_memcg(struct mem_cgroup *memcg);
void unlock_page_memcg(struct page *page);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 446141b6597a..ba3ac7c7644b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -81,9 +81,6 @@
/* Kernel memory accounting disabled? */
static bool cgroup_memory_nokmem;

-/* Whether the swap controller is active */
-int do_swap_account __read_mostly;
-
#ifdef CONFIG_CGROUP_WRITEBACK
static DECLARE_WAIT_QUEUE_HEAD(memcg_cgwb_frn_waitq);
#endif
@@ -91,7 +88,7 @@
/* Whether legacy memory+swap accounting is active */
static bool do_memsw_account(void)
{
- return !cgroup_subsys_on_dfl(memory_cgrp_subsys) && do_swap_account;
+ return !cgroup_subsys_on_dfl(memory_cgrp_subsys);
}

#define THRESHOLDS_EVENTS_TARGET 128
@@ -6444,6 +6441,9 @@ int mem_cgroup_try_charge(struct page *page, struct mm_struct *mm,
goto out;

if (PageSwapCache(page)) {
+ swp_entry_t ent;
+ unsigned short id;
+
/*
* Every swap fault against a single page tries to charge the
* page, bail as early as possible. shmem_unuse() encounters
@@ -6455,16 +6455,13 @@ int mem_cgroup_try_charge(struct page *page, struct mm_struct *mm,
if (compound_head(page)->mem_cgroup)
goto out;

- if (do_swap_account) {
- swp_entry_t ent = { .val = page_private(page), };
- unsigned short id = lookup_swap_cgroup_id(ent);
-
- rcu_read_lock();
- memcg = mem_cgroup_from_id(id);
- if (memcg && !css_tryget_online(&memcg->css))
- memcg = NULL;
- rcu_read_unlock();
- }
+ ent.val = page_private(page);
+ id = lookup_swap_cgroup_id(ent);
+ rcu_read_lock();
+ memcg = mem_cgroup_from_id(id);
+ if (memcg && !css_tryget_online(&memcg->css))
+ memcg = NULL;
+ rcu_read_unlock();
}

if (!memcg)
@@ -7024,7 +7021,7 @@ int mem_cgroup_try_charge_swap(struct page *page, swp_entry_t entry)
struct mem_cgroup *memcg;
unsigned short oldid;

- if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) || !do_swap_account)
+ if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
return 0;

memcg = page->mem_cgroup;
@@ -7068,9 +7065,6 @@ void mem_cgroup_uncharge_swap(swp_entry_t entry, unsigned int nr_pages)
struct mem_cgroup *memcg;
unsigned short id;

- if (!do_swap_account)
- return;
-
id = swap_cgroup_record(entry, 0, nr_pages);
rcu_read_lock();
memcg = mem_cgroup_from_id(id);
@@ -7091,7 +7085,7 @@ long mem_cgroup_get_nr_swap_pages(struct mem_cgroup *memcg)
{
long nr_swap_pages = get_nr_swap_pages();

- if (!do_swap_account || !cgroup_subsys_on_dfl(memory_cgrp_subsys))
+ if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
return nr_swap_pages;
for (; memcg != root_mem_cgroup; memcg = parent_mem_cgroup(memcg))
nr_swap_pages = min_t(long, nr_swap_pages,
@@ -7108,7 +7102,7 @@ bool mem_cgroup_swap_full(struct page *page)

if (vm_swap_full())
return true;
- if (!do_swap_account || !cgroup_subsys_on_dfl(memory_cgrp_subsys))
+ if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
return false;

memcg = page->mem_cgroup;
@@ -7123,19 +7117,6 @@ bool mem_cgroup_swap_full(struct page *page)
return false;
}

-/* for remember boot option*/
-static int really_do_swap_account __initdata = 1;
-
-static int __init enable_swap_account(char *s)
-{
- if (!strcmp(s, "1"))
- really_do_swap_account = 1;
- else if (!strcmp(s, "0"))
- really_do_swap_account = 0;
- return 1;
-}
-__setup("swapaccount=", enable_swap_account);
-
static u64 swap_current_read(struct cgroup_subsys_state *css,
struct cftype *cft)
{
@@ -7229,8 +7210,7 @@ static int swap_events_show(struct seq_file *m, void *v)

static int __init mem_cgroup_swap_init(void)
{
- if (!mem_cgroup_disabled() && really_do_swap_account) {
- do_swap_account = 1;
+ if (!mem_cgroup_disabled()) {
WARN_ON(cgroup_add_dfl_cftypes(&memory_cgrp_subsys,
swap_files));
WARN_ON(cgroup_add_legacy_cftypes(&memory_cgrp_subsys,
diff --git a/mm/swap_cgroup.c b/mm/swap_cgroup.c
index 45affaef3bc6..3cb015179d29 100644
--- a/mm/swap_cgroup.c
+++ b/mm/swap_cgroup.c
@@ -171,9 +171,6 @@ int swap_cgroup_swapon(int type, unsigned long max_pages)
unsigned long length;
struct swap_cgroup_ctrl *ctrl;

- if (!do_swap_account)
- return 0;
-
length = DIV_ROUND_UP(max_pages, SC_PER_PAGE);
array_size = length * sizeof(void *);

@@ -199,7 +196,6 @@ int swap_cgroup_swapon(int type, unsigned long max_pages)
return 0;
nomem:
pr_info("couldn't allocate enough memory for swap_cgroup\n");
- pr_info("swap_cgroup can be disabled by swapaccount=0 boot option\n");
return -ENOMEM;
}

@@ -209,9 +205,6 @@ void swap_cgroup_swapoff(int type)
unsigned long i, length;
struct swap_cgroup_ctrl *ctrl;

- if (!do_swap_account)
- return;
-
mutex_lock(&swap_cgroup_mutex);
ctrl = &swap_cgroup_ctrl[type];
map = ctrl->map;
--
1.8.3.1

2020-04-18 13:48:53

by Alex Shi

[permalink] [raw]
Subject: [PATCH v2 1/2] memcg: folding CONFIG_MEMCG_SWAP as default

This patch fold MEMCG_SWAP feature into kernel as default function. With
the next swapaccount removing patch, the swapout pages for each of memcg
always be recorded into swap_cgroup.id. the cost as Johannes mentioned

"the overhead of tracking is tiny - 512k per G of swap (0.04%).'

With this record info, swapin don't have to add all readahead page
into lru list before do memcg charge.

Signed-off-by: Alex Shi <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Vladimir Davydov <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
arch/arm/configs/omap2plus_defconfig | 1 -
arch/arm64/configs/defconfig | 1 -
arch/mips/configs/db1xxx_defconfig | 1 -
arch/mips/configs/generic_defconfig | 1 -
arch/mips/configs/loongson3_defconfig | 1 -
arch/parisc/configs/generic-64bit_defconfig | 1 -
arch/powerpc/configs/powernv_defconfig | 1 -
arch/powerpc/configs/pseries_defconfig | 1 -
arch/s390/configs/debug_defconfig | 1 -
arch/s390/configs/defconfig | 1 -
arch/sh/configs/sdk7786_defconfig | 1 -
arch/sh/configs/urquell_defconfig | 1 -
include/linux/memcontrol.h | 2 --
include/linux/swap.h | 27 --------------------------
include/linux/swap_cgroup.h | 30 -----------------------------
init/Kconfig | 20 -------------------
mm/Makefile | 4 ++--
mm/memcontrol.c | 18 -----------------
18 files changed, 2 insertions(+), 111 deletions(-)

diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index 3cc3ca5fa027..929cfc4c062a 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -10,7 +10,6 @@ CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=16
CONFIG_CGROUPS=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_BLK_CGROUP=y
CONFIG_CGROUP_SCHED=y
CONFIG_CFS_BANDWIDTH=y
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 24e534d85045..3854c6140a04 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -13,7 +13,6 @@ CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_NUMA_BALANCING=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_BLK_CGROUP=y
CONFIG_CGROUP_PIDS=y
CONFIG_CGROUP_HUGETLB=y
diff --git a/arch/mips/configs/db1xxx_defconfig b/arch/mips/configs/db1xxx_defconfig
index e6f3e8e3da39..4604e826d03f 100644
--- a/arch/mips/configs/db1xxx_defconfig
+++ b/arch/mips/configs/db1xxx_defconfig
@@ -9,7 +9,6 @@ CONFIG_HIGH_RES_TIMERS=y
CONFIG_LOG_BUF_SHIFT=16
CONFIG_CGROUPS=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_BLK_CGROUP=y
CONFIG_CGROUP_SCHED=y
CONFIG_CFS_BANDWIDTH=y
diff --git a/arch/mips/configs/generic_defconfig b/arch/mips/configs/generic_defconfig
index 714169e411cf..48e4e251779b 100644
--- a/arch/mips/configs/generic_defconfig
+++ b/arch/mips/configs/generic_defconfig
@@ -3,7 +3,6 @@ CONFIG_NO_HZ_IDLE=y
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_BLK_CGROUP=y
CONFIG_CFS_BANDWIDTH=y
CONFIG_RT_GROUP_SCHED=y
diff --git a/arch/mips/configs/loongson3_defconfig b/arch/mips/configs/loongson3_defconfig
index 51675f5000d6..f91f49595100 100644
--- a/arch/mips/configs/loongson3_defconfig
+++ b/arch/mips/configs/loongson3_defconfig
@@ -13,7 +13,6 @@ CONFIG_TASK_DELAY_ACCT=y
CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_BLK_CGROUP=y
CONFIG_CPUSETS=y
CONFIG_SCHED_AUTOGROUP=y
diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig
index 59561e04e659..fab4401dd9f6 100644
--- a/arch/parisc/configs/generic-64bit_defconfig
+++ b/arch/parisc/configs/generic-64bit_defconfig
@@ -10,7 +10,6 @@ CONFIG_TASK_XACCT=y
CONFIG_TASK_IO_ACCOUNTING=y
CONFIG_CGROUPS=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_CGROUP_PIDS=y
CONFIG_CPUSETS=y
CONFIG_RELAY=y
diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
index df8bdbaa5d8f..c60c3a4125f7 100644
--- a/arch/powerpc/configs/powernv_defconfig
+++ b/arch/powerpc/configs/powernv_defconfig
@@ -17,7 +17,6 @@ CONFIG_LOG_CPU_MAX_BUF_SHIFT=13
CONFIG_NUMA_BALANCING=y
CONFIG_CGROUPS=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_CGROUP_SCHED=y
CONFIG_CGROUP_FREEZER=y
CONFIG_CPUSETS=y
diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
index 0bea4d3ffb85..426244f491ec 100644
--- a/arch/powerpc/configs/pseries_defconfig
+++ b/arch/powerpc/configs/pseries_defconfig
@@ -16,7 +16,6 @@ CONFIG_LOG_CPU_MAX_BUF_SHIFT=13
CONFIG_NUMA_BALANCING=y
CONFIG_CGROUPS=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_CGROUP_SCHED=y
CONFIG_CGROUP_FREEZER=y
CONFIG_CPUSETS=y
diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
index 46038bc58c9e..864b32403673 100644
--- a/arch/s390/configs/debug_defconfig
+++ b/arch/s390/configs/debug_defconfig
@@ -14,7 +14,6 @@ CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_NUMA_BALANCING=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_BLK_CGROUP=y
CONFIG_CFS_BANDWIDTH=y
CONFIG_RT_GROUP_SCHED=y
diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
index 7cd0648c1f4e..991e60d7c307 100644
--- a/arch/s390/configs/defconfig
+++ b/arch/s390/configs/defconfig
@@ -13,7 +13,6 @@ CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_NUMA_BALANCING=y
CONFIG_MEMCG=y
-CONFIG_MEMCG_SWAP=y
CONFIG_BLK_CGROUP=y
CONFIG_CFS_BANDWIDTH=y
CONFIG_RT_GROUP_SCHED=y
diff --git a/arch/sh/configs/sdk7786_defconfig b/arch/sh/configs/sdk7786_defconfig
index 7fa116b436c3..4a1d19068a8d 100644
--- a/arch/sh/configs/sdk7786_defconfig
+++ b/arch/sh/configs/sdk7786_defconfig
@@ -17,7 +17,6 @@ CONFIG_CPUSETS=y
# CONFIG_PROC_PID_CPUSET is not set
CONFIG_CGROUP_CPUACCT=y
CONFIG_CGROUP_MEMCG=y
-CONFIG_CGROUP_MEMCG_SWAP=y
CONFIG_CGROUP_SCHED=y
CONFIG_RT_GROUP_SCHED=y
CONFIG_BLK_CGROUP=y
diff --git a/arch/sh/configs/urquell_defconfig b/arch/sh/configs/urquell_defconfig
index cb2f56468fe0..be478f3148f2 100644
--- a/arch/sh/configs/urquell_defconfig
+++ b/arch/sh/configs/urquell_defconfig
@@ -14,7 +14,6 @@ CONFIG_CPUSETS=y
# CONFIG_PROC_PID_CPUSET is not set
CONFIG_CGROUP_CPUACCT=y
CONFIG_CGROUP_MEMCG=y
-CONFIG_CGROUP_MEMCG_SWAP=y
CONFIG_CGROUP_SCHED=y
CONFIG_RT_GROUP_SCHED=y
CONFIG_BLK_DEV_INITRD=y
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 1b4150ff64be..45842ed8ba53 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -569,9 +569,7 @@ struct mem_cgroup *mem_cgroup_get_oom_group(struct task_struct *victim,
struct mem_cgroup *oom_domain);
void mem_cgroup_print_oom_group(struct mem_cgroup *memcg);

-#ifdef CONFIG_MEMCG_SWAP
extern int do_swap_account;
-#endif

struct mem_cgroup *lock_page_memcg(struct page *page);
void __unlock_page_memcg(struct mem_cgroup *memcg);
diff --git a/include/linux/swap.h b/include/linux/swap.h
index eae48a3896e8..5627fb8ca827 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -652,38 +652,11 @@ static inline void mem_cgroup_throttle_swaprate(struct mem_cgroup *memcg,
}
#endif

-#ifdef CONFIG_MEMCG_SWAP
extern void mem_cgroup_swapout(struct page *page, swp_entry_t entry);
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);
-#else
-static inline void mem_cgroup_swapout(struct page *page, swp_entry_t entry)
-{
-}
-
-static inline int mem_cgroup_try_charge_swap(struct page *page,
- swp_entry_t entry)
-{
- return 0;
-}
-
-static inline void mem_cgroup_uncharge_swap(swp_entry_t entry,
- unsigned int nr_pages)
-{
-}
-
-static inline long mem_cgroup_get_nr_swap_pages(struct mem_cgroup *memcg)
-{
- return get_nr_swap_pages();
-}
-
-static inline bool mem_cgroup_swap_full(struct page *page)
-{
- return vm_swap_full();
-}
-#endif

#endif /* __KERNEL__*/
#endif /* _LINUX_SWAP_H */
diff --git a/include/linux/swap_cgroup.h b/include/linux/swap_cgroup.h
index a12dd1c3966c..5ac87b3b0e08 100644
--- a/include/linux/swap_cgroup.h
+++ b/include/linux/swap_cgroup.h
@@ -4,8 +4,6 @@

#include <linux/swap.h>

-#ifdef CONFIG_MEMCG_SWAP
-
extern unsigned short swap_cgroup_cmpxchg(swp_entry_t ent,
unsigned short old, unsigned short new);
extern unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id,
@@ -14,32 +12,4 @@ extern unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id,
extern int swap_cgroup_swapon(int type, unsigned long max_pages);
extern void swap_cgroup_swapoff(int type);

-#else
-
-static inline
-unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id,
- unsigned int nr_ents)
-{
- return 0;
-}
-
-static inline
-unsigned short lookup_swap_cgroup_id(swp_entry_t ent)
-{
- return 0;
-}
-
-static inline int
-swap_cgroup_swapon(int type, unsigned long max_pages)
-{
- return 0;
-}
-
-static inline void swap_cgroup_swapoff(int type)
-{
- return;
-}
-
-#endif /* CONFIG_MEMCG_SWAP */
-
#endif /* __LINUX_SWAP_CGROUP_H */
diff --git a/init/Kconfig b/init/Kconfig
index 9e22ee8fbd75..3b09c71e9b57 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -834,26 +834,6 @@ config MEMCG
help
Provides control over the memory footprint of tasks in a cgroup.

-config MEMCG_SWAP
- bool "Swap controller"
- depends on MEMCG && SWAP
- help
- Provides control over the swap space consumed by tasks in a cgroup.
-
-config MEMCG_SWAP_ENABLED
- bool "Swap controller enabled by default"
- depends on MEMCG_SWAP
- default y
- help
- Memory Resource Controller Swap Extension comes with its price in
- a bigger memory consumption. General purpose distribution kernels
- which want to enable the feature but keep it disabled by default
- and let the user enable it by swapaccount=1 boot command line
- parameter should have this option unselected.
- For those who want to have the feature enabled by default should
- select this option (if, for some reason, they need to disable it
- then swapaccount=0 does the trick).
-
config MEMCG_KMEM
bool
depends on MEMCG && !SLOB
diff --git a/mm/Makefile b/mm/Makefile
index fccd3756b25f..812c08323355 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -58,7 +58,8 @@ ifdef CONFIG_MMU
obj-$(CONFIG_ADVISE_SYSCALLS) += madvise.o
endif

-obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o swap_slots.o
+obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o swap_slots.o \
+ swap_cgroup.o
obj-$(CONFIG_FRONTSWAP) += frontswap.o
obj-$(CONFIG_ZSWAP) += zswap.o
obj-$(CONFIG_HAS_DMA) += dmapool.o
@@ -80,7 +81,6 @@ obj-$(CONFIG_MIGRATION) += migrate.o
obj-$(CONFIG_TRANSPARENT_HUGEPAGE) += huge_memory.o khugepaged.o
obj-$(CONFIG_PAGE_COUNTER) += page_counter.o
obj-$(CONFIG_MEMCG) += memcontrol.o vmpressure.o
-obj-$(CONFIG_MEMCG_SWAP) += swap_cgroup.o
obj-$(CONFIG_CGROUP_HUGETLB) += hugetlb_cgroup.o
obj-$(CONFIG_GUP_BENCHMARK) += gup_benchmark.o
obj-$(CONFIG_MEMORY_FAILURE) += memory-failure.o
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5beea03dd58a..446141b6597a 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -82,11 +82,7 @@
static bool cgroup_memory_nokmem;

/* Whether the swap controller is active */
-#ifdef CONFIG_MEMCG_SWAP
int do_swap_account __read_mostly;
-#else
-#define do_swap_account 0
-#endif

#ifdef CONFIG_CGROUP_WRITEBACK
static DECLARE_WAIT_QUEUE_HEAD(memcg_cgwb_frn_waitq);
@@ -3020,7 +3016,6 @@ void mem_cgroup_split_huge_fixup(struct page *head)
}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */

-#ifdef CONFIG_MEMCG_SWAP
/**
* mem_cgroup_move_swap_account - move swap charge and swap_cgroup's record.
* @entry: swap entry to be moved
@@ -3050,13 +3045,6 @@ static int mem_cgroup_move_swap_account(swp_entry_t entry,
}
return -EINVAL;
}
-#else
-static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
- struct mem_cgroup *from, struct mem_cgroup *to)
-{
- return -EINVAL;
-}
-#endif

static DEFINE_MUTEX(memcg_max_mutex);

@@ -6936,7 +6924,6 @@ static int __init mem_cgroup_init(void)
}
subsys_initcall(mem_cgroup_init);

-#ifdef CONFIG_MEMCG_SWAP
static struct mem_cgroup *mem_cgroup_id_get_online(struct mem_cgroup *memcg)
{
while (!refcount_inc_not_zero(&memcg->id.ref)) {
@@ -7137,11 +7124,7 @@ bool mem_cgroup_swap_full(struct page *page)
}

/* for remember boot option*/
-#ifdef CONFIG_MEMCG_SWAP_ENABLED
static int really_do_swap_account __initdata = 1;
-#else
-static int really_do_swap_account __initdata;
-#endif

static int __init enable_swap_account(char *s)
{
@@ -7257,4 +7240,3 @@ static int __init mem_cgroup_swap_init(void)
}
subsys_initcall(mem_cgroup_swap_init);

-#endif /* CONFIG_MEMCG_SWAP */
--
1.8.3.1

2020-04-20 08:06:24

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] memcg: folding CONFIG_MEMCG_SWAP as default

On Sat 18-04-20 21:33:08, Alex Shi wrote:
> This patch fold MEMCG_SWAP feature into kernel as default function. With
> the next swapaccount removing patch, the swapout pages for each of memcg
> always be recorded into swap_cgroup.id. the cost as Johannes mentioned
>
> "the overhead of tracking is tiny - 512k per G of swap (0.04%).'
>
> With this record info, swapin don't have to add all readahead page
> into lru list before do memcg charge.

Why is this really an advantage. Please keep in mind that the changelog
really needs to have a self contained information. If this is a part of
larger changes be _expplicit_ about that.

This is especially important when you are changing a long standing default
behavior. The memory overhead of the swap tracking might be small from
todays perspective but we are not changing behavior just nilly willy. If
there is a really important follow up that would be impossible or too
clumsy without this change then make it clear because that would be
a strong argument to judge this patch.

Btw. I do know that you have referenced the respective discussion with
Johannes in the previous version of the patch but I really want to
stress out the importance of having the information in the changelog.

> Signed-off-by: Alex Shi <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Johannes Weiner <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: Vladimir Davydov <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> arch/arm/configs/omap2plus_defconfig | 1 -
> arch/arm64/configs/defconfig | 1 -
> arch/mips/configs/db1xxx_defconfig | 1 -
> arch/mips/configs/generic_defconfig | 1 -
> arch/mips/configs/loongson3_defconfig | 1 -
> arch/parisc/configs/generic-64bit_defconfig | 1 -
> arch/powerpc/configs/powernv_defconfig | 1 -
> arch/powerpc/configs/pseries_defconfig | 1 -
> arch/s390/configs/debug_defconfig | 1 -
> arch/s390/configs/defconfig | 1 -
> arch/sh/configs/sdk7786_defconfig | 1 -
> arch/sh/configs/urquell_defconfig | 1 -
> include/linux/memcontrol.h | 2 --
> include/linux/swap.h | 27 --------------------------
> include/linux/swap_cgroup.h | 30 -----------------------------
> init/Kconfig | 20 -------------------
> mm/Makefile | 4 ++--
> mm/memcontrol.c | 18 -----------------
> 18 files changed, 2 insertions(+), 111 deletions(-)
>
> diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
> index 3cc3ca5fa027..929cfc4c062a 100644
> --- a/arch/arm/configs/omap2plus_defconfig
> +++ b/arch/arm/configs/omap2plus_defconfig
> @@ -10,7 +10,6 @@ CONFIG_IKCONFIG_PROC=y
> CONFIG_LOG_BUF_SHIFT=16
> CONFIG_CGROUPS=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_BLK_CGROUP=y
> CONFIG_CGROUP_SCHED=y
> CONFIG_CFS_BANDWIDTH=y
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 24e534d85045..3854c6140a04 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -13,7 +13,6 @@ CONFIG_IKCONFIG=y
> CONFIG_IKCONFIG_PROC=y
> CONFIG_NUMA_BALANCING=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_BLK_CGROUP=y
> CONFIG_CGROUP_PIDS=y
> CONFIG_CGROUP_HUGETLB=y
> diff --git a/arch/mips/configs/db1xxx_defconfig b/arch/mips/configs/db1xxx_defconfig
> index e6f3e8e3da39..4604e826d03f 100644
> --- a/arch/mips/configs/db1xxx_defconfig
> +++ b/arch/mips/configs/db1xxx_defconfig
> @@ -9,7 +9,6 @@ CONFIG_HIGH_RES_TIMERS=y
> CONFIG_LOG_BUF_SHIFT=16
> CONFIG_CGROUPS=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_BLK_CGROUP=y
> CONFIG_CGROUP_SCHED=y
> CONFIG_CFS_BANDWIDTH=y
> diff --git a/arch/mips/configs/generic_defconfig b/arch/mips/configs/generic_defconfig
> index 714169e411cf..48e4e251779b 100644
> --- a/arch/mips/configs/generic_defconfig
> +++ b/arch/mips/configs/generic_defconfig
> @@ -3,7 +3,6 @@ CONFIG_NO_HZ_IDLE=y
> CONFIG_IKCONFIG=y
> CONFIG_IKCONFIG_PROC=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_BLK_CGROUP=y
> CONFIG_CFS_BANDWIDTH=y
> CONFIG_RT_GROUP_SCHED=y
> diff --git a/arch/mips/configs/loongson3_defconfig b/arch/mips/configs/loongson3_defconfig
> index 51675f5000d6..f91f49595100 100644
> --- a/arch/mips/configs/loongson3_defconfig
> +++ b/arch/mips/configs/loongson3_defconfig
> @@ -13,7 +13,6 @@ CONFIG_TASK_DELAY_ACCT=y
> CONFIG_TASK_XACCT=y
> CONFIG_TASK_IO_ACCOUNTING=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_BLK_CGROUP=y
> CONFIG_CPUSETS=y
> CONFIG_SCHED_AUTOGROUP=y
> diff --git a/arch/parisc/configs/generic-64bit_defconfig b/arch/parisc/configs/generic-64bit_defconfig
> index 59561e04e659..fab4401dd9f6 100644
> --- a/arch/parisc/configs/generic-64bit_defconfig
> +++ b/arch/parisc/configs/generic-64bit_defconfig
> @@ -10,7 +10,6 @@ CONFIG_TASK_XACCT=y
> CONFIG_TASK_IO_ACCOUNTING=y
> CONFIG_CGROUPS=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_CGROUP_PIDS=y
> CONFIG_CPUSETS=y
> CONFIG_RELAY=y
> diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig
> index df8bdbaa5d8f..c60c3a4125f7 100644
> --- a/arch/powerpc/configs/powernv_defconfig
> +++ b/arch/powerpc/configs/powernv_defconfig
> @@ -17,7 +17,6 @@ CONFIG_LOG_CPU_MAX_BUF_SHIFT=13
> CONFIG_NUMA_BALANCING=y
> CONFIG_CGROUPS=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_CGROUP_SCHED=y
> CONFIG_CGROUP_FREEZER=y
> CONFIG_CPUSETS=y
> diff --git a/arch/powerpc/configs/pseries_defconfig b/arch/powerpc/configs/pseries_defconfig
> index 0bea4d3ffb85..426244f491ec 100644
> --- a/arch/powerpc/configs/pseries_defconfig
> +++ b/arch/powerpc/configs/pseries_defconfig
> @@ -16,7 +16,6 @@ CONFIG_LOG_CPU_MAX_BUF_SHIFT=13
> CONFIG_NUMA_BALANCING=y
> CONFIG_CGROUPS=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_CGROUP_SCHED=y
> CONFIG_CGROUP_FREEZER=y
> CONFIG_CPUSETS=y
> diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
> index 46038bc58c9e..864b32403673 100644
> --- a/arch/s390/configs/debug_defconfig
> +++ b/arch/s390/configs/debug_defconfig
> @@ -14,7 +14,6 @@ CONFIG_IKCONFIG=y
> CONFIG_IKCONFIG_PROC=y
> CONFIG_NUMA_BALANCING=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_BLK_CGROUP=y
> CONFIG_CFS_BANDWIDTH=y
> CONFIG_RT_GROUP_SCHED=y
> diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
> index 7cd0648c1f4e..991e60d7c307 100644
> --- a/arch/s390/configs/defconfig
> +++ b/arch/s390/configs/defconfig
> @@ -13,7 +13,6 @@ CONFIG_IKCONFIG=y
> CONFIG_IKCONFIG_PROC=y
> CONFIG_NUMA_BALANCING=y
> CONFIG_MEMCG=y
> -CONFIG_MEMCG_SWAP=y
> CONFIG_BLK_CGROUP=y
> CONFIG_CFS_BANDWIDTH=y
> CONFIG_RT_GROUP_SCHED=y
> diff --git a/arch/sh/configs/sdk7786_defconfig b/arch/sh/configs/sdk7786_defconfig
> index 7fa116b436c3..4a1d19068a8d 100644
> --- a/arch/sh/configs/sdk7786_defconfig
> +++ b/arch/sh/configs/sdk7786_defconfig
> @@ -17,7 +17,6 @@ CONFIG_CPUSETS=y
> # CONFIG_PROC_PID_CPUSET is not set
> CONFIG_CGROUP_CPUACCT=y
> CONFIG_CGROUP_MEMCG=y
> -CONFIG_CGROUP_MEMCG_SWAP=y
> CONFIG_CGROUP_SCHED=y
> CONFIG_RT_GROUP_SCHED=y
> CONFIG_BLK_CGROUP=y
> diff --git a/arch/sh/configs/urquell_defconfig b/arch/sh/configs/urquell_defconfig
> index cb2f56468fe0..be478f3148f2 100644
> --- a/arch/sh/configs/urquell_defconfig
> +++ b/arch/sh/configs/urquell_defconfig
> @@ -14,7 +14,6 @@ CONFIG_CPUSETS=y
> # CONFIG_PROC_PID_CPUSET is not set
> CONFIG_CGROUP_CPUACCT=y
> CONFIG_CGROUP_MEMCG=y
> -CONFIG_CGROUP_MEMCG_SWAP=y
> CONFIG_CGROUP_SCHED=y
> CONFIG_RT_GROUP_SCHED=y
> CONFIG_BLK_DEV_INITRD=y
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 1b4150ff64be..45842ed8ba53 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -569,9 +569,7 @@ struct mem_cgroup *mem_cgroup_get_oom_group(struct task_struct *victim,
> struct mem_cgroup *oom_domain);
> void mem_cgroup_print_oom_group(struct mem_cgroup *memcg);
>
> -#ifdef CONFIG_MEMCG_SWAP
> extern int do_swap_account;
> -#endif
>
> struct mem_cgroup *lock_page_memcg(struct page *page);
> void __unlock_page_memcg(struct mem_cgroup *memcg);
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index eae48a3896e8..5627fb8ca827 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -652,38 +652,11 @@ static inline void mem_cgroup_throttle_swaprate(struct mem_cgroup *memcg,
> }
> #endif
>
> -#ifdef CONFIG_MEMCG_SWAP
> extern void mem_cgroup_swapout(struct page *page, swp_entry_t entry);
> 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);
> -#else
> -static inline void mem_cgroup_swapout(struct page *page, swp_entry_t entry)
> -{
> -}
> -
> -static inline int mem_cgroup_try_charge_swap(struct page *page,
> - swp_entry_t entry)
> -{
> - return 0;
> -}
> -
> -static inline void mem_cgroup_uncharge_swap(swp_entry_t entry,
> - unsigned int nr_pages)
> -{
> -}
> -
> -static inline long mem_cgroup_get_nr_swap_pages(struct mem_cgroup *memcg)
> -{
> - return get_nr_swap_pages();
> -}
> -
> -static inline bool mem_cgroup_swap_full(struct page *page)
> -{
> - return vm_swap_full();
> -}
> -#endif
>
> #endif /* __KERNEL__*/
> #endif /* _LINUX_SWAP_H */
> diff --git a/include/linux/swap_cgroup.h b/include/linux/swap_cgroup.h
> index a12dd1c3966c..5ac87b3b0e08 100644
> --- a/include/linux/swap_cgroup.h
> +++ b/include/linux/swap_cgroup.h
> @@ -4,8 +4,6 @@
>
> #include <linux/swap.h>
>
> -#ifdef CONFIG_MEMCG_SWAP
> -
> extern unsigned short swap_cgroup_cmpxchg(swp_entry_t ent,
> unsigned short old, unsigned short new);
> extern unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id,
> @@ -14,32 +12,4 @@ extern unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id,
> extern int swap_cgroup_swapon(int type, unsigned long max_pages);
> extern void swap_cgroup_swapoff(int type);
>
> -#else
> -
> -static inline
> -unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id,
> - unsigned int nr_ents)
> -{
> - return 0;
> -}
> -
> -static inline
> -unsigned short lookup_swap_cgroup_id(swp_entry_t ent)
> -{
> - return 0;
> -}
> -
> -static inline int
> -swap_cgroup_swapon(int type, unsigned long max_pages)
> -{
> - return 0;
> -}
> -
> -static inline void swap_cgroup_swapoff(int type)
> -{
> - return;
> -}
> -
> -#endif /* CONFIG_MEMCG_SWAP */
> -
> #endif /* __LINUX_SWAP_CGROUP_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index 9e22ee8fbd75..3b09c71e9b57 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -834,26 +834,6 @@ config MEMCG
> help
> Provides control over the memory footprint of tasks in a cgroup.
>
> -config MEMCG_SWAP
> - bool "Swap controller"
> - depends on MEMCG && SWAP
> - help
> - Provides control over the swap space consumed by tasks in a cgroup.
> -
> -config MEMCG_SWAP_ENABLED
> - bool "Swap controller enabled by default"
> - depends on MEMCG_SWAP
> - default y
> - help
> - Memory Resource Controller Swap Extension comes with its price in
> - a bigger memory consumption. General purpose distribution kernels
> - which want to enable the feature but keep it disabled by default
> - and let the user enable it by swapaccount=1 boot command line
> - parameter should have this option unselected.
> - For those who want to have the feature enabled by default should
> - select this option (if, for some reason, they need to disable it
> - then swapaccount=0 does the trick).
> -
> config MEMCG_KMEM
> bool
> depends on MEMCG && !SLOB
> diff --git a/mm/Makefile b/mm/Makefile
> index fccd3756b25f..812c08323355 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -58,7 +58,8 @@ ifdef CONFIG_MMU
> obj-$(CONFIG_ADVISE_SYSCALLS) += madvise.o
> endif
>
> -obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o swap_slots.o
> +obj-$(CONFIG_SWAP) += page_io.o swap_state.o swapfile.o swap_slots.o \
> + swap_cgroup.o
> obj-$(CONFIG_FRONTSWAP) += frontswap.o
> obj-$(CONFIG_ZSWAP) += zswap.o
> obj-$(CONFIG_HAS_DMA) += dmapool.o
> @@ -80,7 +81,6 @@ obj-$(CONFIG_MIGRATION) += migrate.o
> obj-$(CONFIG_TRANSPARENT_HUGEPAGE) += huge_memory.o khugepaged.o
> obj-$(CONFIG_PAGE_COUNTER) += page_counter.o
> obj-$(CONFIG_MEMCG) += memcontrol.o vmpressure.o
> -obj-$(CONFIG_MEMCG_SWAP) += swap_cgroup.o
> obj-$(CONFIG_CGROUP_HUGETLB) += hugetlb_cgroup.o
> obj-$(CONFIG_GUP_BENCHMARK) += gup_benchmark.o
> obj-$(CONFIG_MEMORY_FAILURE) += memory-failure.o
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 5beea03dd58a..446141b6597a 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -82,11 +82,7 @@
> static bool cgroup_memory_nokmem;
>
> /* Whether the swap controller is active */
> -#ifdef CONFIG_MEMCG_SWAP
> int do_swap_account __read_mostly;
> -#else
> -#define do_swap_account 0
> -#endif
>
> #ifdef CONFIG_CGROUP_WRITEBACK
> static DECLARE_WAIT_QUEUE_HEAD(memcg_cgwb_frn_waitq);
> @@ -3020,7 +3016,6 @@ void mem_cgroup_split_huge_fixup(struct page *head)
> }
> #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>
> -#ifdef CONFIG_MEMCG_SWAP
> /**
> * mem_cgroup_move_swap_account - move swap charge and swap_cgroup's record.
> * @entry: swap entry to be moved
> @@ -3050,13 +3045,6 @@ static int mem_cgroup_move_swap_account(swp_entry_t entry,
> }
> return -EINVAL;
> }
> -#else
> -static inline int mem_cgroup_move_swap_account(swp_entry_t entry,
> - struct mem_cgroup *from, struct mem_cgroup *to)
> -{
> - return -EINVAL;
> -}
> -#endif
>
> static DEFINE_MUTEX(memcg_max_mutex);
>
> @@ -6936,7 +6924,6 @@ static int __init mem_cgroup_init(void)
> }
> subsys_initcall(mem_cgroup_init);
>
> -#ifdef CONFIG_MEMCG_SWAP
> static struct mem_cgroup *mem_cgroup_id_get_online(struct mem_cgroup *memcg)
> {
> while (!refcount_inc_not_zero(&memcg->id.ref)) {
> @@ -7137,11 +7124,7 @@ bool mem_cgroup_swap_full(struct page *page)
> }
>
> /* for remember boot option*/
> -#ifdef CONFIG_MEMCG_SWAP_ENABLED
> static int really_do_swap_account __initdata = 1;
> -#else
> -static int really_do_swap_account __initdata;
> -#endif
>
> static int __init enable_swap_account(char *s)
> {
> @@ -7257,4 +7240,3 @@ static int __init mem_cgroup_swap_init(void)
> }
> subsys_initcall(mem_cgroup_swap_init);
>
> -#endif /* CONFIG_MEMCG_SWAP */
> --
> 1.8.3.1

--
Michal Hocko
SUSE Labs