[v5] mm/damon/lru_sort: adjust local variable to dynamic allocation

Commit Message

Hao Peng July 23, 2024, 3:55 a.m. UTC

From: Peng Hao <[e-mail]>

When KASAN is enabled and built with clang:
    mm/damon/lru_sort.c:199:12: error: stack frame size (2328) exceeds
limit (2048) in 'damon_lru_sort_apply_parameters' [-Werror,-Wframe-larger-than]
    static int damon_lru_sort_apply_parameters(void)
               ^
    1 error generated.

This is because damon_lru_sort_quota contains a large array, and
assigning this variable to a local variable causes a large amount of
stack space to be occupied.

So adjust local variable to dynamic allocation.

Link: https://lore.kernel.org/all/[e-mail]/
Link: https://lore.kernel.org/linux-mm/[e-mail]/T/

Signed-off-by: Peng Hao <[e-mail]>
---
v4 -> v5: rebase on latest mm-unstable tree.
v3 -> v4: adjust release point, tweak format, etc.
v2 -> v3: don't change histgram array in damon_quota, dynamically
          allocating local variables
v1 -> v2: don't modify global variables and change the histgram array in
          damon_quota to dynamic allocation
 mm/damon/lru_sort.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

Comments

SeongJae Park July 23, 2024, 4:08 p.m. UTC | #1

Hi Peng,


Thank you for patiently following the discussion and sending this, Peng!

On Tue, 23 Jul 2024 11:55:13 +0800 [e-mail] wrote:

> From: Peng Hao <[e-mail]>
> 
> When KASAN is enabled and built with clang:
>     mm/damon/lru_sort.c:199:12: error: stack frame size (2328) exceeds
> limit (2048) in 'damon_lru_sort_apply_parameters' [-Werror,-Wframe-larger-than]
>     static int damon_lru_sort_apply_parameters(void)
>                ^
>     1 error generated.
> 
> This is because damon_lru_sort_quota contains a large array, and
> assigning this variable to a local variable causes a large amount of
> stack space to be occupied.
> 
> So adjust local variable to dynamic allocation.
> 
> Link: https://lore.kernel.org/all/[e-mail]/
> Link: https://lore.kernel.org/linux-mm/[e-mail]/T/

I guess above two links are for revision history of this patch?  If so, it
would better to put together with revision history under the below '---' line.
If not, it would be better to add more clear contexts on the commit message.

> 
> Signed-off-by: Peng Hao <[e-mail]>

Except the above trivial comment,

Reviewed-by: SeongJae Park <[e-mail]>

I think Andrew would handle the trivial comment on his own when he add this
patch to mm tree.  You could post v6 with the above my Reviewed-by: tag if you
prefer to, of course.


Thanks,
SJ

[...]

Patch

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 4af8fd4a390b..0b35bd5fb659 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -148,12 +148,17 @@  static struct damon_target *target;
 static struct damos *damon_lru_sort_new_scheme(
 		struct damos_access_pattern *pattern, enum damos_action action)
 {
-	struct damos_quota quota = damon_lru_sort_quota;
+	struct damos *damos;
+	struct damos_quota *quota = kmemdup(&damon_lru_sort_quota,
+				    sizeof(damon_lru_sort_quota), GFP_KERNEL);
+
+	if (!quota)
+		return NULL;
 
 	/* Use half of total quota for hot/cold pages sorting */
-	quota.ms = quota.ms / 2;
+	quota->ms = quota->ms / 2;
 
-	return damon_new_scheme(
+	damos = damon_new_scheme(
 			/* find the pattern, and */
 			pattern,
 			/* (de)prioritize on LRU-lists */
@@ -161,10 +166,12 @@  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 */