Hi Peng,
For DAMON patches, please Cc linux-mm@. I'd also suggest Cc-ing linux-kernel@
if you don't mind.
On Fri, 19 Jul 2024 17:11:00 +0800 flyingpenghao@xxxxxxxxx wrote:
> From: Peng Hao <flyingpeng@xxxxxxxxxxx>
>
> 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.
>
> v2: Change the histgram array in damon_quota to dynamic allocation.
> v1: Modify global variables directly.
Please put the version history under '---' line below, as suggested[1] by the
document. Also, adding lore links to the previous versions would be helpful.
>
> Signed-off-by: Peng Hao <flyingpeng@xxxxxxxxxxx>
> ---
> mm/damon/core.c | 1 +
> mm/damon/lru_sort.c | 10 +++++++---
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 6392f1cc97a3..ac2b736c9115 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -375,6 +375,7 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
> INIT_LIST_HEAD(&scheme->list);
>
> scheme->quota = *(damos_quota_init(quota));
> + kfree(quota);
We don't know if the caller has something to do further with it after this
function returns. Let's keep it do be done by the caller.
> /* quota.goals should be separately set by caller */
> INIT_LIST_HEAD(&scheme->quota.goals);
>
> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 3de2916a65c3..3b417e413bf2 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
> @@ -148,10 +148,14 @@ 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_quota *quota = kmemdup(&damon_lru_sort_quota,
> + sizeof(struct damos_quota), GFP_KERNEL);
Let's do sizeof(*damon_lru_sort_quota). Also, please keep 80 columns line size
rule[2] for DAMON code.
[1] https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format
[2] https://docs.kernel.org/process/coding-style.html#breaking-long-lines-and-strings
> +
> + 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(
> /* find the pattern, and */
> @@ -161,7 +165,7 @@ static struct damos *damon_lru_sort_new_scheme(
> /* for each aggregation interval */
> 0,
> /* under the quota. */
> - "a,
> + quota,
> /* (De)activate this according to the watermarks. */
> &damon_lru_sort_wmarks);
> }
> --
> 2.27.0
Thanks,
SJ
