Message-Id: <20230919095237.26613-3-link@vivo.com>
Date:   Tue, 19 Sep 2023 17:52:35 +0800
From:   Huan Yang <link@...o.com>
To:     SeongJae Park <sj@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org (open list),
        damon@...ts.linux.dev (open list:DATA ACCESS MONITOR),
        linux-mm@...ck.org (open list:DATA ACCESS MONITOR)
Cc:     opensource.kernel@...o.com, Huan Yang <link@...o.com>
Subject: [RFC 2/2] mm/damos/filter: Damos filter cleanup

Now damos support filter contains two type.
The first is scheme filter which will invoke each scheme apply,
second is scheme action filter, which will filter out unwanted action.

But this implement is scattered in different implementations and hard
to reuse or extend.

This patch clean up those filter code, move into filter.c and add header
to expose filter func.

Each filter implement register itself into filter_fn array, and user
can invoke this by there inited type in filter.

Signed-off-by: Huan Yang <link@...o.com>
---
 include/linux/damon.h    |  65 +------------------
 mm/damon/Makefile        |   2 +-
 mm/damon/core.c          |  93 ++++-----------------------
 mm/damon/filter.c        | 135 +++++++++++++++++++++++++++++++++++++++
 mm/damon/filter.h        | 119 ++++++++++++++++++++++++++++++++++
 mm/damon/paddr.c         |  31 +++------
 mm/damon/reclaim.c       |   1 +
 mm/damon/sysfs-schemes.c |   1 +
 8 files changed, 280 insertions(+), 167 deletions(-)
 create mode 100644 mm/damon/filter.c
 create mode 100644 mm/damon/filter.h

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 8e8f35df6a5e..03c718b30bfe 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -222,63 +222,6 @@ struct damos_stat {
 	unsigned long qt_exceeds;
 };
 
-/**
- * enum damos_filter_type - Type of memory for &struct damos_filter
- * @DAMOS_FILTER_TYPE_ANON:	Anonymous pages.
- * @DAMOS_FILTER_TYPE_MEMCG:	Specific memcg's pages.
- * @DAMOS_FILTER_TYPE_ADDR:	Address range.
- * @DAMOS_FILTER_TYPE_TARGET:	Data Access Monitoring target.
- * @DAMOS_FILTER_TYPE_WORKINGSET: Workingset pages, need protect.
- * @NR_DAMOS_FILTER_TYPES:	Number of filter types.
- *
- * The anon pages type, memcg type, workingset page type filters are handled
- * by underlying &struct damon_operations as a part of scheme action trying,
- * and therefore accounted as 'tried'.  In contrast, other types are handled
- * by core layer before trying of the action and therefore not accounted as
- * 'tried'.
- *
- * The support of the filters that handled by &struct damon_operations depend
- * on the running &struct damon_operations.
- * &enum DAMON_OPS_PADDR supports both anon pages type, memcg type and
- * workingset page type filters, while &enum DAMON_OPS_VADDR and &enum
- * DAMON_OPS_FVADDR don't support any of the two types.
- */
-enum damos_filter_type {
-	DAMOS_FILTER_TYPE_ANON,
-	DAMOS_FILTER_TYPE_MEMCG,
-	DAMOS_FILTER_TYPE_WORKINGSET,
-	DAMOS_FILTER_TYPE_ADDR,
-	DAMOS_FILTER_TYPE_TARGET,
-	NR_DAMOS_FILTER_TYPES,
-};
-
-/**
- * struct damos_filter - DAMOS action target memory filter.
- * @type:	Type of the page.
- * @matching:	If the matching page should filtered out or in.
- * @memcg_id:	Memcg id of the question if @type is DAMOS_FILTER_MEMCG.
- * @addr_range:	Address range if @type is DAMOS_FILTER_TYPE_ADDR.
- * @target_idx:	Index of the &struct damon_target of
- *		&damon_ctx->adaptive_targets if @type is
- *		DAMOS_FILTER_TYPE_TARGET.
- * @list:	List head for siblings.
- *
- * Before applying the &damos->action to a memory region, DAMOS checks if each
- * page of the region matches to this and avoid applying the action if so.
- * Support of each filter type depends on the running &struct damon_operations
- * and the type.  Refer to &enum damos_filter_type for more detai.
- */
-struct damos_filter {
-	enum damos_filter_type type;
-	bool matching;
-	union {
-		unsigned short memcg_id;
-		struct damon_addr_range addr_range;
-		int target_idx;
-	};
-	struct list_head list;
-};
-
 /**
  * struct damos_access_pattern - Target access pattern of the given scheme.
  * @min_sz_region:	Minimum size of target regions.
@@ -607,16 +550,14 @@ static inline void damon_insert_region(struct damon_region *r,
 	t->nr_regions++;
 }
 
+void damon_split_region_at(struct damon_target *t,
+				  struct damon_region *r, unsigned long sz_r);
+
 void damon_add_region(struct damon_region *r, struct damon_target *t);
 void damon_destroy_region(struct damon_region *r, struct damon_target *t);
 int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
 		unsigned int nr_ranges);
 
-struct damos_filter *damos_new_filter(enum damos_filter_type type,
-		bool matching);
-void damos_add_filter(struct damos *s, struct damos_filter *f);
-void damos_destroy_filter(struct damos_filter *f);
-
 struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
 			enum damos_action action, struct damos_quota *quota,
 			struct damos_watermarks *wmarks);
diff --git a/mm/damon/Makefile b/mm/damon/Makefile
index f7add3f4aa79..789ab0a9d9c9 100644
--- a/mm/damon/Makefile
+++ b/mm/damon/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-obj-y				:= core.o
+obj-y				:= core.o filter.o
 obj-$(CONFIG_DAMON_VADDR)	+= ops-common.o vaddr.o
 obj-$(CONFIG_DAMON_PADDR)	+= ops-common.o paddr.o
 obj-$(CONFIG_DAMON_SYSFS)	+= sysfs-common.o sysfs-schemes.o sysfs.o
diff --git a/mm/damon/core.c b/mm/damon/core.c
index bcd2bd9d6c10..a74932fcdb11 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -22,6 +22,8 @@
 #define DAMON_MIN_REGION 1
 #endif
 
+#include "filter.h"
+
 static DEFINE_MUTEX(damon_lock);
 static int nr_running_ctxs;
 static bool running_exclusive_ctxs;
@@ -263,41 +265,6 @@ int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
 	return 0;
 }
 
-struct damos_filter *damos_new_filter(enum damos_filter_type type,
-		bool matching)
-{
-	struct damos_filter *filter;
-
-	filter = kmalloc(sizeof(*filter), GFP_KERNEL);
-	if (!filter)
-		return NULL;
-	filter->type = type;
-	filter->matching = matching;
-	INIT_LIST_HEAD(&filter->list);
-	return filter;
-}
-
-void damos_add_filter(struct damos *s, struct damos_filter *f)
-{
-	list_add_tail(&f->list, &s->filters);
-}
-
-static void damos_del_filter(struct damos_filter *f)
-{
-	list_del(&f->list);
-}
-
-static void damos_free_filter(struct damos_filter *f)
-{
-	kfree(f);
-}
-
-void damos_destroy_filter(struct damos_filter *f)
-{
-	damos_del_filter(f);
-	damos_free_filter(f);
-}
-
 /* initialize private fields of damos_quota and return the pointer */
 static struct damos_quota *damos_quota_init_priv(struct damos_quota *quota)
 {
@@ -780,9 +747,6 @@ static void kdamond_reset_aggregated(struct damon_ctx *c)
 	}
 }
 
-static void damon_split_region_at(struct damon_target *t,
-				  struct damon_region *r, unsigned long sz_r);
-
 static bool __damos_valid_target(struct damon_region *r, struct damos *s)
 {
 	unsigned long sz;
@@ -881,49 +845,16 @@ static void damos_update_stat(struct damos *s,
 static bool __damos_filter_out(struct damon_ctx *ctx, struct damon_target *t,
 		struct damon_region *r, struct damos_filter *filter)
 {
-	bool matched = false;
-	struct damon_target *ti;
-	int target_idx = 0;
-	unsigned long start, end;
-
-	switch (filter->type) {
-	case DAMOS_FILTER_TYPE_TARGET:
-		damon_for_each_target(ti, ctx) {
-			if (ti == t)
-				break;
-			target_idx++;
-		}
-		matched = target_idx == filter->target_idx;
-		break;
-	case DAMOS_FILTER_TYPE_ADDR:
-		start = ALIGN_DOWN(filter->addr_range.start, DAMON_MIN_REGION);
-		end = ALIGN_DOWN(filter->addr_range.end, DAMON_MIN_REGION);
-
-		/* inside the range */
-		if (start <= r->ar.start && r->ar.end <= end) {
-			matched = true;
-			break;
-		}
-		/* outside of the range */
-		if (r->ar.end <= start || end <= r->ar.start) {
-			matched = false;
-			break;
-		}
-		/* start before the range and overlap */
-		if (r->ar.start < start) {
-			damon_split_region_at(t, r, start - r->ar.start);
-			matched = false;
-			break;
-		}
-		/* start inside the range */
-		damon_split_region_at(t, r, end - r->ar.start);
-		matched = true;
-		break;
-	default:
-		break;
-	}
+	struct damos_filter_ctx dfctx = {
+		.scheme = {
+			.ctx = ctx,
+			.target = t,
+			.region = r,
+		},
+		.filter = filter,
+	};
 
-	return matched == filter->matching;
+	return damon_filter_invoke(&dfctx);
 }
 
 static bool damos_filter_out(struct damon_ctx *ctx, struct damon_target *t,
@@ -1161,7 +1092,7 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
  * r		the region to be split
  * sz_r		size of the first sub-region that will be made
  */
-static void damon_split_region_at(struct damon_target *t,
+void damon_split_region_at(struct damon_target *t,
 				  struct damon_region *r, unsigned long sz_r)
 {
 	struct damon_region *new;
diff --git a/mm/damon/filter.c b/mm/damon/filter.c
new file mode 100644
index 000000000000..a451d5428fe2
--- /dev/null
+++ b/mm/damon/filter.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Data Access Monitor Filter
+ *
+ * This filter contains the scheme filter, which will be called
+ * for each scheme apply check. action filter will be called when
+ * scheme action invoke.
+ *
+ * Author: Huan Yang <link@...o.com>
+ */
+#include <linux/damon.h>
+
+#include "filter.h"
+
+typedef bool (*DAMON_FILTER_FN) (struct damos_filter_ctx *dfctx);
+
+/* Scheme action filter */
+
+static bool damos_anon_filter(struct damos_filter_ctx *dfctx)
+{
+	return folio_test_anon(dfctx->action.folio) == dfctx->filter->matching;
+}
+
+static bool damos_memcg_filter(struct damos_filter_ctx *dfctx)
+{
+	struct damos_filter *filter = dfctx->filter;
+	struct mem_cgroup *memcg;
+	bool matched;
+
+	rcu_read_lock();
+	memcg = folio_memcg_check(dfctx->action.folio);
+	if (!memcg)
+		matched = false;
+	else
+		matched = filter->memcg_id == mem_cgroup_id(memcg);
+	rcu_read_unlock();
+
+	return matched == filter->matching;
+}
+
+static bool damos_workingset_filter(struct damos_filter_ctx *dfctx)
+{
+	return folio_test_workingset(dfctx->action.folio) ==
+	       dfctx->filter->matching;
+}
+
+/* Scheme filter */
+
+static bool damos_addr_filter(struct damos_filter_ctx *dfctx)
+{
+	bool matched = false;
+	struct damos_filter *filter = dfctx->filter;
+	struct damon_target *t = dfctx->scheme.target;
+	struct damon_region *r = dfctx->scheme.region;
+	unsigned long start, end;
+
+	start = ALIGN_DOWN(filter->addr_range.start, DAMON_MIN_REGION);
+	end = ALIGN_DOWN(filter->addr_range.end, DAMON_MIN_REGION);
+
+	/* inside the range */
+	if (start <= r->ar.start && r->ar.end <= end) {
+		matched = true;
+		goto got_result;
+	}
+	/* outside of the range */
+	if (r->ar.end <= start || end <= r->ar.start) {
+		matched = false;
+		goto got_result;
+	}
+	/* start before the range and overlap */
+	if (r->ar.start < start) {
+		damon_split_region_at(t, r, start - r->ar.start);
+		matched = false;
+		goto got_result;
+	}
+	/* start inside the range */
+	damon_split_region_at(t, r, end - r->ar.start);
+	matched = true;
+
+got_result:
+	return matched == filter->matching;
+}
+
+static bool damos_target_filter(struct damos_filter_ctx *dfctx)
+{
+	bool matched = false;
+	struct damon_target *ti, *t = dfctx->scheme.target;
+	struct damon_ctx *ctx = dfctx->scheme.ctx;
+	struct damos_filter *filter = dfctx->filter;
+	int target_idx = 0;
+
+	damon_for_each_target(ti, ctx) {
+		if (ti == t)
+			break;
+		target_idx++;
+	}
+	matched = target_idx == filter->target_idx;
+	return matched == filter->matching;
+}
+
+static DAMON_FILTER_FN filter_fn[NR_DAMOS_FILTER_TYPES] = {
+	/* Damos scheme action filter */
+	damos_anon_filter,
+	damos_memcg_filter,
+	damos_workingset_filter,
+	/* Damos scheme filter */
+	damos_addr_filter,
+	damos_target_filter,
+};
+
+
+bool damon_filter_invoke(struct damos_filter_ctx *dfctx)
+{
+	return filter_fn[dfctx->filter->type](dfctx);
+}
+
+struct damos_filter *damos_new_filter(enum damos_filter_type type,
+		bool matching)
+{
+	struct damos_filter *filter;
+
+	filter = kmalloc(sizeof(*filter), GFP_KERNEL);
+	if (!filter)
+		return NULL;
+	filter->type = type;
+	filter->matching = matching;
+	INIT_LIST_HEAD(&filter->list);
+	return filter;
+}
+
+void damos_destroy_filter(struct damos_filter *f)
+{
+	damos_del_filter(f);
+	damos_free_filter(f);
+}
diff --git a/mm/damon/filter.h b/mm/damon/filter.h
new file mode 100644
index 000000000000..5d724e8276fc
--- /dev/null
+++ b/mm/damon/filter.h
@@ -0,0 +1,119 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Data Access Monitor Filter
+ *
+ * Author: Huan Yang <link@...o.com>
+ */
+#ifndef __DAMON_FILTER_H__
+#define __DAMON_FILTER_H__
+
+/**
+ * enum damos_filter_type - Type of memory for &struct damos_filter
+ * @DAMOS_FILTER_TYPE_ANON:	Anonymous pages.
+ * @DAMOS_FILTER_TYPE_MEMCG:	Specific memcg's pages.
+ * @DAMOS_FILTER_TYPE_ADDR:	Address range.
+ * @DAMOS_FILTER_TYPE_TARGET:	Data Access Monitoring target.
+ * @DAMOS_FILTER_TYPE_WORKINGSET: Workingset pages, need protect.
+ * @NR_DAMOS_FILTER_TYPES:	Number of filter types.
+ *
+ * The anon pages type, memcg type, workingset page type filters are handled
+ * by underlying &struct damon_operations as a part of scheme action trying,
+ * and therefore accounted as 'tried'.  In contrast, other types are handled
+ * by core layer before trying of the action and therefore not accounted as
+ * 'tried'.
+ *
+ * The support of the filters that handled by &struct damon_operations depend
+ * on the running &struct damon_operations.
+ * &enum DAMON_OPS_PADDR supports both anon pages type, memcg type and
+ * workingset page type filters, while &enum DAMON_OPS_VADDR and &enum
+ * DAMON_OPS_FVADDR don't support any of the two types.
+ */
+enum damos_filter_type {
+	/* Damos action filter, tried */
+	DAMOS_FILTER_TYPE_ANON,
+	DAMOS_FILTER_TYPE_MEMCG,
+	DAMOS_FILTER_TYPE_WORKINGSET,
+	/* Damos scheme filter */
+	DAMOS_FILTER_TYPE_ADDR,
+	DAMOS_FILTER_TYPE_TARGET,
+	NR_DAMOS_FILTER_TYPES,
+};
+
+/**
+ * struct damos_filter - DAMOS action target memory filter.
+ * @type:	Type of the page.
+ * @matching:	If the matching page should filtered out or in.
+ * @memcg_id:	Memcg id of the question if @type is DAMOS_FILTER_MEMCG.
+ * @addr_range:	Address range if @type is DAMOS_FILTER_TYPE_ADDR.
+ * @target_idx:	Index of the &struct damon_target of
+ *		&damon_ctx->adaptive_targets if @type is
+ *		DAMOS_FILTER_TYPE_TARGET.
+ * @list:	List head for siblings.
+ *
+ * Before applying the &damos->action to a memory region, DAMOS checks if each
+ * page of the region matches to this and avoid applying the action if so.
+ * Support of each filter type depends on the running &struct damon_operations
+ * and the type.  Refer to &enum damos_filter_type for more detai.
+ */
+struct damos_filter {
+	enum damos_filter_type type;
+	bool matching;
+	union {
+		unsigned short memcg_id;
+		struct damon_addr_range addr_range;
+		int target_idx;
+	};
+	struct list_head list;
+};
+
+/**
+ * struct damos_filter_ctx - Represents a context for each filter
+ * @scheme:     Each scheme filter context
+ * @action:     Each scheme action filter context
+ * @filter:     DAMOS action target memory filter instance
+ *
+ * User need pass this for each invoke filter, and each filter
+ * will invoke a specified filter function by filter type which
+ * user already target it when register filter.
+ */
+struct damos_filter_ctx {
+	union {
+		/* Use by scheme filter */
+		struct {
+			struct damon_ctx *ctx;
+			struct damon_target *target;
+			struct damon_region *region;
+		} scheme;
+
+		/* Use by action filter */
+		struct {
+			struct folio *folio;
+		} action;
+	};
+
+	struct damos_filter *filter;
+};
+
+bool damon_filter_invoke(struct damos_filter_ctx *dfctx);
+
+struct damos_filter *damos_new_filter(enum damos_filter_type type,
+				      bool matching);
+
+static inline void damos_add_filter(struct damos *s, struct damos_filter *f)
+{
+	list_add_tail(&f->list, &s->filters);
+}
+
+static inline void damos_del_filter(struct damos_filter *f)
+{
+	list_del(&f->list);
+}
+
+static inline void damos_free_filter(struct damos_filter *f)
+{
+	kfree(f);
+}
+
+void damos_destroy_filter(struct damos_filter *f);
+
+#endif //__DAMON_FILTER_H__
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 8a690505e033..748300655ba4 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -16,6 +16,7 @@
 
 #include "../internal.h"
 #include "ops-common.h"
+#include "filter.h"
 
 static bool __damon_pa_mkold(struct folio *folio, struct vm_area_struct *vma,
 		unsigned long addr, void *arg)
@@ -189,30 +190,14 @@ static unsigned int damon_pa_check_accesses(struct damon_ctx *ctx)
 static bool __damos_pa_filter_out(struct damos_filter *filter,
 		struct folio *folio)
 {
-	bool matched = false;
-	struct mem_cgroup *memcg;
-
-	switch (filter->type) {
-	case DAMOS_FILTER_TYPE_ANON:
-		matched = folio_test_anon(folio);
-		break;
-	case DAMOS_FILTER_TYPE_MEMCG:
-		rcu_read_lock();
-		memcg = folio_memcg_check(folio);
-		if (!memcg)
-			matched = false;
-		else
-			matched = filter->memcg_id == mem_cgroup_id(memcg);
-		rcu_read_unlock();
-		break;
-	case DAMOS_FILTER_TYPE_WORKINGSET:
-		matched = folio_test_workingset(folio);
-		break;
-	default:
-		break;
-	}
+	struct damos_filter_ctx dfctx = {
+		.action = {
+			.folio = folio,
+		},
+		.filter = filter,
+	};
 
-	return matched == filter->matching;
+	return damon_filter_invoke(&dfctx);
 }
 
 /*
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 26ae8fa5d088..1fdd55c03f1f 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 
 #include "modules-common.h"
+#include "filter.h"
 
 #ifdef MODULE_PARAM_PREFIX
 #undef MODULE_PARAM_PREFIX
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 527e7d17eb3b..98cd93b28b21 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 
 #include "sysfs-common.h"
+#include "filter.h"
 
 /*
  * scheme region directory
-- 
2.34.1

Powered by blists - more mailing lists