[RFC PATCH 2/2] zram: charge the compressed RAM to the page's memcgroup

[RFC PATCH 2/2] zram: charge the compressed RAM to the page's memcgroup

[Zhongkun He]

The compressed RAM is currently charged to kernel, not to
any memory cgroup. This patch can charge the pages
regardless of direct or indirect zram usage.

Direct zram usage by process within a cgroup will fail
to charge if there is no memory. Indirect zram usage by
process within a cgroup via swap in PF_MEMALLOC context,
will charge successfully.

Signed-off-by: Zhongkun He <hezhongkun.hzk@bytedance.com>
---
 drivers/block/zram/zram_drv.c | 43 +++++++++++++++++++++++++++++++++++
 drivers/block/zram/zram_drv.h |  1 +
 2 files changed, 44 insertions(+)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 5676e6dd5b16..770c7d58afec 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -33,6 +33,7 @@
 #include <linux/debugfs.h>
 #include <linux/cpuhotplug.h>
 #include <linux/part_stat.h>
+#include <linux/memcontrol.h>
 
 #include "zram_drv.h"
 
@@ -135,6 +136,18 @@ static void zram_set_obj_size(struct zram *zram,
 	zram->table[index].flags = (flags << ZRAM_FLAG_SHIFT) | size;
 }
 
+static inline void zram_set_obj_cgroup(struct zram *zram, u32 index,
+			struct obj_cgroup *objcg)
+{
+	zram->table[index].objcg = objcg;
+}
+
+static inline struct obj_cgroup *zram_get_obj_cgroup(struct zram *zram,
+			u32 index)
+{
+	return zram->table[index].objcg;
+}
+
 static inline bool zram_allocated(struct zram *zram, u32 index)
 {
 	return zram_get_obj_size(zram, index) ||
@@ -1256,6 +1269,7 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize)
 static void zram_free_page(struct zram *zram, size_t index)
 {
 	unsigned long handle;
+	struct obj_cgroup *objcg;
 
 #ifdef CONFIG_ZRAM_MEMORY_TRACKING
 	zram->table[index].ac_time = 0;
@@ -1289,6 +1303,13 @@ static void zram_free_page(struct zram *zram, size_t index)
 		goto out;
 	}
 
+	objcg = zram_get_obj_cgroup(zram, index);
+	if (objcg) {
+		obj_cgroup_uncharge_zram(objcg, zram_get_obj_size(zram, index));
+		obj_cgroup_put(objcg);
+		zram_set_obj_cgroup(zram, index, NULL);
+	}
+
 	handle = zram_get_handle(zram, index);
 	if (!handle)
 		return;
@@ -1419,6 +1440,7 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 	struct zcomp_strm *zstrm;
 	unsigned long element = 0;
 	enum zram_pageflags flags = 0;
+	struct obj_cgroup *objcg = NULL;
 
 	mem = kmap_atomic(page);
 	if (page_same_filled(mem, &element)) {
@@ -1494,6 +1516,14 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 		return -ENOMEM;
 	}
 
+	objcg = get_obj_cgroup_from_page(page);
+	if (objcg && obj_cgroup_charge_zram(objcg, comp_len)) {
+		zcomp_stream_put(zram->comps[ZRAM_PRIMARY_COMP]);
+		zs_free(zram->mem_pool, handle);
+		obj_cgroup_put(objcg);
+		return -ENOMEM;
+	}
+
 	dst = zs_map_object(zram->mem_pool, handle, ZS_MM_WO);
 
 	src = zstrm->buffer;
@@ -1526,6 +1556,7 @@ static int zram_write_page(struct zram *zram, struct page *page, u32 index)
 	}  else {
 		zram_set_handle(zram, index, handle);
 		zram_set_obj_size(zram, index, comp_len);
+		zram_set_obj_cgroup(zram, index, objcg);
 	}
 	zram_slot_unlock(zram, index);
 
@@ -1581,6 +1612,8 @@ static int zram_recompress(struct zram *zram, u32 index, struct page *page,
 	unsigned int comp_len_new;
 	unsigned int class_index_old;
 	unsigned int class_index_new;
+	struct obj_cgroup *objcg = NULL;
+	unsigned int noreclaim_flag;
 	u32 num_recomps = 0;
 	void *src, *dst;
 	int ret;
@@ -1692,11 +1725,21 @@ static int zram_recompress(struct zram *zram, u32 index, struct page *page,
 
 	zs_unmap_object(zram->mem_pool, handle_new);
 
+	/*
+	 * Recompress will reclaim some memory, so we set the reclaim
+	 * flag in order to charge comp_len_new successfully.
+	 */
+	noreclaim_flag = memalloc_noreclaim_save();
+	objcg  = zram_get_obj_cgroup(zram, index);
+	obj_cgroup_get(objcg);
 	zram_free_page(zram, index);
+	obj_cgroup_charge_zram(objcg, GFP_KERNEL, comp_len_new);
+	zram_set_obj_cgroup(zram, index, objcg);
 	zram_set_handle(zram, index, handle_new);
 	zram_set_obj_size(zram, index, comp_len_new);
 	zram_set_priority(zram, index, prio);
 
+	memalloc_noreclaim_restore(noreclaim_flag);
 	atomic64_add(comp_len_new, &zram->stats.compr_data_size);
 	atomic64_inc(&zram->stats.pages_stored);
 
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index ca7a15bd4845..959d721d5474 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -72,6 +72,7 @@ struct zram_table_entry {
 #ifdef CONFIG_ZRAM_MEMORY_TRACKING
 	ktime_t ac_time;
 #endif
+	struct obj_cgroup *objcg;
 };
 
 struct zram_stats {
-- 
2.25.1

Re: [RFC PATCH 2/2] zram: charge the compressed RAM to the page's memcgroup

[Michal Hocko]

On Fri 07-07-23 12:47:07, Zhongkun He wrote:
[...]
> @@ -1692,11 +1725,21 @@ static int zram_recompress(struct zram *zram, u32 index, struct page *page,
>  
>  	zs_unmap_object(zram->mem_pool, handle_new);
>  
> +	/*
> +	 * Recompress will reclaim some memory, so we set the reclaim
> +	 * flag in order to charge comp_len_new successfully.
> +	 */
> +	noreclaim_flag = memalloc_noreclaim_save();
> +	objcg  = zram_get_obj_cgroup(zram, index);
> +	obj_cgroup_get(objcg);
>  	zram_free_page(zram, index);
> +	obj_cgroup_charge_zram(objcg, GFP_KERNEL, comp_len_new);

AFAICS your obj_cgroup_charge_zram doesn't have gfp argument.

Anyway, memalloc_noreclaim_save is an abuse IMHO (the primary purpose of
the flag is to prevent recursion into the memory reclaim). Do you really
can not perform any memory recalim to trigger to free up some memory if
the memcg is at the hard limit boundary?

> +	zram_set_obj_cgroup(zram, index, objcg);
>  	zram_set_handle(zram, index, handle_new);
>  	zram_set_obj_size(zram, index, comp_len_new);
>  	zram_set_priority(zram, index, prio);
>  
> +	memalloc_noreclaim_restore(noreclaim_flag);
>  	atomic64_add(comp_len_new, &zram->stats.compr_data_size);
>  	atomic64_inc(&zram->stats.pages_stored);
>  
-- 
Michal Hocko
SUSE Labs

Re: [External] Re: [RFC PATCH 2/2] zram: charge the compressed RAM to the page's memcgroup

[贺中坤]

>
> AFAICS your obj_cgroup_charge_zram doesn't have gfp argument.
>
> Anyway, memalloc_noreclaim_save is an abuse IMHO (the primary purpose of
> the flag is to prevent recursion into the memory reclaim). Do you really
> can not perform any memory recalim to trigger to free up some memory if
> the memcg is at the hard limit boundary?
>

Got it . I agree, memalloc_noreclaim_save should not be used, but return nomem
directly,which is more clear and satisfies both direct and indirect usage.