[RFC PATCH 0/9] mm/damon: make allow filters after reject filters useful and intuitive

[RFC PATCH 0/9] mm/damon: make allow filters after reject filters useful and intuitive

[SeongJae Park]

DAMOS filters do allow or reject elements of memory for given DAMOS
action only if those match the filter criterias.  For elements that
don't match any DAMOS filter, 'allowing' is the default behavior.  This
makes 'allow DAMOS filters' that don't have any 'reject DAMOS filter'
after them meaningless sources of overhead.  The decision was made to
keep the behavior consistent with before-allow filter behavior.  This,
however, only makes usage of DAMOS filters confusing and inefficient.
There is an apparently more intuitive behavior for the case.  If there
is no filter at all or the last filter is a reject filter, allowing by
default.

Update the filtering logic to work in the way.  Decide the default
behavior as the opposite of the last isntalled filter.  If it is an
allow filter, reject by default.  If it is a reject filter or no filter
is installed at all, allow by default.

Note that this is changing the old behavior.  But the old behavior for
the problematic filter installation was definitely confusing,
inefficient and anyway useless.  Also, the behavior has relatively
recently introduced.  It is difficult to anticipate any user that
depends on the behavior.  Hence this is not a user-breaking behavior
change but an obvious improvement.

SeongJae Park (9):
  mm/damon/core: introduce damos->ops_filters
  mm/damon/paddr: support ops_filters
  mm/damon/core: support committing ops_filters
  mm/damon/core: put ops-handled filters to damos->ops_filters
  mm/damon/paddr: support only ops_filters
  mm/damon: add default allow/reject behavior fields to struct damos
  mm/damon/core: set damos_filter default allowance behavior based on
    installed filters
  mm/damon/paddr: respect ops_filters_default_reject
  Docs/mm/damon/design: update for changed filter-default behavior

 Documentation/mm/damon/design.rst | 10 ++--
 include/linux/damon.h             | 11 ++++
 mm/damon/core.c                   | 87 +++++++++++++++++++++++++++++--
 mm/damon/paddr.c                  |  8 +--
 4 files changed, 102 insertions(+), 14 deletions(-)


base-commit: 3fb99e5d512f03963c91926628fcc08a96ee07f0
-- 
2.39.5

[RFC PATCH 1/9] mm/damon/core: introduce damos->ops_filters

[SeongJae Park]

DAMOS filters can be categorized into two groups depending on which
layer they are handled, namely core layer and ops layer.  Also their
evaluation sequence is decided by the categorization.  Currently, all
filters are maintained in single list in mix.  It makes the filters
evaluation loop inefficient since it should do the categorization in the
loop for every iteration.  Introduce another list that will be used for
having all operations layer-handled DAMOS filters.

Note that this change simply adds the list and does not change any DAMON
code to use it in real.  It will be done by following changes.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h | 8 ++++++++
 mm/damon/core.c       | 1 +
 2 files changed, 9 insertions(+)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 795ca09b1107..add82fdc1117 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -448,6 +448,7 @@ struct damos_access_pattern {
  * @wmarks:		Watermarks for automated (in)activation of this scheme.
  * @target_nid:		Destination node if @action is "migrate_{hot,cold}".
  * @filters:		Additional set of &struct damos_filter for &action.
+ * @ops_filters:	ops layer handling &struct damos_filter objects list.
  * @last_applied:	Last @action applied ops-managing entity.
  * @stat:		Statistics of this scheme.
  * @list:		List head for siblings.
@@ -503,6 +504,7 @@ struct damos {
 		int target_nid;
 	};
 	struct list_head filters;
+	struct list_head ops_filters;
 	void *last_applied;
 	struct damos_stat stat;
 	struct list_head list;
@@ -810,6 +812,12 @@ static inline unsigned long damon_sz_region(struct damon_region *r)
 #define damos_for_each_filter_safe(f, next, scheme) \
 	list_for_each_entry_safe(f, next, &(scheme)->filters, list)
 
+#define damos_for_each_ops_filter(f, scheme) \
+	list_for_each_entry(f, &(scheme)->ops_filters, list)
+
+#define damos_for_each_ops_filter_safe(f, next, scheme) \
+	list_for_each_entry_safe(f, next, &(scheme)->ops_filters, list)
+
 #ifdef CONFIG_DAMON
 
 struct damon_region *damon_new_region(unsigned long start, unsigned long end);
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 38f545fea585..bcb7e42098dc 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -374,6 +374,7 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
 	 */
 	scheme->next_apply_sis = 0;
 	INIT_LIST_HEAD(&scheme->filters);
+	INIT_LIST_HEAD(&scheme->ops_filters);
 	scheme->stat = (struct damos_stat){};
 	INIT_LIST_HEAD(&scheme->list);
 
-- 
2.39.5

[RFC PATCH 2/9] mm/damon/paddr: support ops_filters

[SeongJae Park]

DAMON keeps all DAMOS filters in damos->filters, and will make
damos->ops_filters to have all operations layer handled DAMOS filters
with upcoming changes.  But DAMON physical address space operations set
implementation (paddr) is handling only damos->filters.  To avoid any
breakage during the upcoming change, make paddr to handle both.  After
the change is made, ->filters support on paddr can be safely removed.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/paddr.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 25090230da17..3e651308ba5d 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -257,6 +257,10 @@ static bool damos_pa_filter_out(struct damos *scheme, struct folio *folio)
 		if (damos_pa_filter_match(filter, folio))
 			return !filter->allow;
 	}
+	damos_for_each_ops_filter(filter, scheme) {
+		if (damos_pa_filter_match(filter, folio))
+			return !filter->allow;
+	}
 	return false;
 }
 
@@ -287,6 +291,12 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s,
 			break;
 		}
 	}
+	damos_for_each_ops_filter(filter, s) {
+		if (filter->type == DAMOS_FILTER_TYPE_YOUNG) {
+			install_young_filter = false;
+			break;
+		}
+	}
 	if (install_young_filter) {
 		filter = damos_new_filter(
 				DAMOS_FILTER_TYPE_YOUNG, true, false);
@@ -535,6 +545,8 @@ static bool damon_pa_scheme_has_filter(struct damos *s)
 
 	damos_for_each_filter(f, s)
 		return true;
+	damos_for_each_ops_filter(f, s)
+		return true;
 	return false;
 }
 
-- 
2.39.5

[RFC PATCH 3/9] mm/damon/core: support committing ops_filters

[SeongJae Park]

DAMON kernel API callers should use damon_commit_ctx() to install DAMON
parameters including DAMOS filters.  But damos_commit_ops_filters(),
which is called by damon_commit_ctx() for filters installing, is not
supporting committing damos->ops_filters.  Do the parameters committing
so that DAMON kernel API callers can use damos->ops_filters.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index bcb7e42098dc..ffdd84ff6fa5 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -793,7 +793,7 @@ static void damos_commit_filter(
 	damos_commit_filter_arg(dst, src);
 }
 
-static int damos_commit_filters(struct damos *dst, struct damos *src)
+static int damos_commit_core_filters(struct damos *dst, struct damos *src)
 {
 	struct damos_filter *dst_filter, *next, *src_filter, *new_filter;
 	int i = 0, j = 0;
@@ -821,6 +821,44 @@ static int damos_commit_filters(struct damos *dst, struct damos *src)
 	return 0;
 }
 
+static int damos_commit_ops_filters(struct damos *dst, struct damos *src)
+{
+	struct damos_filter *dst_filter, *next, *src_filter, *new_filter;
+	int i = 0, j = 0;
+
+	damos_for_each_ops_filter_safe(dst_filter, next, dst) {
+		src_filter = damos_nth_filter(i++, src);
+		if (src_filter)
+			damos_commit_filter(dst_filter, src_filter);
+		else
+			damos_destroy_filter(dst_filter);
+	}
+
+	damos_for_each_ops_filter_safe(src_filter, next, src) {
+		if (j++ < i)
+			continue;
+
+		new_filter = damos_new_filter(
+				src_filter->type, src_filter->matching,
+				src_filter->allow);
+		if (!new_filter)
+			return -ENOMEM;
+		damos_commit_filter_arg(new_filter, src_filter);
+		damos_add_filter(dst, new_filter);
+	}
+	return 0;
+}
+
+static int damos_commit_filters(struct damos *dst, struct damos *src)
+{
+	int err;
+
+	err = damos_commit_core_filters(dst, src);
+	if (err)
+		return err;
+	return damos_commit_ops_filters(dst, src);
+}
+
 static struct damos *damon_nth_scheme(int n, struct damon_ctx *ctx)
 {
 	struct damos *s;
-- 
2.39.5

[RFC PATCH 4/9] mm/damon/core: put ops-handled filters to damos->ops_filters

[SeongJae Park]

damos->ops_filters has introduced to be used for operations layer
handled filters.  But DAMON kernel API callers can put any type of DAMOS
filters to any of damos->filters and damos->ops_filters.  DAMON ABI
users have no way other than still using only damos->filters.  Update
damos_add_filter(), which should be used by API callers to install DAMOS
filters, to add core layers and operation layer handled filters to
->filters and ->ops_filters, respectively.  The change is applied to
both API callers and ABI users since ABI users use the API internally.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index ffdd84ff6fa5..78126a5145fd 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -281,9 +281,24 @@ struct damos_filter *damos_new_filter(enum damos_filter_type type,
 	return filter;
 }
 
+static bool damos_filter_for_ops(enum damos_filter_type type)
+{
+	switch (type) {
+	case DAMOS_FILTER_TYPE_ADDR:
+	case DAMOS_FILTER_TYPE_TARGET:
+		return false;
+	default:
+		break;
+	}
+	return true;
+}
+
 void damos_add_filter(struct damos *s, struct damos_filter *f)
 {
-	list_add_tail(&f->list, &s->filters);
+	if (damos_filter_for_ops(f->type))
+		list_add_tail(&f->list, &s->ops_filters);
+	else
+		list_add_tail(&f->list, &s->filters);
 }
 
 static void damos_del_filter(struct damos_filter *f)
-- 
2.39.5

[RFC PATCH 5/9] mm/damon/paddr: support only ops_filters

[SeongJae Park]

DAMON physical address space operation set implementation (paddr) has
updated to support both ->filters and ->ops_filters to avoid breakage
during change for the ->ops_filters setup.  Now the change is done, so
paddr can safely drop support of ->filters.  The support is only waste
of the time.  Remove it.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/paddr.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 3e651308ba5d..432ea4efdc4b 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -253,10 +253,6 @@ static bool damos_pa_filter_out(struct damos *scheme, struct folio *folio)
 {
 	struct damos_filter *filter;
 
-	damos_for_each_filter(filter, scheme) {
-		if (damos_pa_filter_match(filter, folio))
-			return !filter->allow;
-	}
 	damos_for_each_ops_filter(filter, scheme) {
 		if (damos_pa_filter_match(filter, folio))
 			return !filter->allow;
@@ -285,12 +281,6 @@ static unsigned long damon_pa_pageout(struct damon_region *r, struct damos *s,
 	struct folio *folio;
 
 	/* check access in page level again by default */
-	damos_for_each_filter(filter, s) {
-		if (filter->type == DAMOS_FILTER_TYPE_YOUNG) {
-			install_young_filter = false;
-			break;
-		}
-	}
 	damos_for_each_ops_filter(filter, s) {
 		if (filter->type == DAMOS_FILTER_TYPE_YOUNG) {
 			install_young_filter = false;
@@ -543,8 +533,6 @@ static bool damon_pa_scheme_has_filter(struct damos *s)
 {
 	struct damos_filter *f;
 
-	damos_for_each_filter(f, s)
-		return true;
 	damos_for_each_ops_filter(f, s)
 		return true;
 	return false;
-- 
2.39.5

[RFC PATCH 6/9] mm/damon: add default allow/reject behavior fields to struct damos

[SeongJae Park]

It is more intuitive to decide the default filtering stage allow/reject
behavior as opposite to the last filter's behavior.  The decision should
be made separately for core and operations layers' filtering stages,
since last core layer-handled filter is not really a last filter if
there are operations layer handling filters.

Keeping separate decisions for the two categories can make the logic
simpler.  Add fields for string the two decisions.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index add82fdc1117..1d8479f57f85 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -497,6 +497,9 @@ struct damos {
 	unsigned long next_apply_sis;
 	/* informs if ongoing DAMOS walk for this scheme is finished */
 	bool walk_completed;
+	/* whether to reject core/ops filters umatched regions */
+	bool core_filters_default_reject;
+	bool ops_filters_default_reject;
 /* public: */
 	struct damos_quota quota;
 	struct damos_watermarks wmarks;
-- 
2.39.5

[RFC PATCH 7/9] mm/damon/core: set damos_filter default allowance behavior based on installed filters

[SeongJae Park]

Decide whether to allow or reject by default on core and opertions layer
handled filters evaluation stages, based on the last-installed filter's
behavior.  It is the opposite of the last installed filter's behavior.
If there is any operations layer handled filters, core layer handled
filters stage keeps allowing as the default behavior, since the last
filter of core layer handled filters in the case is not really the last
filter of the entire filtering stage.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 78126a5145fd..9744ab9ca5c5 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -864,6 +864,29 @@ static int damos_commit_ops_filters(struct damos *dst, struct damos *src)
 	return 0;
 }
 
+/**
+ * damos_filters_default_reject() - decide whether to reject memory that didn't
+ *				    match with any given filter.
+ * @filters:	Given DAMOS filters of a group.
+ */
+static bool damos_filters_default_reject(struct list_head *filters)
+{
+	struct damos_filter *last_filter;
+
+	if (list_empty(filters))
+		return false;
+	last_filter = list_last_entry(filters, struct damos_filter, list);
+	return last_filter->allow;
+}
+
+static void damos_set_filters_default_reject(struct damos *s)
+{
+	s->core_filters_default_reject =
+		damos_filters_default_reject(&s->filters);
+	s->ops_filters_default_reject =
+		damos_filters_default_reject(&s->ops_filters);
+}
+
 static int damos_commit_filters(struct damos *dst, struct damos *src)
 {
 	int err;
@@ -871,7 +894,11 @@ static int damos_commit_filters(struct damos *dst, struct damos *src)
 	err = damos_commit_core_filters(dst, src);
 	if (err)
 		return err;
-	return damos_commit_ops_filters(dst, src);
+	err = damos_commit_ops_filters(dst, src);
+	if (err)
+		return err;
+	damos_set_filters_default_reject(dst);
+	return 0;
 }
 
 static struct damos *damon_nth_scheme(int n, struct damon_ctx *ctx)
@@ -1490,7 +1517,7 @@ static bool damos_filter_out(struct damon_ctx *ctx, struct damon_target *t,
 		if (damos_filter_match(ctx, t, r, filter))
 			return !filter->allow;
 	}
-	return false;
+	return s->core_filters_default_reject;
 }
 
 /*
-- 
2.39.5

Re: [RFC PATCH 7/9] mm/damon/core: set damos_filter default allowance behavior based on installed filters

[SeongJae Park]

On Thu, 20 Feb 2025 11:35:07 -0800 SeongJae Park <sj@kernel.org> wrote:

> Decide whether to allow or reject by default on core and opertions layer
> handled filters evaluation stages, based on the last-installed filter's
> behavior.  It is the opposite of the last installed filter's behavior.
> If there is any operations layer handled filters, core layer handled
> filters stage keeps allowing as the default behavior, since the last
> filter of core layer handled filters in the case is not really the last
> filter of the entire filtering stage.

The last sentence describing behavior is not really implemented with this
commit.  See below.

> 
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
>  mm/damon/core.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 78126a5145fd..9744ab9ca5c5 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -864,6 +864,29 @@ static int damos_commit_ops_filters(struct damos *dst, struct damos *src)
>  	return 0;
>  }
>  
> +/**
> + * damos_filters_default_reject() - decide whether to reject memory that didn't
> + *				    match with any given filter.
> + * @filters:	Given DAMOS filters of a group.
> + */
> +static bool damos_filters_default_reject(struct list_head *filters)
> +{
> +	struct damos_filter *last_filter;
> +
> +	if (list_empty(filters))
> +		return false;
> +	last_filter = list_last_entry(filters, struct damos_filter, list);
> +	return last_filter->allow;
> +}
> +
> +static void damos_set_filters_default_reject(struct damos *s)
> +{
> +	s->core_filters_default_reject =
> +		damos_filters_default_reject(&s->filters);
> +	s->ops_filters_default_reject =
> +		damos_filters_default_reject(&s->ops_filters);
> +}

->core_filters_default_reject should be 'false' if s->ops_filters is not empty,
since the last one of ->ops_filters is not the real last filter.  But this code
is not handling the case.

I will fix this in the next revision.


Thanks,
SJ

[...]

[RFC PATCH 8/9] mm/damon/paddr: respect ops_filters_default_reject

[SeongJae Park]

Use damos->ops_filters_default_reject, which is decided based on the
installed filters' behaviors, from physical address space DAMON
operations set.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/paddr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 432ea4efdc4b..fee66a3cc82b 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -257,7 +257,7 @@ static bool damos_pa_filter_out(struct damos *scheme, struct folio *folio)
 		if (damos_pa_filter_match(filter, folio))
 			return !filter->allow;
 	}
-	return false;
+	return scheme->ops_filters_default_reject;
 }
 
 static bool damon_pa_invalid_damos_folio(struct folio *folio, struct damos *s)
-- 
2.39.5

[RFC PATCH 9/9] Docs/mm/damon/design: update for changed filter-default behavior

[SeongJae Park]

DAMOS filters evaluation stages' default behaviors were always allowing
before.  But the previous commits have changed the behavior to be
decided by installed filters.  Update the documentation to clearly
describe it.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 Documentation/mm/damon/design.rst | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
index 5af991551a86..ffea744e4889 100644
--- a/Documentation/mm/damon/design.rst
+++ b/Documentation/mm/damon/design.rst
@@ -581,9 +581,10 @@ When multiple filters are installed, the group of filters that handled by the
 core layer are evaluated first.  After that, the group of filters that handled
 by the operations layer are evaluated.  Filters in each of the groups are
 evaluated in the installed order.  If a part of memory is matched to one of the
-filter, next filters are ignored.  If the memory passes through the filters
+filter, next filters are ignored.  If the part passes through the filters
 evaluation stage because it is not matched to any of the filters, applying the
-scheme's action to it is allowed, same to the behavior when no filter exists.
+scheme's action to it depends on the last filter's allowance type.  If the last
+filter was for allowing, the part of memory will be rejected, and vice versa.
 
 For example, let's assume 1) a filter for allowing anonymous pages and 2)
 another filter for rejecting young pages are installed in the order.  If a page
@@ -595,11 +596,6 @@ second reject-filter blocks it.  If the page is neither anonymous nor young,
 the page will pass through the filters evaluation stage since there is no
 matching filter, and the action will be applied to the page.
 
-Note that the action can equally be applied to memory that either explicitly
-filter-allowed or filters evaluation stage passed.  It means that installing
-allow-filters at the end of the list makes no practical change but only
-filters-checking overhead.
-
 Below ``type`` of filters are currently supported.
 
 - Core layer handled
-- 
2.39.5