Re: [PATCH] mm/damon: add MADV_COLLAPSE support in damos_action

[haoxin]

Hi  SJ,

在 2022/9/13 下午11:17, SeongJae Park 写道:
> Hi Xin,
>
> On Tue, 13 Sep 2022 19:47:35 +0800 Xin Hao <xhao@xxxxxxxxxxxxxxxxx> 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.

Yes, internally, we use this feature mainly focus on two reason:

the fist one,

    prevent performance jitter casused by khugepage thread under memory 
pressure.

the second one,

    khugepage thread merge hugepages is  slowness then MADV_COLLAPSE.

> Als, 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@xxxxxxxxxxxxxxxxx>
> > ---
> >   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