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]
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ