| Message ID | 20250227015754.38789-8-sj@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/damon: make allow filters after reject filters useful and intuitive | expand |
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;
diff --git a/mm/damon/core.c b/mm/damon/core.c index 78126a5145fd..8661f64ab1b4 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -864,6 +864,32 @@ 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) +{ + if (!list_empty(&s->ops_filters)) + s->core_filters_default_reject = false; + else + 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 +897,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 +1520,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; } /*
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. Also, make the core layer's DAMOS filters handling stage uses the newly set behavior field. Signed-off-by: SeongJae Park <sj@kernel.org> --- mm/damon/core.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-)