[PATCH 0/4] replace per-quota region priorities histogram buffer with per-context one

[PATCH 0/4] replace per-quota region priorities histogram buffer with per-context one

[SeongJae Park]

Each DAMOS quota (struct damos_quota) maintains a histogram for total
regions size per its prioritization score.  DAMOS calcultes minimum
prioritization score of regions that are ok to apply the DAMOS action to
while respecting the quota.  The histogram is constructed only for the
calculation of the minimum score in damos_adjust_quota() for each quota
which called by kdamond_fn().

Hence, there is no real reason to have per-quota histogram.  Only
per-kdamond histogram is needed, since parallel kdamonds could have
races otherwise.  The current implementation is only wasting the memory,
and can easily cause unintended stack usage[1].

So, introducing a per-kdamond histogram and replacing the per-quota one
with it would be the right solution for the issue.  However, supporting
multiple DAMON contexts per kdamond is still an ongoing work[2] without
a clear estimated time of arrival.  Meanwhile, per-context histogram
could be an effective and straightforward solution having no blocker.
Let's fix the problem first in the way.

SeongJae Park (4):
  mm/damon/core: intorduce per-context region priorities histogram
    buffer
  mm/damon/core: replace per-quota regions priority histogram buffer
    usage with per-context one
  mm/damon/core: remove per-scheme region priority histogram buffer
  Revert "mm/damon/lru_sort: adjust local variable to dynamic
    allocation"

 include/linux/damon.h |  3 ++-
 mm/damon/core.c       | 14 +++++++++++---
 mm/damon/lru_sort.c   | 15 ++++-----------
 3 files changed, 17 insertions(+), 15 deletions(-)


base-commit: 9b7ae00cf6b2d882ac7062d1cf0f752933c8e461
-- 
2.39.2

[PATCH 1/4] mm/damon/core: intorduce per-context region priorities histogram buffer

[SeongJae Park]

Introduce per-context buffer for region priority scores-total size
histogram.  Same to the per-quota one (->histogram of
struct damos_quota), the new buffer is hidden from DAMON API users by
being defined as a private field of DAMON context structure.  It is
dynamically allocated and de-allocated at the beginning and ending of
the execution of the kdamond by kdamond_fn() itself.

[1] commit 0742cadf5e4c ("mm/damon/lru_sort: adjust local variable to dynamic allocation")
[2] https://lore.kernel.org/20240531122320.909060-1-yorha.op@gmail.com

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

diff --git a/include/linux/damon.h b/include/linux/damon.h
index fcccaa9b9d54..6342d8f9b0fd 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -661,6 +661,8 @@ struct damon_ctx {
 	unsigned long next_ops_update_sis;
 	/* for waiting until the execution of the kdamond_fn is started */
 	struct completion kdamond_started;
+	/* for scheme quotas prioritization */
+	unsigned long *regions_score_histogram;
 
 /* public: */
 	struct task_struct *kdamond;
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 84d9b0fd8ace..be3d05357667 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2019,6 +2019,10 @@ static int kdamond_fn(void *data)
 		ctx->ops.init(ctx);
 	if (ctx->callback.before_start && ctx->callback.before_start(ctx))
 		goto done;
+	ctx->regions_score_histogram = kmalloc_array(DAMOS_MAX_SCORE + 1,
+			sizeof(*ctx->regions_score_histogram), GFP_KERNEL);
+	if (!ctx->regions_score_histogram)
+		goto done;
 
 	sz_limit = damon_region_sz_limit(ctx);
 
@@ -2096,6 +2100,7 @@ static int kdamond_fn(void *data)
 		ctx->callback.before_terminate(ctx);
 	if (ctx->ops.cleanup)
 		ctx->ops.cleanup(ctx);
+	kfree(ctx->regions_score_histogram);
 
 	pr_debug("kdamond (%d) finishes\n", current->pid);
 	mutex_lock(&ctx->kdamond_lock);
-- 
2.39.2

[PATCH 2/4] mm/damon/core: replace per-quota regions priority histogram buffer usage with per-context one

[SeongJae Park]

Replace the usage of per-quota region priorities histogram buffer with
the per-context one.  After this change, the per-quota histogram is not
used by anyone, and hence it is ready to be removed.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index be3d05357667..a1c32becfc73 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1623,13 +1623,16 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 		return;
 
 	/* Fill up the score histogram */
-	memset(quota->histogram, 0, sizeof(quota->histogram));
+	memset(c->regions_score_histogram, 0,
+			sizeof(*c->regions_score_histogram) *
+			(DAMOS_MAX_SCORE + 1));
 	damon_for_each_target(t, c) {
 		damon_for_each_region(r, t) {
 			if (!__damos_valid_target(r, s))
 				continue;
 			score = c->ops.get_scheme_score(c, t, r, s);
-			quota->histogram[score] += damon_sz_region(r);
+			c->regions_score_histogram[score] +=
+				damon_sz_region(r);
 			if (score > max_score)
 				max_score = score;
 		}
@@ -1637,7 +1640,7 @@ static void damos_adjust_quota(struct damon_ctx *c, struct damos *s)
 
 	/* Set the min score limit */
 	for (cumulated_sz = 0, score = max_score; ; score--) {
-		cumulated_sz += quota->histogram[score];
+		cumulated_sz += c->regions_score_histogram[score];
 		if (cumulated_sz >= quota->esz || !score)
 			break;
 	}
-- 
2.39.2

[PATCH 3/4] mm/damon/core: remove per-scheme region priority histogram buffer

[SeongJae Park]

Nobody is reading from or writing to the per-scheme region priorities
histogram buffer.  It is only wasting memory.  Remove it.

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

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 6342d8f9b0fd..0ac8b862bdcb 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -242,7 +242,6 @@ struct damos_quota {
 	unsigned long charge_addr_from;
 
 	/* For prioritization */
-	unsigned long histogram[DAMOS_MAX_SCORE + 1];
 	unsigned int min_score;
 
 	/* For feedback loop */
-- 
2.39.2

[PATCH 4/4] Revert "mm/damon/lru_sort: adjust local variable to dynamic allocation"

[SeongJae Park]

This reverts commit 0742cadf5e4c080aa9bab323dfb234c37a86e884.

The commit was introduced to avoid unnecessary usage of stack memory for
per-scheme region priorities histogram buffer.  The fix is nice, but the
point of the fix looks not very clear if the commit message is not read
together.  That's mainly because the buffer is a private field, which
means it is hidden from the DAMON API users.  That's not the fault of
the fix but the underlying data structure.

Now the per-scheme histogram buffer is gone, so the problem that the
commit was fixing is also removed.  The use of kmemdup() has no more
point but just making the code bit difficult to understand.  Revert the
fix.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/lru_sort.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 0b35bd5fb659..4af8fd4a390b 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -148,17 +148,12 @@ static struct damon_target *target;
 static struct damos *damon_lru_sort_new_scheme(
 		struct damos_access_pattern *pattern, enum damos_action action)
 {
-	struct damos *damos;
-	struct damos_quota *quota = kmemdup(&damon_lru_sort_quota,
-				    sizeof(damon_lru_sort_quota), GFP_KERNEL);
-
-	if (!quota)
-		return NULL;
+	struct damos_quota quota = damon_lru_sort_quota;
 
 	/* Use half of total quota for hot/cold pages sorting */
-	quota->ms = quota->ms / 2;
+	quota.ms = quota.ms / 2;
 
-	damos = damon_new_scheme(
+	return damon_new_scheme(
 			/* find the pattern, and */
 			pattern,
 			/* (de)prioritize on LRU-lists */
@@ -166,12 +161,10 @@ static struct damos *damon_lru_sort_new_scheme(
 			/* for each aggregation interval */
 			0,
 			/* under the quota. */
-			quota,
+			&quota,
 			/* (De)activate this according to the watermarks. */
 			&damon_lru_sort_wmarks,
 			NUMA_NO_NODE);
-	kfree(quota);
-	return damos;
 }
 
 /* Create a DAMON-based operation scheme for hot memory regions */
-- 
2.39.2