linux-kernel - Re: [PATCH] mm/damon: add MADV_COLLAPSE support in damos

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220913151755.103334-1-sj@kernel.org>
Date:   Tue, 13 Sep 2022 15:17:55 +0000
From:   SeongJae Park <sj@...nel.org>
To:     Xin Hao <xhao@...ux.alibaba.com>
Cc:     sj@...nel.org, akpm@...ux-foundation.org, damon@...ts.linux.dev,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/damon: add MADV_COLLAPSE support in damos_action

Hi Xin,

On Tue, 13 Sep 2022 19:47:35 +0800 Xin Hao <xhao@...ux.alibaba.com> wrote:

> MADV_COLLAPSE has large different with MDVA_HUGE in dealing with huge
> pages, it doesn't need to rely on 'khugepage' thread to create THP.
> So combining with it will give damon users more options.

I think this is a simple and worthy change, but... could you put some more
detailed explanation of the scenario that the action could be used?  Also, it
would be good to have some evaluation results you got with this change.

Also, to my understanding, MADV_COLLAPSE is not merged in the mainline yet,
right?  I believe it will make it in v6.1-rc1.  That said, to make Andrew's
burden less, how about delaying this change until MADV_COLLAPSE is mainlined?
If you have some numbers showing clear and great benefit of this change, I
think it would be worthy to add immediately, though.


Thanks,
SJ

> 
> Signed-off-by: Xin Hao <xhao@...ux.alibaba.com>
> ---
>  include/linux/damon.h | 1 +
>  mm/damon/vaddr.c      | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 016b6c9c03d6..c44f9410d997 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -97,6 +97,7 @@ enum damos_action {
>  	DAMOS_PAGEOUT,
>  	DAMOS_HUGEPAGE,
>  	DAMOS_NOHUGEPAGE,
> +	DAMOS_COLLAPSE,
>  	DAMOS_LRU_PRIO,
>  	DAMOS_LRU_DEPRIO,
>  	DAMOS_STAT,		/* Do nothing but only record the stat */
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index c2c08c1b316b..58c0d068563d 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -655,6 +655,9 @@ static unsigned long damon_va_apply_scheme(struct damon_ctx *ctx,
>  	case DAMOS_NOHUGEPAGE:
>  		madv_action = MADV_NOHUGEPAGE;
>  		break;
> +	case DAMOS_COLLAPSE:
> +		madv_action = MADV_COLLAPSE;
> +		break;
>  	case DAMOS_STAT:
>  		return 0;
>  	default:
> -- 
> 2.31.0