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

[SeongJae Park <sj@kernel.org>]

On Wed, 26 Feb 2025 17:57:52 -0800 SeongJae Park <sj@kernel.org> wrote:

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

This is not sufficient enough.  Even with this change, core-handled allow
filters after core-handled reject filters are still meaningless.

If a region is matched to a core layer handled filter, the allow/reject
decision should be respected while ignoring all remaining filters, regardless
of on what layer those are handled.  It works in the way for reect filters,
since core layer-rejected regions are not passed to the ops layer at all.  In
case of allow filter, however, the region is passed to ops layer without the
information about whether it has passed to the ops layer because it was
allowed, or just not matched to any filter.  Hence, all ops filters can be
applied to the region.

We can implement this missing part by storing the core layer filtering stage
decision somewhere and let ops filter filtering stage repsect it.  Changes like
attached diff at the end of this mail may work.  I will add such changes to
next version of this patch series.

Thanks,
SJ

[...]

=================================== >8 ========================================
diff --git a/include/linux/damon.h b/include/linux/damon.h
index b7a4d12ce532..8d8621d8b58d 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -528,6 +528,7 @@ struct damos {
        unsigned long next_apply_sis;
        /* informs if ongoing DAMOS walk for this scheme is finished */
        bool walk_completed;
+       bool skip_ops_filters;
        /* whether to reject core/ops filters umatched regions */
        bool core_filters_default_reject;
        bool ops_filters_default_reject;
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 783cc05a9fcc..a2e4bdbb6b19 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1662,9 +1662,19 @@ static bool damos_filter_out(struct damon_ctx *ctx, struct damon_target *t,
 {
        struct damos_filter *filter;

+       s->skip_ops_filters = false;
        damos_for_each_filter(filter, s) {
-               if (damos_filter_match(ctx, t, r, filter))
+               if (damos_filter_match(ctx, t, r, filter)) {
+                       /*
+                        * ops layer filters should also respect the decision.
+                        * Store the information in s->skip_ops_filters.
+                        * If the decision is reject, the region will not be
+                        * passed to ops layer, so no need to set the flag.
+                        */
+                       if (filter->allow)
+                               s->skip_ops_filters = true;
                        return !filter->allow;
+               }
        }
        return s->core_filters_default_reject;
 }
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 4a170086852a..8568b5a34888 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -393,6 +393,9 @@ static bool damos_pa_filter_out(struct damos *scheme, struct folio *folio)
 {
        struct damos_filter *filter;

+       if (scheme->skip_ops_filters)
+               return false;
+
        damos_for_each_ops_filter(filter, scheme) {
                if (damos_pa_filter_match(filter, folio))
                        return !filter->allow;