Re: [RFC PATCH v2 0/7] DAMON based 2-tier memory management for CXL memory

[SeongJae Park]

On Sun, 17 Mar 2024 08:31:44 -0700 SeongJae Park <sj@xxxxxxxxxx> wrote:

> Hi Honggyu,
> 
> On Sun, 17 Mar 2024 17:36:29 +0900 Honggyu Kim <honggyu.kim@xxxxxx> wrote:
> 
> > Hi SeongJae,
> > 
> > Thanks for the confirmation.  I have a few comments on young filter so
> > please read the inline comments again.
> > 
> > On Wed, 12 Mar 2024 08:53:00 -0800 SeongJae Park <sj@xxxxxxxxxx> wrote:
> > > Hi Honggyu,
[...]
> > Thanks.  I see that it works fine, but I would like to have more
> > discussion about "young" filter.  What I think about filter is that if I
> > apply "young" filter "true" for demotion, then the action applies only
> > for "young" pages, but the current implementation works opposite.
> > 
> > I understand the function name of internal implementation is
> > "damos_pa_filter_out" so the basic action is filtering out, but the
> > cgroup filter works in the opposite way for now.
> 
> Does memcg filter works in the opposite way?  I don't think so because
> __damos_pa_filter_out() sets 'matches' as 'true' only if the the given folio is
> contained in the given memcg.  'young' filter also simply sets 'matches' as
> 'true' only if the given folio is young.
> 
> If it works in the opposite way, it's a bug that need to be fixed.  Please let
> me know if I'm missing something.

I just read the DAMOS filters part of the documentation for DAMON sysfs
interface again.  I believe it is explaining the meaning of 'matching' as I
intended to, as below:

    You can write ``Y`` or ``N`` to ``matching`` file to filter out pages that does
    or does not match to the type, respectively.  Then, the scheme's action will
    not be applied to the pages that specified to be filtered out.

But, I found the following example for memcg filter is wrong, as below:

    For example, below restricts a DAMOS action to be applied to only non-anonymous
    pages of all memory cgroups except ``/having_care_already``.::
    
        # echo 2 > nr_filters
        # # filter out anonymous pages
        echo anon > 0/type
        echo Y > 0/matching
        # # further filter out all cgroups except one at '/having_care_already'
        echo memcg > 1/type
        echo /having_care_already > 1/memcg_path
        echo N > 1/matching

Specifically, the last line of the commands should write 'Y' instead of 'N' to
do what explained.  Without the fix, the action will be applied to only
non-anonymous pages of 'having_care_already' memcg.  This is definitely wrong.
I will fix this soon.  I'm unsure if this is what made you to believe memcg
DAMOS filter is working in the opposite way, though.


Thanks,
SJ

[...]